On Fri, Sep 28, 2018 at 10:06:48AM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2018-09-27 at 15:49 +0200, Christoph Hellwig wrote: > > On Thu, Sep 27, 2018 at 11:45:15AM +1000, Benjamin Herrenschmidt wrote: > > > I'm not sure this is entirely right. > > > > > > Let's say the mask is 30 bits. You will return GFP_DMA32, which will > > > fail if you allocate something above 1G (which is legit for > > > ZONE_DMA32). > > > > And then we will try GFP_DMA further down in the function: > > > > if (IS_ENABLED(CONFIG_ZONE_DMA) && > > dev->coherent_dma_mask < DMA_BIT_MASK(32) && > > !(gfp & GFP_DMA)) { > > gfp = (gfp & ~GFP_DMA32) | GFP_DMA; > > goto again; > > } > > > > This is and old optimization from x86, because chances are high that > > GFP_DMA32 will give you suitable memory for the infamous 31-bit > > dma mask devices (at least at boot time) and thus we don't have > > to deplete the tiny ZONE_DMA pool. > > I see, it's rather confusing :-) Wouldn't it be better to check against > top of 32-bit memory instead here too ?
Where is here? In __dma_direct_optimal_gfp_mask we already handled it due to the optimistic zone selection we are discussing. In the fallback quoted above there is no point for it, as with a physical memory size smaller than ZONE_DMA32 (or ZONE_DMA for that matter) we will have succeeded with the optimistic zone selection and not hit the fallback path. Either way this code probably needs much better comments. I'll send a patch on top of the recent series.