On Fri, Sep 01, 2017 at 06:59:49PM +0800, Qu Wenruo wrote:
> On 2017年09月01日 16:58, Naohiro Aota wrote:
> > commit 524272607e88 ("btrfs: Handle delalloc error correctly to avoid
> > ordered extent hang") introduced btrfs_cleanup_ordered_extents() to cleanup
> > submitted ordered extents. However, it does not clear the ordered bit
> > (Private2) of coresponding pages. Thus, the following BUG occurs from
> > free_pages_check_bad() (on btrfs/125 with nospace_cache).
> >
> > BUG: Bad page state in process btrfs pfn:3fa787
> > page:ffffdf2acfe9e1c0 count:0 mapcount:0 mapping: (null) index:0xd
> > flags: 0x8000000000002008(uptodate|private_2)
> > raw: 8000000000002008 0000000000000000 000000000000000d 00000000ffffffff
> > raw: ffffdf2acf5c1b20 ffffb443802238b0 0000000000000000 0000000000000000
> > page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set
> > bad because of flags: 0x2000(private_2)
> >
> > This patch clear the flag as same as other places calling
> > btrfs_dec_test_ordered_pending() for every page in the specified range.
> >
> > Signed-off-by: Naohiro Aota <naohiro.a...@wdc.com>
> > Fixes: 524272607e88 ("btrfs: Handle delalloc error correctly to avoid
> > ordered extent hang")
> > Cc: <sta...@vger.kernel.org> # 4.12
> > ---
> > fs/btrfs/inode.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 24bcd5cd9cf2..ae4c0a1bef38 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -135,6 +135,18 @@ static inline void
> > btrfs_cleanup_ordered_extents(struct inode *inode,
> > const u64 offset,
> > const u64 bytes)
> > {
> > + unsigned long index = offset >> PAGE_SHIFT;
> > + unsigned long end_index = (offset + bytes - 1) >> PAGE_SHIFT;
> > + struct page *page;
> > +
> > + while (index <= end_index) {
> > + page = find_get_page(inode->i_mapping, index);
> > + index++;
> > + if (!page)
> > + continue;
> > + ClearPagePrivate2(page);
> > + put_page(page);
> > + }
>
> At first glance, explicitly clearing Private2 flag here seems a little
> strange to me.
> However btrfs_invalidatepage() is also doing the same thing, I think
> it's fine.
>
> Reviewed-by: Qu Wenruo <quwenruo.bt...@gmx.com>
>
> BTW, Private2 flag is set by extent_clear_unlock_delalloc() with
> page_ops |= PAGE_SET_PRIVATE2, but we're clearing the page flag without
> any encapsulation, it may be better to use similar function to clear
> Private2 flag.
I agree, the Private2 flag is given another meaning in btrfs, ie. the
writeback status, so this would be better wrapped in helpers that
reflect what is the private2 flag used for. The helpers might be
trivial, but their name will be a better documentation than the random
comments that we can be found next to its use.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
