Re: [PATCH 1/2] mm/page_alloc: remove track of active PCP lists range in bulk free

[Chris Li]

Hi Kemeng,

On Wed, Aug 16, 2023 at 7:22 PM Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx> wrote:
> Hi Chris, there is no functional change intended in this patch. As
> I menthioned in changelog, there is no wrap for list iteration, so
> that the active PCP lists range will never be used.
> > 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.

In this case the BPF allocates a page inside spin_lock. The "pcp->count" is
smaller than "count". The loop condition only checks "count > 0" but all
pcp->lists pages have been free. pcp->count is 0 but "count" is 1.
After a few times wrap around, the pindex_max is smaller than pindex_min,
then reach to -1 cause the invalid page fault.

> I guess pcp->count is set to bigger than it should be. In this case,
> we will keep trying get pages while all pages in pcp list were taken
> off already and dead lock will happen. In this case, dead looo will
> happen with or without this patch as the root cause is that we try
> to get pages more than pcp list owns.> Maybe I can send my out alone side with yours for discussion?

My patch is split into two parts, the first patch is a functional
change to allow
pcp->count drop below "count".
The other patch is just to clean up, and should have the same function.

Sure will send it out and CC you for discussion.

> > I don't mind my patch combined with yours.
> >
> Either way is acceptable to me, just feel free to choose one you like
> and I'd like to see if more we could do to this.

Thanks

Chris