On 1/16/21 2:15 AM, Qu Wenruo wrote:

When __process_pages_contig() get called for
extent_clear_unlock_delalloc(), if we hit the locked page, only Private2
bit is updated, but dirty/writeback/error bits are all skipped.

There are several call sites call extent_clear_unlock_delalloc() with
@locked_page and PAGE_CLEAR_DIRTY/PAGE_SET_WRITEBACK/PAGE_END_WRITEBACK

- cow_file_range()
- run_delalloc_nocow()
- cow_file_range_async()
   All for their error handling branches.

For those call sites, since we skip the locked page for
dirty/error/writeback bit update, the locked page will still have its
dirty bit remaining.

Thankfully, since all those call sites can only be hit with various
serious errors, it's pretty hard to hit and shouldn't affect regular
btrfs operations.

But still, we shouldn't leave the locked_page with its
dirty/error/writeback bits untouched.

Fix this by only skipping lock/unlock page operations for locked_page.

Signed-off-by: Qu Wenruo <redacted>

Except this is handled by the callers.  We clear_page_dirty_for_io() the page 
before calling btrfs_run_delalloc_range(), so we don't need the 
PAGE_CLEAR_DIRTY, it's already cleared.  The SetPageError() is handled in the 
error path for locked_page, as is the set_writeback/end_writeback.  Now I don't 
think this patch causes problems specifically, but the changelog is at least 
wrong, and I'd rather we'd skip the handling of the locked_page here and leave 
it in the proper error handling.  If you need to do this for some other reason 
that I haven't gotten to yet then you need to make that clear in the changelog, 
because as of right now I don't see why this is needed.  Thanks,

Josef