Re: [f2fs-dev] [PATCH] f2fs: initialize page->private when using for our internal use

[Jaegeuk Kim]

On 07/06, Chao Yu wrote:
> On 2021/7/5 19:47, Matthew Wilcox wrote:
> > On Mon, Jul 05, 2021 at 07:33:35PM +0800, Chao Yu wrote:
> > > On 2021/7/5 16:56, Jaegeuk Kim wrote:
> > > > On 07/05, Chao Yu wrote:
> > > > > On 2021/7/5 13:22, Jaegeuk Kim wrote:
> > > > > > We need to guarantee it's initially zero. Otherwise, it'll hurt entire flag
> > > > > > operations.
> > > > > 
> > > > > Oops, I didn't get the point, shouldn't .private be zero after page was
> > > > > just allocated by filesystem? What's the case we will encounter stall
> > > > > private data left in page?
> > > > 
> > > > I'm seeing f2fs_migrate_page() has the newpage with some value without Private
> > > > flag. That causes a kernel panic later due to wrong private flag used in f2fs.
> > > 
> > > I'm not familiar with that part of codes, so Cc mm mailing list for help.
> > > 
> > > My question is newpage in .migrate_page() may contain non-zero value in .private
> > > field but w/o setting PagePrivate flag, is it a normal case?
> > 
> > I think freshly allocated pages have a page->private of 0.  ie this
> > code in mm/page_alloc.c:
> > 
> >                  page = rmqueue(ac->preferred_zoneref->zone, zone, order,
> >                                  gfp_mask, alloc_flags, ac->migratetype);
> >                  if (page) {
> >                          prep_new_page(page, order, gfp_mask, alloc_flags);
> > 
> > where prep_new_page() calls post_alloc_hook() which contains:
> >          set_page_private(page, 0);
> > 
> > Now, I do see in __buffer_migrate_page() (mm/migrate.c):
> > 
> >          attach_page_private(newpage, detach_page_private(page));
> > 
> > but as far as I can tell, f2fs doesn't call any of the
> > buffer_migrate_page() paths.  So I'm not sure why you're seeing
> > a non-zero page->private.
> 
> Well, that's strange.
> 
> Jaegeuk, let's add a BUGON in f2fs to track the call path where newpage
> has non-zero private value? if this issue is reproducible.

We can debug anything tho, this issue is blocking the production, and I'd
like to get this in this merge windows. Could you please check the patch
has any holes?

> 
> Thanks,
> 
> >