On Tue, May 31, 2022 at 8:47 AM Jakub Kicinski <kuba@xxxxxxxxxx> wrote: > > On Tue, 31 May 2022 23:36:22 +0800 Chen Lin wrote: > > At 2022-05-31 22:14:12, "Jakub Kicinski" <kuba@xxxxxxxxxx> wrote: > > >On Tue, 31 May 2022 22:41:12 +0800 Chen Lin wrote: > > >> The sample code above cannot completely solve the current problem. > > >> For example, when fragsz is greater than PAGE_FRAG_CACHE_MAX_SIZE(32768), > > >> __page_frag_cache_refill will return a memory of only 32768 bytes, so > > >> should we continue to expand the PAGE_FRAG_CACHE_MAX_SIZE? Maybe more > > >> work needs to be done > > > > > >Right, but I can think of two drivers off the top of my head which will > > >allocate <=32k frags but none which will allocate more. > > > > In fact, it is rare to apply for more than one page, so is it necessary to > > change it to support? > > I don't really care if it's supported TBH, but I dislike adding > a branch to the fast path just to catch one or two esoteric bad > callers. > > Maybe you can wrap the check with some debug CONFIG_ so it won't > run on production builds? Also the example used here to define what is triggering the behavior is seriously flawed. The code itself is meant to allow for order0 page reuse, and the 32K page was just an optimization. So the assumption that you could request more than 4k is a bad assumption in the driver that is making this call. So I am in agreement with Kuba. We shouldn't be needing to add code in the fast path to tell users not to shoot themselves in the foot. We already have code in place in __netdev_alloc_skb that is calling the slab allocator if "len > SKB_WITH_OVERHEAD(PAGE_SIZE)". We could probably just add a DEBUG wrapped BUG_ON to capture those cases where a driver is making that mistake with __netdev_alloc_frag_align.
