On Thu, Apr 10, 2025 at 01:46:06PM -0700, Dan Williams wrote: > Matthew Wilcox wrote: > > On Thu, Apr 10, 2025 at 01:15:07PM -0700, Dan Williams wrote: > > > For consistency and clarity what about this incremental change, to make > > > the __split_folio_to_order() path reuse folio_reset_order(), and use > > > typical bitfield helpers for manipulating _flags_1? > > > > I dislike this intensely. It obfuscates rather than providing clarity. > > I'm used to pushing folks to use bitfield.h in driver land, but will not > push it further here.
I think it can make sense in places. Just not here. > What about this hunk? > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index 2a47682d1ab7..301ca9459122 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -3404,7 +3404,7 @@ static void __split_folio_to_order(struct folio *folio, > int old_order, > if (new_order) > folio_set_order(folio, new_order); > else > - ClearPageCompound(&folio->page); > + folio_reset_order(folio); > } I think that's wrong. We're splitting this folio into order-0 folios, but folio_reset_order() is going to modify folio->_flags_1 which is in the next page.
