Messages in this thread | | | From | Chris Li <> | Date | Thu, 17 Aug 2023 00:51:23 -0700 | Subject | Re: [PATCH 1/2] mm/page_alloc: remove track of active PCP lists range in bulk free |
| |
Hi Kemeng,
On Wed, Aug 16, 2023 at 7:22 PM Kemeng Shi <shikemeng@huaweicloud.com> 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
| |