On Tue, Mar 12, 2019 at 08:42:12AM +0800, Qu Wenruo wrote:
>
>
> On 2019/3/12 上午8:33, David Sterba wrote:
> > On Mon, Feb 18, 2019 at 01:27:51PM +0800, Qu Wenruo wrote:
> >> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> >> Reviewed-by: Johannes Thumshirn <jthumshirn@xxxxxxx>
> >> ---
> >> fs/btrfs/extent_io.c | 10 ++++++++--
> >> 1 file changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> >> index 1572e892ec7b..480e138051f0 100644
> >> --- a/fs/btrfs/extent_io.c
> >> +++ b/fs/btrfs/extent_io.c
> >> @@ -3998,7 +3998,10 @@ static int extent_write_cache_pages(struct address_space *mapping,
> >> */
> >> if (!trylock_page(page)) {
> >> ret = flush_write_bio(epd);
> >> - BUG_ON(ret < 0);
> >> + if (ret < 0) {
> >
> > This needs some more explanation why it's correct, there's conditional
> > locking and writeback status manipulation. Thanks.
>
> Because flush_write_bio() handles the cleanup in its endio function.
>
> Thus we don't need extra cleanup in this case.
Non-trivial changes need changelog, that's nothing new. Especially for
error handling you have to provide a proof that things don't get broken.
If you spend a week working on the code, everything is trivial, but only
for you and for a short period of time. That's why we need good
changelogs that make sense to other people and after a long period of
time. Thanks.
