| Message ID | 20190709083729.11135-1-peng.fan@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm: xen: mm: use __GPF_DMA32 for arm64 | expand |
Hi Russell, Stefano > Subject: [PATCH] arm: xen: mm: use __GPF_DMA32 for arm64 Any comments? > > arm64 shares some code under arch/arm/xen, including mm.c. > However ZONE_DMA is removed by commit > ad67f5a6545("arm64: replace ZONE_DMA with ZONE_DMA32"). > So to ARM64, need use __GFP_DMA32. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > arch/arm/xen/mm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index > e1d44b903dfc..a95e76d18bf9 100644 > --- a/arch/arm/xen/mm.c > +++ b/arch/arm/xen/mm.c > @@ -27,7 +27,7 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int > order) > > for_each_memblock(memory, reg) { > if (reg->base < (phys_addr_t)0xffffffff) { > - flags |= __GFP_DMA; > + flags |= __GFP_DMA | __GFP_DMA32; > break; > } > } Thanks, Peng. > -- > 2.16.4
Ping again.. +Julien > Subject: RE: [PATCH] arm: xen: mm: use __GPF_DMA32 for arm64 > > Hi Russell, Stefano > > > Subject: [PATCH] arm: xen: mm: use __GPF_DMA32 for arm64 > > Any comments? > > > > > arm64 shares some code under arch/arm/xen, including mm.c. > > However ZONE_DMA is removed by commit > > ad67f5a6545("arm64: replace ZONE_DMA with ZONE_DMA32"). > > So to ARM64, need use __GFP_DMA32. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > arch/arm/xen/mm.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index > > e1d44b903dfc..a95e76d18bf9 100644 > > --- a/arch/arm/xen/mm.c > > +++ b/arch/arm/xen/mm.c > > @@ -27,7 +27,7 @@ unsigned long xen_get_swiotlb_free_pages(unsigned > int > > order) > > > > for_each_memblock(memory, reg) { > > if (reg->base < (phys_addr_t)0xffffffff) { > > - flags |= __GFP_DMA; > > + flags |= __GFP_DMA | __GFP_DMA32; > > break; > > } > > } > > Thanks, > Peng. Thanks, Peng. > > > -- > > 2.16.4
You need to find someone who is interested in Xen on 32-bit ARM, and who knows this code - and therefore what impact your change causes. That isn't me, sorry. On Tue, Aug 27, 2019 at 09:27:53AM +0000, Peng Fan wrote: > Ping again.. > > +Julien > > > Subject: RE: [PATCH] arm: xen: mm: use __GPF_DMA32 for arm64 > > > > Hi Russell, Stefano > > > > > Subject: [PATCH] arm: xen: mm: use __GPF_DMA32 for arm64 > > > > Any comments? > > > > > > > > arm64 shares some code under arch/arm/xen, including mm.c. > > > However ZONE_DMA is removed by commit > > > ad67f5a6545("arm64: replace ZONE_DMA with ZONE_DMA32"). > > > So to ARM64, need use __GFP_DMA32. > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > --- > > > arch/arm/xen/mm.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index > > > e1d44b903dfc..a95e76d18bf9 100644 > > > --- a/arch/arm/xen/mm.c > > > +++ b/arch/arm/xen/mm.c > > > @@ -27,7 +27,7 @@ unsigned long xen_get_swiotlb_free_pages(unsigned > > int > > > order) > > > > > > for_each_memblock(memory, reg) { > > > if (reg->base < (phys_addr_t)0xffffffff) { > > > - flags |= __GFP_DMA; > > > + flags |= __GFP_DMA | __GFP_DMA32; > > > break; > > > } > > > } > > > > Thanks, > > Peng. > > Thanks, > Peng. > > > > > > -- > > > 2.16.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On 09/07/2019 09:22, Peng Fan wrote: > arm64 shares some code under arch/arm/xen, including mm.c. > However ZONE_DMA is removed by commit > ad67f5a6545("arm64: replace ZONE_DMA with ZONE_DMA32"). > So to ARM64, need use __GFP_DMA32. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > arch/arm/xen/mm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c > index e1d44b903dfc..a95e76d18bf9 100644 > --- a/arch/arm/xen/mm.c > +++ b/arch/arm/xen/mm.c > @@ -27,7 +27,7 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int order) > > for_each_memblock(memory, reg) { > if (reg->base < (phys_addr_t)0xffffffff) { > - flags |= __GFP_DMA; > + flags |= __GFP_DMA | __GFP_DMA32; Given the definition of GFP_ZONE_BAD, I'm not sure this combination of flags is strictly valid, but rather is implicitly reliant on only one of those zones ever actually existing. As such, it seems liable to blow up if the plans to add ZONE_DMA to arm64[1] go ahead. Robin. [1] https://lore.kernel.org/linux-arm-kernel/20190820145821.27214-1-nsaenzjulienne@suse.de/ > break; > } > } >
Hi Robin, > Subject: Re: [PATCH] arm: xen: mm: use __GPF_DMA32 for arm64 > > On 09/07/2019 09:22, Peng Fan wrote: > > arm64 shares some code under arch/arm/xen, including mm.c. > > However ZONE_DMA is removed by commit > > ad67f5a6545("arm64: replace ZONE_DMA with ZONE_DMA32"). > > So to ARM64, need use __GFP_DMA32. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > arch/arm/xen/mm.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index > > e1d44b903dfc..a95e76d18bf9 100644 > > --- a/arch/arm/xen/mm.c > > +++ b/arch/arm/xen/mm.c > > @@ -27,7 +27,7 @@ unsigned long xen_get_swiotlb_free_pages(unsigned > > int order) > > > > for_each_memblock(memory, reg) { > > if (reg->base < (phys_addr_t)0xffffffff) { > > - flags |= __GFP_DMA; > > + flags |= __GFP_DMA | __GFP_DMA32; > > Given the definition of GFP_ZONE_BAD, I'm not sure this combination of flags > is strictly valid, but rather is implicitly reliant on only one of those zones ever > actually existing. As such, it seems liable to blow up if the plans to add > ZONE_DMA to arm64[1] go ahead. How about this, or do you have any suggestions? diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index d33b77e9add3..f61c29a4430f 100644 --- a/arch/arm/xen/mm.c +++ b/arch/arm/xen/mm.c @@ -28,7 +28,11 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int order) for_each_memblock(memory, reg) { if (reg->base < (phys_addr_t)0xffffffff) { +#ifdef CONFIG_ARM64 + flags |= __GFP_DMA32; +#else flags |= __GFP_DMA; +#endif break; } } Thanks, Peng. > > Robin. > > [1] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke > rnel.org%2Flinux-arm-kernel%2F20190820145821.27214-1-nsaenzjulienne% > 40suse.de%2F&data=02%7C01%7Cpeng.fan%40nxp.com%7C5d2a641b1 > e3f449562f908d72ae95d85%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 > %7C0%7C637025054169859035&sdata=1ZPGH0gZnvgmlMpX7VrjUNME > XbEjiv4%2FZ9pYwTQTWxY%3D&reserved=0 > > > break; > > } > > } > >
On Wed, 28 Aug 2019, Peng Fan wrote: > Hi Robin, > > > Subject: Re: [PATCH] arm: xen: mm: use __GPF_DMA32 for arm64 > > > > On 09/07/2019 09:22, Peng Fan wrote: > > > arm64 shares some code under arch/arm/xen, including mm.c. > > > However ZONE_DMA is removed by commit > > > ad67f5a6545("arm64: replace ZONE_DMA with ZONE_DMA32"). > > > So to ARM64, need use __GFP_DMA32. Hi Peng, Sorry for being so late in replying, this email got lost in the noise. > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > --- > > > arch/arm/xen/mm.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index > > > e1d44b903dfc..a95e76d18bf9 100644 > > > --- a/arch/arm/xen/mm.c > > > +++ b/arch/arm/xen/mm.c > > > @@ -27,7 +27,7 @@ unsigned long xen_get_swiotlb_free_pages(unsigned > > > int order) > > > > > > for_each_memblock(memory, reg) { > > > if (reg->base < (phys_addr_t)0xffffffff) { > > > - flags |= __GFP_DMA; > > > + flags |= __GFP_DMA | __GFP_DMA32; > > > > Given the definition of GFP_ZONE_BAD, I'm not sure this combination of flags > > is strictly valid, but rather is implicitly reliant on only one of those zones ever > > actually existing. As such, it seems liable to blow up if the plans to add > > ZONE_DMA to arm64[1] go ahead. > > How about this, or do you have any suggestions? > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c > index d33b77e9add3..f61c29a4430f 100644 > --- a/arch/arm/xen/mm.c > +++ b/arch/arm/xen/mm.c > @@ -28,7 +28,11 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int order) > > for_each_memblock(memory, reg) { > if (reg->base < (phys_addr_t)0xffffffff) { > +#ifdef CONFIG_ARM64 > + flags |= __GFP_DMA32; > +#else > flags |= __GFP_DMA; > +#endif > break; > } > } Yes I think this is the way to go, but we are trying not to add any #ifdef CONFIG_ARM64 under arch/arm. Maybe you could introduce a static inline function to set GFP_DMA: static inline void xen_set_gfp_dma(gfp_t *flags) it could be implemented under arch/arm/include/asm/xen/page.h for arm and under arch/arm64/include/asm/xen/page.h for arm64 using __GFP_DMA for the former and __GFP_DMA32 for the latter.
Hi Stefano, > Subject: RE: [PATCH] arm: xen: mm: use __GPF_DMA32 for arm64 > > On Wed, 28 Aug 2019, Peng Fan wrote: > > Hi Robin, > > > > > Subject: Re: [PATCH] arm: xen: mm: use __GPF_DMA32 for arm64 > > > > > > On 09/07/2019 09:22, Peng Fan wrote: > > > > arm64 shares some code under arch/arm/xen, including mm.c. > > > > However ZONE_DMA is removed by commit > > > > ad67f5a6545("arm64: replace ZONE_DMA with ZONE_DMA32"). > > > > So to ARM64, need use __GFP_DMA32. > > Hi Peng, > > Sorry for being so late in replying, this email got lost in the noise. > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > > --- > > > > arch/arm/xen/mm.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index > > > > e1d44b903dfc..a95e76d18bf9 100644 > > > > --- a/arch/arm/xen/mm.c > > > > +++ b/arch/arm/xen/mm.c > > > > @@ -27,7 +27,7 @@ unsigned long > > > > xen_get_swiotlb_free_pages(unsigned > > > > int order) > > > > > > > > for_each_memblock(memory, reg) { > > > > if (reg->base < (phys_addr_t)0xffffffff) { > > > > - flags |= __GFP_DMA; > > > > + flags |= __GFP_DMA | __GFP_DMA32; > > > > > > Given the definition of GFP_ZONE_BAD, I'm not sure this combination > > > of flags is strictly valid, but rather is implicitly reliant on only > > > one of those zones ever actually existing. As such, it seems liable > > > to blow up if the plans to add ZONE_DMA to arm64[1] go ahead. > > > > How about this, or do you have any suggestions? > > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index > > d33b77e9add3..f61c29a4430f 100644 > > --- a/arch/arm/xen/mm.c > > +++ b/arch/arm/xen/mm.c > > @@ -28,7 +28,11 @@ unsigned long xen_get_swiotlb_free_pages(unsigned > > int order) > > > > for_each_memblock(memory, reg) { > > if (reg->base < (phys_addr_t)0xffffffff) { > > +#ifdef CONFIG_ARM64 > > + flags |= __GFP_DMA32; #else > > flags |= __GFP_DMA; > > +#endif > > break; > > } > > } > > Yes I think this is the way to go, but we are trying not to add any #ifdef > CONFIG_ARM64 under arch/arm. Maybe you could introduce a static inline > function to set GFP_DMA: > > static inline void xen_set_gfp_dma(gfp_t *flags) > > it could be implemented under arch/arm/include/asm/xen/page.h for arm > and under arch/arm64/include/asm/xen/page.h for arm64 using __GFP_DMA > for the former and __GFP_DMA32 for the latter. Thanks for your suggestion. I'll use this in V2. Thanks, Peng.
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index e1d44b903dfc..a95e76d18bf9 100644 --- a/arch/arm/xen/mm.c +++ b/arch/arm/xen/mm.c @@ -27,7 +27,7 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int order) for_each_memblock(memory, reg) { if (reg->base < (phys_addr_t)0xffffffff) { - flags |= __GFP_DMA; + flags |= __GFP_DMA | __GFP_DMA32; break; } }
arm64 shares some code under arch/arm/xen, including mm.c. However ZONE_DMA is removed by commit ad67f5a6545("arm64: replace ZONE_DMA with ZONE_DMA32"). So to ARM64, need use __GFP_DMA32. Signed-off-by: Peng Fan <peng.fan@nxp.com> --- arch/arm/xen/mm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)