On 01/20/2020 06:06 PM, Wei Yang wrote: > On Mon, Jan 20, 2020 at 12:13:38PM +0530, Anshuman Khandual wrote: >> >> >> On 01/20/2020 08:34 AM, Wei Yang wrote: >>> During free and new page, we did some check on the status of page >>> struct. There is some common part, just extract them. >> >> Makes sense. >> >>> >>> Besides this, this patch also rename two functions to keep the name >>> convention, since free_pages_check_bad/free_pages_check are counterparts >>> of check_new_page_bad/check_new_page. >> >> This probably should be in a different patch. >> > > In v1, they are in two separate patches. While David Suggest to merge it. > > I am not sure whether my understanding is correct. Keeping code refactoring and renaming separate is always better but its okay, will leave it upto you. > >>> >>> Signed-off-by: Wei Yang <richardw.yang@xxxxxxxxxxxxxxx> >>> --- >>> mm/page_alloc.c | 49 ++++++++++++++++++++++++------------------------- >>> 1 file changed, 24 insertions(+), 25 deletions(-) >>> >>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>> index a7b793c739fc..7f23cc836f90 100644 >>> --- a/mm/page_alloc.c >>> +++ b/mm/page_alloc.c >>> @@ -1025,36 +1025,44 @@ static inline bool page_expected_state(struct page *page, >>> return true; >>> } >>> >>> -static void free_pages_check_bad(struct page *page) >>> +static inline int __check_page(struct page *page, int nr, >>> + const char **bad_reason) >> >> free and new page checks are in and out of the buddy allocator, hence >> this common factored function should have a more relevant name. > > Hmm... naming is really difficult. Do you have any suggestion? > Probably something like bad_page_fetch_reasons() and also this helper can be expanded to include an additional argument 'bool free' which can test either PAGE_FLAGS_CHECK_AT_FREE or PAGE_FLAGS_CHECK_AT_PREP. This way bad_page_fetch_reasons() is where all possible reasons are evaluated including page->flags based and then final 'int nr' can be returned once and for all. bad_flags does not seem to be needed. Wondering what it adds more in bad_page() when page->flags gets printed universally through __dump_page(). In case it is still required, it can be derived from 'bad_reasons' evaluated in bad_page_fetch_reasons().
