On 1/27/21 2:20 AM, Michal Hocko wrote: > [sorry for jumping in late] > > On Fri 22-01-21 11:52:27, Mike Kravetz wrote: >> As hugetlbfs evolved, state information about hugetlb pages was added. >> One 'convenient' way of doing this was to use available fields in tail >> pages. Over time, it has become difficult to know the meaning or contents >> of fields simply by looking at a small bit of code. Sometimes, the >> naming is just confusing. For example: The PagePrivate flag indicates >> a huge page reservation was consumed and needs to be restored if an error >> is encountered and the page is freed before it is instantiated. The >> page.private field contains the pointer to a subpool if the page is >> associated with one. > > OK, I thought the page.private was abused more than for this very > specific case. > >> In an effort to make the code more readable, use page.private to contain >> hugetlb specific page flags. These flags will have test, set and clear >> functions similar to those used for 'normal' page flags. More importantly, >> an enum of flag values will be created with names that actually reflect >> their purpose. > > This is definitely a step into the right direction! > >> In this patch, >> - Create infrastructure for hugetlb specific page flag functions >> - Move subpool pointer to page[1].private to make way for flags >> Create routines with meaningful names to modify subpool field > > This makes some sense as well. It is really important that the primary > state is stored in the head page. The respective data can be in tail > pages. > >> - Use new HPageRestoreReserve flag instead of PagePrivate > > Much better! Although wouldn't HPageReserve be sufficient? The flag name > doesn't really need to tell explicitly what to do with the reserve, > right? Or would that be too confusing? Thanks for taking a look. HPageReserve could be sufficient. I don't have a strong opinion and was just trying to add as much meaning to the name as possible. If you do not have a strong opinion, I would just leave it as is. -- Mike Kravetz
