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
