| Message ID | 20250321010710.548105-1-ye.liu@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/page_alloc: Simplify free_page_is_bad by removing free_page_is_bad_report | expand |
On 3/21/25 06:37, Ye Liu wrote: > From: Ye Liu <liuye@kylinos.cn> > > This patch refactors the free_page_is_bad function by directly calling > bad_page() instead of using the intermediary function > free_page_is_bad_report(). The removal of free_page_is_bad_report() > reduces unnecessary indirection, making the code cleaner and easier to > read. > > The functionality remains the same, as free_page_is_bad_report() was > merely a wrapper for the bad_page() call. The patch also improves > maintainability by reducing the function call depth. Seems to be a sensible clean up indeed. > > Signed-off-by: Ye Liu <liuye@kylinos.cn> > --- > mm/page_alloc.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 61d6a3b1b286..2842da893eea 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -933,19 +933,14 @@ static const char *page_bad_reason(struct page *page, unsigned long flags) > return bad_reason; > } > > -static void free_page_is_bad_report(struct page *page) > -{ > - bad_page(page, > - page_bad_reason(page, PAGE_FLAGS_CHECK_AT_FREE)); > -} > - > static inline bool free_page_is_bad(struct page *page) > { > if (likely(page_expected_state(page, PAGE_FLAGS_CHECK_AT_FREE))) > return false; > > /* Something has gone sideways, find it */ > - free_page_is_bad_report(page); > + bad_page(page, > + page_bad_reason(page, PAGE_FLAGS_CHECK_AT_FREE)); Please fold these above two lines into a single line instead. bad_page(page, page_bad_reason(page, PAGE_FLAGS_CHECK_AT_FREE)); > return true; > } > With the above suggested change in place. Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
在 2025/3/21 12:39, Anshuman Khandual 写道: > > On 3/21/25 06:37, Ye Liu wrote: >> From: Ye Liu <liuye@kylinos.cn> >> >> This patch refactors the free_page_is_bad function by directly calling >> bad_page() instead of using the intermediary function >> free_page_is_bad_report(). The removal of free_page_is_bad_report() >> reduces unnecessary indirection, making the code cleaner and easier to >> read. >> >> The functionality remains the same, as free_page_is_bad_report() was >> merely a wrapper for the bad_page() call. The patch also improves >> maintainability by reducing the function call depth. > Seems to be a sensible clean up indeed. > >> Signed-off-by: Ye Liu <liuye@kylinos.cn> >> --- >> mm/page_alloc.c | 9 ++------- >> 1 file changed, 2 insertions(+), 7 deletions(-) >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index 61d6a3b1b286..2842da893eea 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -933,19 +933,14 @@ static const char *page_bad_reason(struct page *page, unsigned long flags) >> return bad_reason; >> } >> >> -static void free_page_is_bad_report(struct page *page) >> -{ >> - bad_page(page, >> - page_bad_reason(page, PAGE_FLAGS_CHECK_AT_FREE)); >> -} >> - >> static inline bool free_page_is_bad(struct page *page) >> { >> if (likely(page_expected_state(page, PAGE_FLAGS_CHECK_AT_FREE))) >> return false; >> >> /* Something has gone sideways, find it */ >> - free_page_is_bad_report(page); >> + bad_page(page, >> + page_bad_reason(page, PAGE_FLAGS_CHECK_AT_FREE)); > Please fold these above two lines into a single line instead. > > bad_page(page, page_bad_reason(page, PAGE_FLAGS_CHECK_AT_FREE)); > >> return true; >> } >> > With the above suggested change in place. > > Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com> Thank you for the suggestion. I'll make the adjustment and submit a new patch. Appreciate your time and the review! Thanks, Ye Liu
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 61d6a3b1b286..2842da893eea 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -933,19 +933,14 @@ static const char *page_bad_reason(struct page *page, unsigned long flags) return bad_reason; } -static void free_page_is_bad_report(struct page *page) -{ - bad_page(page, - page_bad_reason(page, PAGE_FLAGS_CHECK_AT_FREE)); -} - static inline bool free_page_is_bad(struct page *page) { if (likely(page_expected_state(page, PAGE_FLAGS_CHECK_AT_FREE))) return false; /* Something has gone sideways, find it */ - free_page_is_bad_report(page); + bad_page(page, + page_bad_reason(page, PAGE_FLAGS_CHECK_AT_FREE)); return true; }