Re: [PATCH v2] usb: core: warn if a GFP zone flag is passed to hcd_buffer_alloc()

[Petr Tesarik <ptesarik@suse.com>]

On Tue, 15 Apr 2025 09:53:24 +0200
Oliver Neukum <oneukum@suse.com> wrote:

> On 14.04.25 09:02, Petr Tesarik wrote:
> 
> Hi,
>   
> > That's the point. AFAICS there are _no_ in-tree callers that would pass
> > GFP_DMA or GFP_DMA32 to hcd_buffer_alloc(), directly or indirectly. But
> > nobody should be tempted to add the flag, because I cannot imagine how
> > that would ever be the right thing to do.  
> 
> You do not dream about putting USB onto PCMCIA over Thunderbolt?

Oh, I do, and that's precisely why these GFP flags are no good. The
address (and other) constraints imposed by different buses may not
(and often do not) match any existing memory zone.

However, zone address ranges are determined statically at compile time,
or latest at boot time (e.g. arm64). It's too late to adjust the limits
when you hotplug a more constrained bus at run-time. And I haven't even
mentioned bus bridges which add a non-zero offset to the address...

> > I can change it back to mem_flags &= ~GFP_ZONEMASK to fix it silently;
> > I simply thought that driver authors may appreciate a warning that
> > they're trying to do something silly.  
> 
> People rarely appreciate warnings. I think we should limit them
> to cases where something goes wrong or something unexpected happens.

I'm certainly no expert on what is expected to happen if you include
GFP_DMA in your HCD buffer allocation flags, but the current code will
*ignore* it, unless the HCD uses PIO.

I thought this was rather unexpected.

> > Whatever works for you, but please keep in mind that there seems to be
> > agreement among mm people that DMA and DMA32 zones should be removed
> > from the kernel eventually.  
> 
> Well, if somebody finds a legitimate use case for these flags, the mm
> people should deal with it. They are likelier to find a good solution than
> all driver writers being forced into finding individual solutions.

My goal is to provide an allocator that is a better match for the
constraints defined by dma_mask, coherent_dma_mask and bus_dma_limit.
For now, I'm trying to clean up some users of GFP_DMA and GFP_DMA32
flags which look obviously incorrect.

Petr T