Hi Kemeng,
Can you confirm this patch has no intended functional change?
I have a patch sitting in my tree for a while related to this
count vs pcp->count. The BPF function hook can potentially change
pcp->count and make count out of sync with pcp->count which causes
a dead loop.
Maybe I can send my out alone side with yours for discussion?
I don't mind my patch combined with yours.
Your change looks fine to me. There is more can be done
on the clean up.
Chris
On Wed, Aug 09, 2023 at 06:07:53PM +0800, Kemeng Shi wrote:
> After commit fd56eef258a17 ("mm/page_alloc: simplify how many pages are
> selected per pcp list during bulk free"), we will drain all pages in
> selected pcp list. And we ensured passed count is < pcp->count. Then,
> the search will finish before wrap-around and track of active PCP lists
> range intended for wrap-around case is no longer needed.
>
> Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx>
> ---
> mm/page_alloc.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 96b7c1a7d1f2..1ddcb2707d05 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1207,8 +1207,6 @@ static void free_pcppages_bulk(struct zone *zone, int count,
> int pindex)
> {
> unsigned long flags;
> - int min_pindex = 0;
> - int max_pindex = NR_PCP_LISTS - 1;
> unsigned int order;
> bool isolated_pageblocks;
> struct page *page;
> @@ -1231,17 +1229,10 @@ static void free_pcppages_bulk(struct zone *zone, int count,
>
> /* Remove pages from lists in a round-robin fashion. */
> do {
> - if (++pindex > max_pindex)
> - pindex = min_pindex;
> + if (++pindex > NR_PCP_LISTS - 1)
> + pindex = 0;
> list = &pcp->lists[pindex];
> - if (!list_empty(list))
> - break;
> -
> - if (pindex == max_pindex)
> - max_pindex--;
> - if (pindex == min_pindex)
> - min_pindex++;
> - } while (1);
> + } while (list_empty(list));
>
> order = pindex_to_order(pindex);
> nr_pages = 1 << order;
> --
> 2.30.0
>
