Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Thu, Dec 6, 2018 at 5:37 PM Vlastimil Babka wrote: > > On 12/6/18 4:49 AM, Nicolas Boichat wrote: > >> So it would be fine even unchanged. The check would anyway need some > >> more love to catch the same with __GFP_DMA to be consistent and cover > >> all corner cases. > > Yes, the test is not complete. If we really wanted this to be > > accurate, we'd need to check that GFP_* exactly matches SLAB_CACHE_*. > > > > The only problem with dropping this is test that we should restore > > GFP_DMA32 warning/errors somewhere else (as Christopher pointed out > > here: https://lkml.org/lkml/2018/11/22/430), especially for kmalloc > > case. > > I meant just dropping that patch hunk, not the whole test. Then the test > stays as it is and will keep warning anyone calling kmalloc(GFP_DMA32). > It would also warn anyone calling kmem_cache_alloc(GFP_DMA32) on > SLAB_CACHE_DMA32 cache, but since the gfp can be just dropped, and you > as the only user of this so far will do that, it's fine? I missed your point, this would work fine indeed. Thanks. > > Maybe this can be done in kmalloc_slab. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On 12/6/18 4:49 AM, Nicolas Boichat wrote: >> So it would be fine even unchanged. The check would anyway need some >> more love to catch the same with __GFP_DMA to be consistent and cover >> all corner cases. > Yes, the test is not complete. If we really wanted this to be > accurate, we'd need to check that GFP_* exactly matches SLAB_CACHE_*. > > The only problem with dropping this is test that we should restore > GFP_DMA32 warning/errors somewhere else (as Christopher pointed out > here: https://lkml.org/lkml/2018/11/22/430), especially for kmalloc > case. I meant just dropping that patch hunk, not the whole test. Then the test stays as it is and will keep warning anyone calling kmalloc(GFP_DMA32). It would also warn anyone calling kmem_cache_alloc(GFP_DMA32) on SLAB_CACHE_DMA32 cache, but since the gfp can be just dropped, and you as the only user of this so far will do that, it's fine? > Maybe this can be done in kmalloc_slab. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Thu, Dec 06, 2018 at 11:55:02AM +0800, Nicolas Boichat wrote:
>On Thu, Dec 6, 2018 at 11:32 AM Wei Yang wrote:
>>
>> On Thu, Dec 06, 2018 at 08:41:36AM +0800, Nicolas Boichat wrote:
>> >On Wed, Dec 5, 2018 at 8:18 PM Wei Yang wrote:
>> >>
>> >> On Wed, Dec 05, 2018 at 03:39:51PM +0800, Nicolas Boichat wrote:
>> >> >On Wed, Dec 5, 2018 at 3:25 PM Wei Yang
>> >> >wrote:
>> >> >>
>> >> >> On Wed, Dec 05, 2018 at 01:48:27PM +0800, Nicolas Boichat wrote:
>> >> >> >In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate
>> >> >> >data structures smaller than a page with GFP_DMA32 flag.
>> >> >> >
>> >> >> >This change makes it possible to create a custom cache in DMA32 zone
>> >> >> >using kmem_cache_create, then allocate memory using kmem_cache_alloc.
>> >> >> >
>> >> >> >We do not create a DMA32 kmalloc cache array, as there are currently
>> >> >> >no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags
>> >> >> >ensures that such calls still fail (as they do before this change).
>> >> >> >
>> >> >> >Fixes: ad67f5a6545f ("arm64: replace ZONE_DMA with ZONE_DMA32")
>> >> >> >Signed-off-by: Nicolas Boichat
>> >> >> >---
>> >> >> >
>> >> >> >Changes since v2:
>> >> >> > - Clarified commit message
>> >> >> > - Add entry in sysfs-kernel-slab to document the new sysfs file
>> >> >> >
>> >> >> >(v3 used the page_frag approach)
>> >> >> >
>> >> >> >Documentation/ABI/testing/sysfs-kernel-slab | 9 +
>> >> >> > include/linux/slab.h| 2 ++
>> >> >> > mm/internal.h | 8 ++--
>> >> >> > mm/slab.c | 4 +++-
>> >> >> > mm/slab.h | 3 ++-
>> >> >> > mm/slab_common.c| 2 +-
>> >> >> > mm/slub.c | 18 +-
>> >> >> > 7 files changed, 40 insertions(+), 6 deletions(-)
>> >> >> >
>> >> >> >diff --git a/Documentation/ABI/testing/sysfs-kernel-slab
>> >> >> >b/Documentation/ABI/testing/sysfs-kernel-slab
>> >> >> >index 29601d93a1c2ea..d742c6cfdffbe9 100644
>> >> >> >--- a/Documentation/ABI/testing/sysfs-kernel-slab
>> >> >> >+++ b/Documentation/ABI/testing/sysfs-kernel-slab
>> >> >> >@@ -106,6 +106,15 @@ Description:
>> >> >> > are from ZONE_DMA.
>> >> >> > Available when CONFIG_ZONE_DMA is enabled.
>> >> >> >
>> >> >> >+What: /sys/kernel/slab/cache/cache_dma32
>> >> >> >+Date: December 2018
>> >> >> >+KernelVersion:4.21
>> >> >> >+Contact: Nicolas Boichat
>> >> >> >+Description:
>> >> >> >+ The cache_dma32 file is read-only and specifies
>> >> >> >whether objects
>> >> >> >+ are from ZONE_DMA32.
>> >> >> >+ Available when CONFIG_ZONE_DMA32 is enabled.
>> >> >> >+
>> >> >> > What: /sys/kernel/slab/cache/cpu_slabs
>> >> >> > Date: May 2007
>> >> >> > KernelVersion:2.6.22
>> >> >> >diff --git a/include/linux/slab.h b/include/linux/slab.h
>> >> >> >index 11b45f7ae4057c..9449b19c5f107a 100644
>> >> >> >--- a/include/linux/slab.h
>> >> >> >+++ b/include/linux/slab.h
>> >> >> >@@ -32,6 +32,8 @@
>> >> >> > #define SLAB_HWCACHE_ALIGN((slab_flags_t __force)0x2000U)
>> >> >> > /* Use GFP_DMA memory */
>> >> >> > #define SLAB_CACHE_DMA((slab_flags_t
>> >> >> > __force)0x4000U)
>> >> >> >+/* Use GFP_DMA32 memory */
>> >> >> >+#define SLAB_CACHE_DMA32 ((slab_flags_t __force)0x8000U)
>> >> >> > /* DEBUG: Store the last owner for bug hunting */
>> >> >> > #define SLAB_STORE_USER ((slab_flags_t
>> >> >> > __force)0x0001U)
>> >> >> > /* Panic if kmem_cache_create() fails */
>> >> >> >diff --git a/mm/internal.h b/mm/internal.h
>> >> >> >index a2ee82a0cd44ae..fd244ad716eaf8 100644
>> >> >> >--- a/mm/internal.h
>> >> >> >+++ b/mm/internal.h
>> >> >> >@@ -14,6 +14,7 @@
>> >> >> > #include
>> >> >> > #include
>> >> >> > #include
>> >> >> >+#include
>> >> >> > #include
>> >> >> >
>> >> >> > /*
>> >> >> >@@ -34,9 +35,12 @@
>> >> >> > #define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
>> >> >> >
>> >> >> > /* Check for flags that must not be used with a slab allocator */
>> >> >> >-static inline gfp_t check_slab_flags(gfp_t flags)
>> >> >> >+static inline gfp_t check_slab_flags(gfp_t flags, slab_flags_t
>> >> >> >slab_flags)
>> >> >> > {
>> >> >> >- gfp_t bug_mask = __GFP_DMA32 | __GFP_HIGHMEM |
>> >> >> >~__GFP_BITS_MASK;
>> >> >> >+ gfp_t bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>> >> >> >+
>> >> >> >+ if (!IS_ENABLED(CONFIG_ZONE_DMA32) || !(slab_flags &
>> >> >> >SLAB_CACHE_DMA32))
>> >> >> >+ bug_mask |= __GFP_DMA32;
>> >> >>
>> >> >> The original version doesn't check CONFIG_ZONE_DMA32.
>> >> >>
>> >> >> Do we need to add this condition here?
>> >> >> Could we just decide the bug_mask based on slab_flags?
>> >> >
>> >> >We can. The reason I did it this way is that when we don't have
>> >>
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Thu, Dec 6, 2018 at 11:32 AM Wei Yang wrote:
>
> On Thu, Dec 06, 2018 at 08:41:36AM +0800, Nicolas Boichat wrote:
> >On Wed, Dec 5, 2018 at 8:18 PM Wei Yang wrote:
> >>
> >> On Wed, Dec 05, 2018 at 03:39:51PM +0800, Nicolas Boichat wrote:
> >> >On Wed, Dec 5, 2018 at 3:25 PM Wei Yang wrote:
> >> >>
> >> >> On Wed, Dec 05, 2018 at 01:48:27PM +0800, Nicolas Boichat wrote:
> >> >> >In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate
> >> >> >data structures smaller than a page with GFP_DMA32 flag.
> >> >> >
> >> >> >This change makes it possible to create a custom cache in DMA32 zone
> >> >> >using kmem_cache_create, then allocate memory using kmem_cache_alloc.
> >> >> >
> >> >> >We do not create a DMA32 kmalloc cache array, as there are currently
> >> >> >no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags
> >> >> >ensures that such calls still fail (as they do before this change).
> >> >> >
> >> >> >Fixes: ad67f5a6545f ("arm64: replace ZONE_DMA with ZONE_DMA32")
> >> >> >Signed-off-by: Nicolas Boichat
> >> >> >---
> >> >> >
> >> >> >Changes since v2:
> >> >> > - Clarified commit message
> >> >> > - Add entry in sysfs-kernel-slab to document the new sysfs file
> >> >> >
> >> >> >(v3 used the page_frag approach)
> >> >> >
> >> >> >Documentation/ABI/testing/sysfs-kernel-slab | 9 +
> >> >> > include/linux/slab.h| 2 ++
> >> >> > mm/internal.h | 8 ++--
> >> >> > mm/slab.c | 4 +++-
> >> >> > mm/slab.h | 3 ++-
> >> >> > mm/slab_common.c| 2 +-
> >> >> > mm/slub.c | 18 +-
> >> >> > 7 files changed, 40 insertions(+), 6 deletions(-)
> >> >> >
> >> >> >diff --git a/Documentation/ABI/testing/sysfs-kernel-slab
> >> >> >b/Documentation/ABI/testing/sysfs-kernel-slab
> >> >> >index 29601d93a1c2ea..d742c6cfdffbe9 100644
> >> >> >--- a/Documentation/ABI/testing/sysfs-kernel-slab
> >> >> >+++ b/Documentation/ABI/testing/sysfs-kernel-slab
> >> >> >@@ -106,6 +106,15 @@ Description:
> >> >> > are from ZONE_DMA.
> >> >> > Available when CONFIG_ZONE_DMA is enabled.
> >> >> >
> >> >> >+What: /sys/kernel/slab/cache/cache_dma32
> >> >> >+Date: December 2018
> >> >> >+KernelVersion:4.21
> >> >> >+Contact: Nicolas Boichat
> >> >> >+Description:
> >> >> >+ The cache_dma32 file is read-only and specifies whether
> >> >> >objects
> >> >> >+ are from ZONE_DMA32.
> >> >> >+ Available when CONFIG_ZONE_DMA32 is enabled.
> >> >> >+
> >> >> > What: /sys/kernel/slab/cache/cpu_slabs
> >> >> > Date: May 2007
> >> >> > KernelVersion:2.6.22
> >> >> >diff --git a/include/linux/slab.h b/include/linux/slab.h
> >> >> >index 11b45f7ae4057c..9449b19c5f107a 100644
> >> >> >--- a/include/linux/slab.h
> >> >> >+++ b/include/linux/slab.h
> >> >> >@@ -32,6 +32,8 @@
> >> >> > #define SLAB_HWCACHE_ALIGN((slab_flags_t __force)0x2000U)
> >> >> > /* Use GFP_DMA memory */
> >> >> > #define SLAB_CACHE_DMA((slab_flags_t
> >> >> > __force)0x4000U)
> >> >> >+/* Use GFP_DMA32 memory */
> >> >> >+#define SLAB_CACHE_DMA32 ((slab_flags_t __force)0x8000U)
> >> >> > /* DEBUG: Store the last owner for bug hunting */
> >> >> > #define SLAB_STORE_USER ((slab_flags_t
> >> >> > __force)0x0001U)
> >> >> > /* Panic if kmem_cache_create() fails */
> >> >> >diff --git a/mm/internal.h b/mm/internal.h
> >> >> >index a2ee82a0cd44ae..fd244ad716eaf8 100644
> >> >> >--- a/mm/internal.h
> >> >> >+++ b/mm/internal.h
> >> >> >@@ -14,6 +14,7 @@
> >> >> > #include
> >> >> > #include
> >> >> > #include
> >> >> >+#include
> >> >> > #include
> >> >> >
> >> >> > /*
> >> >> >@@ -34,9 +35,12 @@
> >> >> > #define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
> >> >> >
> >> >> > /* Check for flags that must not be used with a slab allocator */
> >> >> >-static inline gfp_t check_slab_flags(gfp_t flags)
> >> >> >+static inline gfp_t check_slab_flags(gfp_t flags, slab_flags_t
> >> >> >slab_flags)
> >> >> > {
> >> >> >- gfp_t bug_mask = __GFP_DMA32 | __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> >> >> >+ gfp_t bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> >> >> >+
> >> >> >+ if (!IS_ENABLED(CONFIG_ZONE_DMA32) || !(slab_flags &
> >> >> >SLAB_CACHE_DMA32))
> >> >> >+ bug_mask |= __GFP_DMA32;
> >> >>
> >> >> The original version doesn't check CONFIG_ZONE_DMA32.
> >> >>
> >> >> Do we need to add this condition here?
> >> >> Could we just decide the bug_mask based on slab_flags?
> >> >
> >> >We can. The reason I did it this way is that when we don't have
> >> >CONFIG_ZONE_DMA32, the compiler should be able to simplify to:
> >> >
> >> >bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> >> >if (true || ..) => if (true)
> >> > bug_mask |= __GFP_DMA32;
>
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Wed, Dec 5, 2018 at 10:02 PM Vlastimil Babka wrote:
>
> On 12/5/18 6:48 AM, Nicolas Boichat wrote:
> > In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate
> > data structures smaller than a page with GFP_DMA32 flag.
> >
> > This change makes it possible to create a custom cache in DMA32 zone
> > using kmem_cache_create, then allocate memory using kmem_cache_alloc.
> >
> > We do not create a DMA32 kmalloc cache array, as there are currently
> > no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags
> > ensures that such calls still fail (as they do before this change).
> >
> > Fixes: ad67f5a6545f ("arm64: replace ZONE_DMA with ZONE_DMA32")
>
> Same as my comment for 1/3.
I'll drop.
> > Signed-off-by: Nicolas Boichat
>
> In general,
> Acked-by: Vlastimil Babka
>
> Some comments below:
>
> > ---
> >
> > Changes since v2:
> > - Clarified commit message
> > - Add entry in sysfs-kernel-slab to document the new sysfs file
> >
> > (v3 used the page_frag approach)
> >
> > Documentation/ABI/testing/sysfs-kernel-slab | 9 +
> > include/linux/slab.h| 2 ++
> > mm/internal.h | 8 ++--
> > mm/slab.c | 4 +++-
> > mm/slab.h | 3 ++-
> > mm/slab_common.c| 2 +-
> > mm/slub.c | 18 +-
> > 7 files changed, 40 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-kernel-slab
> > b/Documentation/ABI/testing/sysfs-kernel-slab
> > index 29601d93a1c2ea..d742c6cfdffbe9 100644
> > --- a/Documentation/ABI/testing/sysfs-kernel-slab
> > +++ b/Documentation/ABI/testing/sysfs-kernel-slab
> > @@ -106,6 +106,15 @@ Description:
> > are from ZONE_DMA.
> > Available when CONFIG_ZONE_DMA is enabled.
> >
> > +What:/sys/kernel/slab/cache/cache_dma32
> > +Date:December 2018
> > +KernelVersion: 4.21
> > +Contact: Nicolas Boichat
> > +Description:
> > + The cache_dma32 file is read-only and specifies whether
> > objects
> > + are from ZONE_DMA32.
> > + Available when CONFIG_ZONE_DMA32 is enabled.
>
> I don't have a strong opinion. It's a new file, yeah, but consistent
> with already existing ones. I'd leave the decision with SL*B maintainers.
>
> > What:/sys/kernel/slab/cache/cpu_slabs
> > Date:May 2007
> > KernelVersion: 2.6.22
> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > index 11b45f7ae4057c..9449b19c5f107a 100644
> > --- a/include/linux/slab.h
> > +++ b/include/linux/slab.h
> > @@ -32,6 +32,8 @@
> > #define SLAB_HWCACHE_ALIGN ((slab_flags_t __force)0x2000U)
> > /* Use GFP_DMA memory */
> > #define SLAB_CACHE_DMA ((slab_flags_t __force)0x4000U)
> > +/* Use GFP_DMA32 memory */
> > +#define SLAB_CACHE_DMA32 ((slab_flags_t __force)0x8000U)
> > /* DEBUG: Store the last owner for bug hunting */
> > #define SLAB_STORE_USER ((slab_flags_t __force)0x0001U)
> > /* Panic if kmem_cache_create() fails */
> > diff --git a/mm/internal.h b/mm/internal.h
> > index a2ee82a0cd44ae..fd244ad716eaf8 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -14,6 +14,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > /*
> > @@ -34,9 +35,12 @@
> > #define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
> >
> > /* Check for flags that must not be used with a slab allocator */
> > -static inline gfp_t check_slab_flags(gfp_t flags)
> > +static inline gfp_t check_slab_flags(gfp_t flags, slab_flags_t slab_flags)
> > {
> > - gfp_t bug_mask = __GFP_DMA32 | __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> > + gfp_t bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> > +
> > + if (!IS_ENABLED(CONFIG_ZONE_DMA32) || !(slab_flags &
> > SLAB_CACHE_DMA32))
> > + bug_mask |= __GFP_DMA32;
>
> I'll point out that this is not even strictly needed AFAICS, as only
> flags passed to kmem_cache_alloc() are checked - the cache->allocflags
> derived from SLAB_CACHE_DMA32 are appended only after check_slab_flags()
> (in both SLAB and SLUB AFAICS). And for a cache created with
> SLAB_CACHE_DMA32, the caller of kmem_cache_alloc() doesn't need to also
> include __GFP_DMA32, the allocation will be from ZONE_DMA32 regardless.
Yes, you're right. I also looked at existing users of SLAB_CACHE_DMA,
and there is one case in drivers/scsi/scsi_lib.c where GFP_DMA is not
be passed (all the other users pass it).
I can drop GFP_DMA32 from my call in io-pgtable-arm-v7s.c.
> So it would be fine even unchanged. The check would anyway need some
> more love to catch the same with __GFP_DMA to be consistent and cover
> all corner cases.
Yes, the test is not complete. If we really wanted this to be
accurate, we'd need to check that GFP_* exactly
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Thu, Dec 06, 2018 at 08:41:36AM +0800, Nicolas Boichat wrote:
>On Wed, Dec 5, 2018 at 8:18 PM Wei Yang wrote:
>>
>> On Wed, Dec 05, 2018 at 03:39:51PM +0800, Nicolas Boichat wrote:
>> >On Wed, Dec 5, 2018 at 3:25 PM Wei Yang wrote:
>> >>
>> >> On Wed, Dec 05, 2018 at 01:48:27PM +0800, Nicolas Boichat wrote:
>> >> >In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate
>> >> >data structures smaller than a page with GFP_DMA32 flag.
>> >> >
>> >> >This change makes it possible to create a custom cache in DMA32 zone
>> >> >using kmem_cache_create, then allocate memory using kmem_cache_alloc.
>> >> >
>> >> >We do not create a DMA32 kmalloc cache array, as there are currently
>> >> >no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags
>> >> >ensures that such calls still fail (as they do before this change).
>> >> >
>> >> >Fixes: ad67f5a6545f ("arm64: replace ZONE_DMA with ZONE_DMA32")
>> >> >Signed-off-by: Nicolas Boichat
>> >> >---
>> >> >
>> >> >Changes since v2:
>> >> > - Clarified commit message
>> >> > - Add entry in sysfs-kernel-slab to document the new sysfs file
>> >> >
>> >> >(v3 used the page_frag approach)
>> >> >
>> >> >Documentation/ABI/testing/sysfs-kernel-slab | 9 +
>> >> > include/linux/slab.h| 2 ++
>> >> > mm/internal.h | 8 ++--
>> >> > mm/slab.c | 4 +++-
>> >> > mm/slab.h | 3 ++-
>> >> > mm/slab_common.c| 2 +-
>> >> > mm/slub.c | 18 +-
>> >> > 7 files changed, 40 insertions(+), 6 deletions(-)
>> >> >
>> >> >diff --git a/Documentation/ABI/testing/sysfs-kernel-slab
>> >> >b/Documentation/ABI/testing/sysfs-kernel-slab
>> >> >index 29601d93a1c2ea..d742c6cfdffbe9 100644
>> >> >--- a/Documentation/ABI/testing/sysfs-kernel-slab
>> >> >+++ b/Documentation/ABI/testing/sysfs-kernel-slab
>> >> >@@ -106,6 +106,15 @@ Description:
>> >> > are from ZONE_DMA.
>> >> > Available when CONFIG_ZONE_DMA is enabled.
>> >> >
>> >> >+What: /sys/kernel/slab/cache/cache_dma32
>> >> >+Date: December 2018
>> >> >+KernelVersion:4.21
>> >> >+Contact: Nicolas Boichat
>> >> >+Description:
>> >> >+ The cache_dma32 file is read-only and specifies whether
>> >> >objects
>> >> >+ are from ZONE_DMA32.
>> >> >+ Available when CONFIG_ZONE_DMA32 is enabled.
>> >> >+
>> >> > What: /sys/kernel/slab/cache/cpu_slabs
>> >> > Date: May 2007
>> >> > KernelVersion:2.6.22
>> >> >diff --git a/include/linux/slab.h b/include/linux/slab.h
>> >> >index 11b45f7ae4057c..9449b19c5f107a 100644
>> >> >--- a/include/linux/slab.h
>> >> >+++ b/include/linux/slab.h
>> >> >@@ -32,6 +32,8 @@
>> >> > #define SLAB_HWCACHE_ALIGN((slab_flags_t __force)0x2000U)
>> >> > /* Use GFP_DMA memory */
>> >> > #define SLAB_CACHE_DMA((slab_flags_t
>> >> > __force)0x4000U)
>> >> >+/* Use GFP_DMA32 memory */
>> >> >+#define SLAB_CACHE_DMA32 ((slab_flags_t __force)0x8000U)
>> >> > /* DEBUG: Store the last owner for bug hunting */
>> >> > #define SLAB_STORE_USER ((slab_flags_t
>> >> > __force)0x0001U)
>> >> > /* Panic if kmem_cache_create() fails */
>> >> >diff --git a/mm/internal.h b/mm/internal.h
>> >> >index a2ee82a0cd44ae..fd244ad716eaf8 100644
>> >> >--- a/mm/internal.h
>> >> >+++ b/mm/internal.h
>> >> >@@ -14,6 +14,7 @@
>> >> > #include
>> >> > #include
>> >> > #include
>> >> >+#include
>> >> > #include
>> >> >
>> >> > /*
>> >> >@@ -34,9 +35,12 @@
>> >> > #define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
>> >> >
>> >> > /* Check for flags that must not be used with a slab allocator */
>> >> >-static inline gfp_t check_slab_flags(gfp_t flags)
>> >> >+static inline gfp_t check_slab_flags(gfp_t flags, slab_flags_t
>> >> >slab_flags)
>> >> > {
>> >> >- gfp_t bug_mask = __GFP_DMA32 | __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>> >> >+ gfp_t bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>> >> >+
>> >> >+ if (!IS_ENABLED(CONFIG_ZONE_DMA32) || !(slab_flags &
>> >> >SLAB_CACHE_DMA32))
>> >> >+ bug_mask |= __GFP_DMA32;
>> >>
>> >> The original version doesn't check CONFIG_ZONE_DMA32.
>> >>
>> >> Do we need to add this condition here?
>> >> Could we just decide the bug_mask based on slab_flags?
>> >
>> >We can. The reason I did it this way is that when we don't have
>> >CONFIG_ZONE_DMA32, the compiler should be able to simplify to:
>> >
>> >bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>> >if (true || ..) => if (true)
>> > bug_mask |= __GFP_DMA32;
>> >
>> >Then just
>> >bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK | __GFP_DMA32;
>> >
>> >And since the function is inline, slab_flags would not even need to be
>> >accessed at all.
>> >
>>
>> Hmm, I get one confusion.
>>
>> This means if CONFIG_ZONE_DMA32 is not enabled,
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Wed, Dec 5, 2018 at 8:18 PM Wei Yang wrote:
>
> On Wed, Dec 05, 2018 at 03:39:51PM +0800, Nicolas Boichat wrote:
> >On Wed, Dec 5, 2018 at 3:25 PM Wei Yang wrote:
> >>
> >> On Wed, Dec 05, 2018 at 01:48:27PM +0800, Nicolas Boichat wrote:
> >> >In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate
> >> >data structures smaller than a page with GFP_DMA32 flag.
> >> >
> >> >This change makes it possible to create a custom cache in DMA32 zone
> >> >using kmem_cache_create, then allocate memory using kmem_cache_alloc.
> >> >
> >> >We do not create a DMA32 kmalloc cache array, as there are currently
> >> >no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags
> >> >ensures that such calls still fail (as they do before this change).
> >> >
> >> >Fixes: ad67f5a6545f ("arm64: replace ZONE_DMA with ZONE_DMA32")
> >> >Signed-off-by: Nicolas Boichat
> >> >---
> >> >
> >> >Changes since v2:
> >> > - Clarified commit message
> >> > - Add entry in sysfs-kernel-slab to document the new sysfs file
> >> >
> >> >(v3 used the page_frag approach)
> >> >
> >> >Documentation/ABI/testing/sysfs-kernel-slab | 9 +
> >> > include/linux/slab.h| 2 ++
> >> > mm/internal.h | 8 ++--
> >> > mm/slab.c | 4 +++-
> >> > mm/slab.h | 3 ++-
> >> > mm/slab_common.c| 2 +-
> >> > mm/slub.c | 18 +-
> >> > 7 files changed, 40 insertions(+), 6 deletions(-)
> >> >
> >> >diff --git a/Documentation/ABI/testing/sysfs-kernel-slab
> >> >b/Documentation/ABI/testing/sysfs-kernel-slab
> >> >index 29601d93a1c2ea..d742c6cfdffbe9 100644
> >> >--- a/Documentation/ABI/testing/sysfs-kernel-slab
> >> >+++ b/Documentation/ABI/testing/sysfs-kernel-slab
> >> >@@ -106,6 +106,15 @@ Description:
> >> > are from ZONE_DMA.
> >> > Available when CONFIG_ZONE_DMA is enabled.
> >> >
> >> >+What: /sys/kernel/slab/cache/cache_dma32
> >> >+Date: December 2018
> >> >+KernelVersion:4.21
> >> >+Contact: Nicolas Boichat
> >> >+Description:
> >> >+ The cache_dma32 file is read-only and specifies whether
> >> >objects
> >> >+ are from ZONE_DMA32.
> >> >+ Available when CONFIG_ZONE_DMA32 is enabled.
> >> >+
> >> > What: /sys/kernel/slab/cache/cpu_slabs
> >> > Date: May 2007
> >> > KernelVersion:2.6.22
> >> >diff --git a/include/linux/slab.h b/include/linux/slab.h
> >> >index 11b45f7ae4057c..9449b19c5f107a 100644
> >> >--- a/include/linux/slab.h
> >> >+++ b/include/linux/slab.h
> >> >@@ -32,6 +32,8 @@
> >> > #define SLAB_HWCACHE_ALIGN((slab_flags_t __force)0x2000U)
> >> > /* Use GFP_DMA memory */
> >> > #define SLAB_CACHE_DMA((slab_flags_t __force)0x4000U)
> >> >+/* Use GFP_DMA32 memory */
> >> >+#define SLAB_CACHE_DMA32 ((slab_flags_t __force)0x8000U)
> >> > /* DEBUG: Store the last owner for bug hunting */
> >> > #define SLAB_STORE_USER ((slab_flags_t __force)0x0001U)
> >> > /* Panic if kmem_cache_create() fails */
> >> >diff --git a/mm/internal.h b/mm/internal.h
> >> >index a2ee82a0cd44ae..fd244ad716eaf8 100644
> >> >--- a/mm/internal.h
> >> >+++ b/mm/internal.h
> >> >@@ -14,6 +14,7 @@
> >> > #include
> >> > #include
> >> > #include
> >> >+#include
> >> > #include
> >> >
> >> > /*
> >> >@@ -34,9 +35,12 @@
> >> > #define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
> >> >
> >> > /* Check for flags that must not be used with a slab allocator */
> >> >-static inline gfp_t check_slab_flags(gfp_t flags)
> >> >+static inline gfp_t check_slab_flags(gfp_t flags, slab_flags_t
> >> >slab_flags)
> >> > {
> >> >- gfp_t bug_mask = __GFP_DMA32 | __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> >> >+ gfp_t bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> >> >+
> >> >+ if (!IS_ENABLED(CONFIG_ZONE_DMA32) || !(slab_flags &
> >> >SLAB_CACHE_DMA32))
> >> >+ bug_mask |= __GFP_DMA32;
> >>
> >> The original version doesn't check CONFIG_ZONE_DMA32.
> >>
> >> Do we need to add this condition here?
> >> Could we just decide the bug_mask based on slab_flags?
> >
> >We can. The reason I did it this way is that when we don't have
> >CONFIG_ZONE_DMA32, the compiler should be able to simplify to:
> >
> >bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> >if (true || ..) => if (true)
> > bug_mask |= __GFP_DMA32;
> >
> >Then just
> >bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK | __GFP_DMA32;
> >
> >And since the function is inline, slab_flags would not even need to be
> >accessed at all.
> >
>
> Hmm, I get one confusion.
>
> This means if CONFIG_ZONE_DMA32 is not enabled, bug_mask will always
> contains __GFP_DMA32. This will check with cachep->flags.
>
> If cachep->flags has GFP_DMA32, this always fail?
>
> Is this possible?
Not fully sure to understand the question,
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On 12/5/18 6:48 AM, Nicolas Boichat wrote:
> In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate
> data structures smaller than a page with GFP_DMA32 flag.
>
> This change makes it possible to create a custom cache in DMA32 zone
> using kmem_cache_create, then allocate memory using kmem_cache_alloc.
>
> We do not create a DMA32 kmalloc cache array, as there are currently
> no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags
> ensures that such calls still fail (as they do before this change).
>
> Fixes: ad67f5a6545f ("arm64: replace ZONE_DMA with ZONE_DMA32")
Same as my comment for 1/3.
> Signed-off-by: Nicolas Boichat
In general,
Acked-by: Vlastimil Babka
Some comments below:
> ---
>
> Changes since v2:
> - Clarified commit message
> - Add entry in sysfs-kernel-slab to document the new sysfs file
>
> (v3 used the page_frag approach)
>
> Documentation/ABI/testing/sysfs-kernel-slab | 9 +
> include/linux/slab.h| 2 ++
> mm/internal.h | 8 ++--
> mm/slab.c | 4 +++-
> mm/slab.h | 3 ++-
> mm/slab_common.c| 2 +-
> mm/slub.c | 18 +-
> 7 files changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-kernel-slab
> b/Documentation/ABI/testing/sysfs-kernel-slab
> index 29601d93a1c2ea..d742c6cfdffbe9 100644
> --- a/Documentation/ABI/testing/sysfs-kernel-slab
> +++ b/Documentation/ABI/testing/sysfs-kernel-slab
> @@ -106,6 +106,15 @@ Description:
> are from ZONE_DMA.
> Available when CONFIG_ZONE_DMA is enabled.
>
> +What:/sys/kernel/slab/cache/cache_dma32
> +Date:December 2018
> +KernelVersion: 4.21
> +Contact: Nicolas Boichat
> +Description:
> + The cache_dma32 file is read-only and specifies whether objects
> + are from ZONE_DMA32.
> + Available when CONFIG_ZONE_DMA32 is enabled.
I don't have a strong opinion. It's a new file, yeah, but consistent
with already existing ones. I'd leave the decision with SL*B maintainers.
> What:/sys/kernel/slab/cache/cpu_slabs
> Date:May 2007
> KernelVersion: 2.6.22
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 11b45f7ae4057c..9449b19c5f107a 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -32,6 +32,8 @@
> #define SLAB_HWCACHE_ALIGN ((slab_flags_t __force)0x2000U)
> /* Use GFP_DMA memory */
> #define SLAB_CACHE_DMA ((slab_flags_t __force)0x4000U)
> +/* Use GFP_DMA32 memory */
> +#define SLAB_CACHE_DMA32 ((slab_flags_t __force)0x8000U)
> /* DEBUG: Store the last owner for bug hunting */
> #define SLAB_STORE_USER ((slab_flags_t __force)0x0001U)
> /* Panic if kmem_cache_create() fails */
> diff --git a/mm/internal.h b/mm/internal.h
> index a2ee82a0cd44ae..fd244ad716eaf8 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -14,6 +14,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> /*
> @@ -34,9 +35,12 @@
> #define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
>
> /* Check for flags that must not be used with a slab allocator */
> -static inline gfp_t check_slab_flags(gfp_t flags)
> +static inline gfp_t check_slab_flags(gfp_t flags, slab_flags_t slab_flags)
> {
> - gfp_t bug_mask = __GFP_DMA32 | __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> + gfp_t bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> +
> + if (!IS_ENABLED(CONFIG_ZONE_DMA32) || !(slab_flags & SLAB_CACHE_DMA32))
> + bug_mask |= __GFP_DMA32;
I'll point out that this is not even strictly needed AFAICS, as only
flags passed to kmem_cache_alloc() are checked - the cache->allocflags
derived from SLAB_CACHE_DMA32 are appended only after check_slab_flags()
(in both SLAB and SLUB AFAICS). And for a cache created with
SLAB_CACHE_DMA32, the caller of kmem_cache_alloc() doesn't need to also
include __GFP_DMA32, the allocation will be from ZONE_DMA32 regardless.
So it would be fine even unchanged. The check would anyway need some
more love to catch the same with __GFP_DMA to be consistent and cover
all corner cases.
>
> if (unlikely(flags & bug_mask)) {
> gfp_t invalid_mask = flags & bug_mask;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Wed, Dec 05, 2018 at 03:39:51PM +0800, Nicolas Boichat wrote:
>On Wed, Dec 5, 2018 at 3:25 PM Wei Yang wrote:
>>
>> On Wed, Dec 05, 2018 at 01:48:27PM +0800, Nicolas Boichat wrote:
>> >In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate
>> >data structures smaller than a page with GFP_DMA32 flag.
>> >
>> >This change makes it possible to create a custom cache in DMA32 zone
>> >using kmem_cache_create, then allocate memory using kmem_cache_alloc.
>> >
>> >We do not create a DMA32 kmalloc cache array, as there are currently
>> >no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags
>> >ensures that such calls still fail (as they do before this change).
>> >
>> >Fixes: ad67f5a6545f ("arm64: replace ZONE_DMA with ZONE_DMA32")
>> >Signed-off-by: Nicolas Boichat
>> >---
>> >
>> >Changes since v2:
>> > - Clarified commit message
>> > - Add entry in sysfs-kernel-slab to document the new sysfs file
>> >
>> >(v3 used the page_frag approach)
>> >
>> >Documentation/ABI/testing/sysfs-kernel-slab | 9 +
>> > include/linux/slab.h| 2 ++
>> > mm/internal.h | 8 ++--
>> > mm/slab.c | 4 +++-
>> > mm/slab.h | 3 ++-
>> > mm/slab_common.c| 2 +-
>> > mm/slub.c | 18 +-
>> > 7 files changed, 40 insertions(+), 6 deletions(-)
>> >
>> >diff --git a/Documentation/ABI/testing/sysfs-kernel-slab
>> >b/Documentation/ABI/testing/sysfs-kernel-slab
>> >index 29601d93a1c2ea..d742c6cfdffbe9 100644
>> >--- a/Documentation/ABI/testing/sysfs-kernel-slab
>> >+++ b/Documentation/ABI/testing/sysfs-kernel-slab
>> >@@ -106,6 +106,15 @@ Description:
>> > are from ZONE_DMA.
>> > Available when CONFIG_ZONE_DMA is enabled.
>> >
>> >+What: /sys/kernel/slab/cache/cache_dma32
>> >+Date: December 2018
>> >+KernelVersion:4.21
>> >+Contact: Nicolas Boichat
>> >+Description:
>> >+ The cache_dma32 file is read-only and specifies whether
>> >objects
>> >+ are from ZONE_DMA32.
>> >+ Available when CONFIG_ZONE_DMA32 is enabled.
>> >+
>> > What: /sys/kernel/slab/cache/cpu_slabs
>> > Date: May 2007
>> > KernelVersion:2.6.22
>> >diff --git a/include/linux/slab.h b/include/linux/slab.h
>> >index 11b45f7ae4057c..9449b19c5f107a 100644
>> >--- a/include/linux/slab.h
>> >+++ b/include/linux/slab.h
>> >@@ -32,6 +32,8 @@
>> > #define SLAB_HWCACHE_ALIGN((slab_flags_t __force)0x2000U)
>> > /* Use GFP_DMA memory */
>> > #define SLAB_CACHE_DMA((slab_flags_t __force)0x4000U)
>> >+/* Use GFP_DMA32 memory */
>> >+#define SLAB_CACHE_DMA32 ((slab_flags_t __force)0x8000U)
>> > /* DEBUG: Store the last owner for bug hunting */
>> > #define SLAB_STORE_USER ((slab_flags_t __force)0x0001U)
>> > /* Panic if kmem_cache_create() fails */
>> >diff --git a/mm/internal.h b/mm/internal.h
>> >index a2ee82a0cd44ae..fd244ad716eaf8 100644
>> >--- a/mm/internal.h
>> >+++ b/mm/internal.h
>> >@@ -14,6 +14,7 @@
>> > #include
>> > #include
>> > #include
>> >+#include
>> > #include
>> >
>> > /*
>> >@@ -34,9 +35,12 @@
>> > #define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
>> >
>> > /* Check for flags that must not be used with a slab allocator */
>> >-static inline gfp_t check_slab_flags(gfp_t flags)
>> >+static inline gfp_t check_slab_flags(gfp_t flags, slab_flags_t slab_flags)
>> > {
>> >- gfp_t bug_mask = __GFP_DMA32 | __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>> >+ gfp_t bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>> >+
>> >+ if (!IS_ENABLED(CONFIG_ZONE_DMA32) || !(slab_flags &
>> >SLAB_CACHE_DMA32))
>> >+ bug_mask |= __GFP_DMA32;
>>
>> The original version doesn't check CONFIG_ZONE_DMA32.
>>
>> Do we need to add this condition here?
>> Could we just decide the bug_mask based on slab_flags?
>
>We can. The reason I did it this way is that when we don't have
>CONFIG_ZONE_DMA32, the compiler should be able to simplify to:
>
>bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>if (true || ..) => if (true)
> bug_mask |= __GFP_DMA32;
>
>Then just
>bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK | __GFP_DMA32;
>
>And since the function is inline, slab_flags would not even need to be
>accessed at all.
>
Hmm, I get one confusion.
This means if CONFIG_ZONE_DMA32 is not enabled, bug_mask will always
contains __GFP_DMA32. This will check with cachep->flags.
If cachep->flags has GFP_DMA32, this always fail?
Is this possible?
--
Wei Yang
Help you, Help me
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Wed 05-12-18 19:01:03, Nicolas Boichat wrote: [...] > > Secondly, why do we need a new sysfs file? Who is going to consume it? > > We have cache_dma, so it seems consistent to add cache_dma32. I wouldn't copy a pattern unless there is an explicit usecase for it. We do expose way too much to userspace and that keeps kicking us later. Not that I am aware of any specific example for cache_dma but seeing other examples I would rather be more careful. > I wasn't aware of tools/vm/slabinfo.c, so I can add support for > cache_dma32 in a follow-up patch. Any other user I should take care > of? In general zones are inernal MM implementation details and the less we export to userspace the better. > > Then why do we need SLAB_MERGE_SAME to cover GFP_DMA32 as well? > > SLAB_MERGE_SAME tells us which flags _need_ to be the same for the > slabs to be merged. We don't want slab caches with GFP_DMA32 and > ~GFP_DMA32 to be merged, so it should be in there. > (https://elixir.bootlin.com/linux/v4.19.6/source/mm/slab_common.c#L342). Ohh, my bad, I have misread the change. Sure we definitely not want to allow merging here. My bad. -- Michal Hocko SUSE Labs ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Wed, Dec 5, 2018 at 5:56 PM Michal Hocko wrote: > > On Wed 05-12-18 13:48:27, Nicolas Boichat wrote: > > In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate > > data structures smaller than a page with GFP_DMA32 flag. > > > > This change makes it possible to create a custom cache in DMA32 zone > > using kmem_cache_create, then allocate memory using kmem_cache_alloc. > > > > We do not create a DMA32 kmalloc cache array, as there are currently > > no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags > > ensures that such calls still fail (as they do before this change). > > The changelog should be much more specific about decisions made here. > First of all it would be nice to mention the usecase. Ok, I'll copy most of the cover letter text here (i.e. the fact that IOMMU wants physical memory <4GB for L2 page tables, why it's better than genalloc/page_frag). > Secondly, why do we need a new sysfs file? Who is going to consume it? We have cache_dma, so it seems consistent to add cache_dma32. I wasn't aware of tools/vm/slabinfo.c, so I can add support for cache_dma32 in a follow-up patch. Any other user I should take care of? > Then why do we need SLAB_MERGE_SAME to cover GFP_DMA32 as well? SLAB_MERGE_SAME tells us which flags _need_ to be the same for the slabs to be merged. We don't want slab caches with GFP_DMA32 and ~GFP_DMA32 to be merged, so it should be in there. (https://elixir.bootlin.com/linux/v4.19.6/source/mm/slab_common.c#L342). > I > thought the whole point is to use dedicated slab cache. Who is this > going to merge with? Well, if there was another SLAB cache requiring 1KB GFP_DMA32 elements, then I don't see why we would not merge the caches. This is what happens with this IOMMU L2 tables cache pre-CONFIG_ZONE_DMA32 on arm64 (output on some 3.18 kernel below), and what would happen on arm32 since we still use GFP_DMA. /sys/kernel/slab # ls -l | grep dt-0001024 drwxr-xr-x. 2 root root 0 Dec 5 02:25 :dt-0001024 lrwxrwxrwx. 1 root root 0 Dec 5 02:25 dma-kmalloc-1024 -> :dt-0001024 lrwxrwxrwx. 1 root root 0 Dec 5 02:25 io-pgtable_armv7s_l2 -> :dt-0001024 Thanks! > -- > Michal Hocko > SUSE Labs ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Wed 05-12-18 13:48:27, Nicolas Boichat wrote: > In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate > data structures smaller than a page with GFP_DMA32 flag. > > This change makes it possible to create a custom cache in DMA32 zone > using kmem_cache_create, then allocate memory using kmem_cache_alloc. > > We do not create a DMA32 kmalloc cache array, as there are currently > no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags > ensures that such calls still fail (as they do before this change). The changelog should be much more specific about decisions made here. First of all it would be nice to mention the usecase. Secondly, why do we need a new sysfs file? Who is going to consume it? Then why do we need SLAB_MERGE_SAME to cover GFP_DMA32 as well? I thought the whole point is to use dedicated slab cache. Who is this going to merge with? -- Michal Hocko SUSE Labs ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Wed, Dec 05, 2018 at 03:39:51PM +0800, Nicolas Boichat wrote:
>On Wed, Dec 5, 2018 at 3:25 PM Wei Yang wrote:
>>
>> On Wed, Dec 05, 2018 at 01:48:27PM +0800, Nicolas Boichat wrote:
>> >In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate
>> >data structures smaller than a page with GFP_DMA32 flag.
>> >
>> >This change makes it possible to create a custom cache in DMA32 zone
>> >using kmem_cache_create, then allocate memory using kmem_cache_alloc.
>> >
>> >We do not create a DMA32 kmalloc cache array, as there are currently
>> >no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags
>> >ensures that such calls still fail (as they do before this change).
>> >
>> >Fixes: ad67f5a6545f ("arm64: replace ZONE_DMA with ZONE_DMA32")
>> >Signed-off-by: Nicolas Boichat
>> >---
>> >
>> >Changes since v2:
>> > - Clarified commit message
>> > - Add entry in sysfs-kernel-slab to document the new sysfs file
>> >
>> >(v3 used the page_frag approach)
>> >
>> >Documentation/ABI/testing/sysfs-kernel-slab | 9 +
>> > include/linux/slab.h| 2 ++
>> > mm/internal.h | 8 ++--
>> > mm/slab.c | 4 +++-
>> > mm/slab.h | 3 ++-
>> > mm/slab_common.c| 2 +-
>> > mm/slub.c | 18 +-
>> > 7 files changed, 40 insertions(+), 6 deletions(-)
>> >
>> >diff --git a/Documentation/ABI/testing/sysfs-kernel-slab
>> >b/Documentation/ABI/testing/sysfs-kernel-slab
>> >index 29601d93a1c2ea..d742c6cfdffbe9 100644
>> >--- a/Documentation/ABI/testing/sysfs-kernel-slab
>> >+++ b/Documentation/ABI/testing/sysfs-kernel-slab
>> >@@ -106,6 +106,15 @@ Description:
>> > are from ZONE_DMA.
>> > Available when CONFIG_ZONE_DMA is enabled.
>> >
>> >+What: /sys/kernel/slab/cache/cache_dma32
>> >+Date: December 2018
>> >+KernelVersion:4.21
>> >+Contact: Nicolas Boichat
>> >+Description:
>> >+ The cache_dma32 file is read-only and specifies whether
>> >objects
>> >+ are from ZONE_DMA32.
>> >+ Available when CONFIG_ZONE_DMA32 is enabled.
>> >+
>> > What: /sys/kernel/slab/cache/cpu_slabs
>> > Date: May 2007
>> > KernelVersion:2.6.22
>> >diff --git a/include/linux/slab.h b/include/linux/slab.h
>> >index 11b45f7ae4057c..9449b19c5f107a 100644
>> >--- a/include/linux/slab.h
>> >+++ b/include/linux/slab.h
>> >@@ -32,6 +32,8 @@
>> > #define SLAB_HWCACHE_ALIGN((slab_flags_t __force)0x2000U)
>> > /* Use GFP_DMA memory */
>> > #define SLAB_CACHE_DMA((slab_flags_t __force)0x4000U)
>> >+/* Use GFP_DMA32 memory */
>> >+#define SLAB_CACHE_DMA32 ((slab_flags_t __force)0x8000U)
>> > /* DEBUG: Store the last owner for bug hunting */
>> > #define SLAB_STORE_USER ((slab_flags_t __force)0x0001U)
>> > /* Panic if kmem_cache_create() fails */
>> >diff --git a/mm/internal.h b/mm/internal.h
>> >index a2ee82a0cd44ae..fd244ad716eaf8 100644
>> >--- a/mm/internal.h
>> >+++ b/mm/internal.h
>> >@@ -14,6 +14,7 @@
>> > #include
>> > #include
>> > #include
>> >+#include
>> > #include
>> >
>> > /*
>> >@@ -34,9 +35,12 @@
>> > #define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
>> >
>> > /* Check for flags that must not be used with a slab allocator */
>> >-static inline gfp_t check_slab_flags(gfp_t flags)
>> >+static inline gfp_t check_slab_flags(gfp_t flags, slab_flags_t slab_flags)
>> > {
>> >- gfp_t bug_mask = __GFP_DMA32 | __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>> >+ gfp_t bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>> >+
>> >+ if (!IS_ENABLED(CONFIG_ZONE_DMA32) || !(slab_flags &
>> >SLAB_CACHE_DMA32))
>> >+ bug_mask |= __GFP_DMA32;
>>
>> The original version doesn't check CONFIG_ZONE_DMA32.
>>
>> Do we need to add this condition here?
>> Could we just decide the bug_mask based on slab_flags?
>
>We can. The reason I did it this way is that when we don't have
>CONFIG_ZONE_DMA32, the compiler should be able to simplify to:
>
>bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>if (true || ..) => if (true)
> bug_mask |= __GFP_DMA32;
>
>Then just
>bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK | __GFP_DMA32;
>
>And since the function is inline, slab_flags would not even need to be
>accessed at all.
>
Thanks for explanation. This make sense to me.
--
Wei Yang
Help you, Help me
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Wed, Dec 5, 2018 at 3:25 PM Wei Yang wrote:
>
> On Wed, Dec 05, 2018 at 01:48:27PM +0800, Nicolas Boichat wrote:
> >In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate
> >data structures smaller than a page with GFP_DMA32 flag.
> >
> >This change makes it possible to create a custom cache in DMA32 zone
> >using kmem_cache_create, then allocate memory using kmem_cache_alloc.
> >
> >We do not create a DMA32 kmalloc cache array, as there are currently
> >no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags
> >ensures that such calls still fail (as they do before this change).
> >
> >Fixes: ad67f5a6545f ("arm64: replace ZONE_DMA with ZONE_DMA32")
> >Signed-off-by: Nicolas Boichat
> >---
> >
> >Changes since v2:
> > - Clarified commit message
> > - Add entry in sysfs-kernel-slab to document the new sysfs file
> >
> >(v3 used the page_frag approach)
> >
> >Documentation/ABI/testing/sysfs-kernel-slab | 9 +
> > include/linux/slab.h| 2 ++
> > mm/internal.h | 8 ++--
> > mm/slab.c | 4 +++-
> > mm/slab.h | 3 ++-
> > mm/slab_common.c| 2 +-
> > mm/slub.c | 18 +-
> > 7 files changed, 40 insertions(+), 6 deletions(-)
> >
> >diff --git a/Documentation/ABI/testing/sysfs-kernel-slab
> >b/Documentation/ABI/testing/sysfs-kernel-slab
> >index 29601d93a1c2ea..d742c6cfdffbe9 100644
> >--- a/Documentation/ABI/testing/sysfs-kernel-slab
> >+++ b/Documentation/ABI/testing/sysfs-kernel-slab
> >@@ -106,6 +106,15 @@ Description:
> > are from ZONE_DMA.
> > Available when CONFIG_ZONE_DMA is enabled.
> >
> >+What: /sys/kernel/slab/cache/cache_dma32
> >+Date: December 2018
> >+KernelVersion:4.21
> >+Contact: Nicolas Boichat
> >+Description:
> >+ The cache_dma32 file is read-only and specifies whether
> >objects
> >+ are from ZONE_DMA32.
> >+ Available when CONFIG_ZONE_DMA32 is enabled.
> >+
> > What: /sys/kernel/slab/cache/cpu_slabs
> > Date: May 2007
> > KernelVersion:2.6.22
> >diff --git a/include/linux/slab.h b/include/linux/slab.h
> >index 11b45f7ae4057c..9449b19c5f107a 100644
> >--- a/include/linux/slab.h
> >+++ b/include/linux/slab.h
> >@@ -32,6 +32,8 @@
> > #define SLAB_HWCACHE_ALIGN((slab_flags_t __force)0x2000U)
> > /* Use GFP_DMA memory */
> > #define SLAB_CACHE_DMA((slab_flags_t __force)0x4000U)
> >+/* Use GFP_DMA32 memory */
> >+#define SLAB_CACHE_DMA32 ((slab_flags_t __force)0x8000U)
> > /* DEBUG: Store the last owner for bug hunting */
> > #define SLAB_STORE_USER ((slab_flags_t __force)0x0001U)
> > /* Panic if kmem_cache_create() fails */
> >diff --git a/mm/internal.h b/mm/internal.h
> >index a2ee82a0cd44ae..fd244ad716eaf8 100644
> >--- a/mm/internal.h
> >+++ b/mm/internal.h
> >@@ -14,6 +14,7 @@
> > #include
> > #include
> > #include
> >+#include
> > #include
> >
> > /*
> >@@ -34,9 +35,12 @@
> > #define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
> >
> > /* Check for flags that must not be used with a slab allocator */
> >-static inline gfp_t check_slab_flags(gfp_t flags)
> >+static inline gfp_t check_slab_flags(gfp_t flags, slab_flags_t slab_flags)
> > {
> >- gfp_t bug_mask = __GFP_DMA32 | __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> >+ gfp_t bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
> >+
> >+ if (!IS_ENABLED(CONFIG_ZONE_DMA32) || !(slab_flags &
> >SLAB_CACHE_DMA32))
> >+ bug_mask |= __GFP_DMA32;
>
> The original version doesn't check CONFIG_ZONE_DMA32.
>
> Do we need to add this condition here?
> Could we just decide the bug_mask based on slab_flags?
We can. The reason I did it this way is that when we don't have
CONFIG_ZONE_DMA32, the compiler should be able to simplify to:
bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
if (true || ..) => if (true)
bug_mask |= __GFP_DMA32;
Then just
bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK | __GFP_DMA32;
And since the function is inline, slab_flags would not even need to be
accessed at all.
> >
> > if (unlikely(flags & bug_mask)) {
> > gfp_t invalid_mask = flags & bug_mask;
> >diff --git a/mm/slab.c b/mm/slab.c
> >index 65a774f05e7836..2fd3b9a996cbe6 100644
> >--- a/mm/slab.c
> >+++ b/mm/slab.c
> >@@ -2109,6 +2109,8 @@ int __kmem_cache_create(struct kmem_cache *cachep,
> >slab_flags_t flags)
> > cachep->allocflags = __GFP_COMP;
> > if (flags & SLAB_CACHE_DMA)
> > cachep->allocflags |= GFP_DMA;
> >+ if (flags & SLAB_CACHE_DMA32)
> >+ cachep->allocflags |= GFP_DMA32;
> > if (flags & SLAB_RECLAIM_ACCOUNT)
> > cachep->allocflags |= __GFP_RECLAIMABLE;
> > cachep->size = size;
> >@@ -2643,7 +2645,7 @@ static struct page
Re: [PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
On Wed, Dec 05, 2018 at 01:48:27PM +0800, Nicolas Boichat wrote:
>In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate
>data structures smaller than a page with GFP_DMA32 flag.
>
>This change makes it possible to create a custom cache in DMA32 zone
>using kmem_cache_create, then allocate memory using kmem_cache_alloc.
>
>We do not create a DMA32 kmalloc cache array, as there are currently
>no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags
>ensures that such calls still fail (as they do before this change).
>
>Fixes: ad67f5a6545f ("arm64: replace ZONE_DMA with ZONE_DMA32")
>Signed-off-by: Nicolas Boichat
>---
>
>Changes since v2:
> - Clarified commit message
> - Add entry in sysfs-kernel-slab to document the new sysfs file
>
>(v3 used the page_frag approach)
>
>Documentation/ABI/testing/sysfs-kernel-slab | 9 +
> include/linux/slab.h| 2 ++
> mm/internal.h | 8 ++--
> mm/slab.c | 4 +++-
> mm/slab.h | 3 ++-
> mm/slab_common.c| 2 +-
> mm/slub.c | 18 +-
> 7 files changed, 40 insertions(+), 6 deletions(-)
>
>diff --git a/Documentation/ABI/testing/sysfs-kernel-slab
>b/Documentation/ABI/testing/sysfs-kernel-slab
>index 29601d93a1c2ea..d742c6cfdffbe9 100644
>--- a/Documentation/ABI/testing/sysfs-kernel-slab
>+++ b/Documentation/ABI/testing/sysfs-kernel-slab
>@@ -106,6 +106,15 @@ Description:
> are from ZONE_DMA.
> Available when CONFIG_ZONE_DMA is enabled.
>
>+What: /sys/kernel/slab/cache/cache_dma32
>+Date: December 2018
>+KernelVersion:4.21
>+Contact: Nicolas Boichat
>+Description:
>+ The cache_dma32 file is read-only and specifies whether objects
>+ are from ZONE_DMA32.
>+ Available when CONFIG_ZONE_DMA32 is enabled.
>+
> What: /sys/kernel/slab/cache/cpu_slabs
> Date: May 2007
> KernelVersion:2.6.22
>diff --git a/include/linux/slab.h b/include/linux/slab.h
>index 11b45f7ae4057c..9449b19c5f107a 100644
>--- a/include/linux/slab.h
>+++ b/include/linux/slab.h
>@@ -32,6 +32,8 @@
> #define SLAB_HWCACHE_ALIGN((slab_flags_t __force)0x2000U)
> /* Use GFP_DMA memory */
> #define SLAB_CACHE_DMA((slab_flags_t __force)0x4000U)
>+/* Use GFP_DMA32 memory */
>+#define SLAB_CACHE_DMA32 ((slab_flags_t __force)0x8000U)
> /* DEBUG: Store the last owner for bug hunting */
> #define SLAB_STORE_USER ((slab_flags_t __force)0x0001U)
> /* Panic if kmem_cache_create() fails */
>diff --git a/mm/internal.h b/mm/internal.h
>index a2ee82a0cd44ae..fd244ad716eaf8 100644
>--- a/mm/internal.h
>+++ b/mm/internal.h
>@@ -14,6 +14,7 @@
> #include
> #include
> #include
>+#include
> #include
>
> /*
>@@ -34,9 +35,12 @@
> #define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
>
> /* Check for flags that must not be used with a slab allocator */
>-static inline gfp_t check_slab_flags(gfp_t flags)
>+static inline gfp_t check_slab_flags(gfp_t flags, slab_flags_t slab_flags)
> {
>- gfp_t bug_mask = __GFP_DMA32 | __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>+ gfp_t bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
>+
>+ if (!IS_ENABLED(CONFIG_ZONE_DMA32) || !(slab_flags & SLAB_CACHE_DMA32))
>+ bug_mask |= __GFP_DMA32;
The original version doesn't check CONFIG_ZONE_DMA32.
Do we need to add this condition here?
Could we just decide the bug_mask based on slab_flags?
>
> if (unlikely(flags & bug_mask)) {
> gfp_t invalid_mask = flags & bug_mask;
>diff --git a/mm/slab.c b/mm/slab.c
>index 65a774f05e7836..2fd3b9a996cbe6 100644
>--- a/mm/slab.c
>+++ b/mm/slab.c
>@@ -2109,6 +2109,8 @@ int __kmem_cache_create(struct kmem_cache *cachep,
>slab_flags_t flags)
> cachep->allocflags = __GFP_COMP;
> if (flags & SLAB_CACHE_DMA)
> cachep->allocflags |= GFP_DMA;
>+ if (flags & SLAB_CACHE_DMA32)
>+ cachep->allocflags |= GFP_DMA32;
> if (flags & SLAB_RECLAIM_ACCOUNT)
> cachep->allocflags |= __GFP_RECLAIMABLE;
> cachep->size = size;
>@@ -2643,7 +2645,7 @@ static struct page *cache_grow_begin(struct kmem_cache
>*cachep,
>* Be lazy and only check for valid flags here, keeping it out of the
>* critical path in kmem_cache_alloc().
>*/
>- flags = check_slab_flags(flags);
>+ flags = check_slab_flags(flags, cachep->flags);
> WARN_ON_ONCE(cachep->ctor && (flags & __GFP_ZERO));
> local_flags = flags & (GFP_CONSTRAINT_MASK|GFP_RECLAIM_MASK);
>
>diff --git a/mm/slab.h b/mm/slab.h
>index 4190c24ef0e9df..fcf717e12f0a86 100644
>--- a/mm/slab.h
>+++ b/mm/slab.h
>@@ -127,7 +127,8 @@ static inline slab_flags_t kmem_cache_flags(unsigned int
>object_size,
>
>
> /* Legal flag mask for kmem_cache_create(),
[PATCH v4 2/3] mm: Add support for kmem caches in DMA32 zone
In some cases (e.g. IOMMU ARMv7s page allocator), we need to allocate
data structures smaller than a page with GFP_DMA32 flag.
This change makes it possible to create a custom cache in DMA32 zone
using kmem_cache_create, then allocate memory using kmem_cache_alloc.
We do not create a DMA32 kmalloc cache array, as there are currently
no users of kmalloc(..., GFP_DMA32). The new test in check_slab_flags
ensures that such calls still fail (as they do before this change).
Fixes: ad67f5a6545f ("arm64: replace ZONE_DMA with ZONE_DMA32")
Signed-off-by: Nicolas Boichat
---
Changes since v2:
- Clarified commit message
- Add entry in sysfs-kernel-slab to document the new sysfs file
(v3 used the page_frag approach)
Documentation/ABI/testing/sysfs-kernel-slab | 9 +
include/linux/slab.h| 2 ++
mm/internal.h | 8 ++--
mm/slab.c | 4 +++-
mm/slab.h | 3 ++-
mm/slab_common.c| 2 +-
mm/slub.c | 18 +-
7 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-kernel-slab
b/Documentation/ABI/testing/sysfs-kernel-slab
index 29601d93a1c2ea..d742c6cfdffbe9 100644
--- a/Documentation/ABI/testing/sysfs-kernel-slab
+++ b/Documentation/ABI/testing/sysfs-kernel-slab
@@ -106,6 +106,15 @@ Description:
are from ZONE_DMA.
Available when CONFIG_ZONE_DMA is enabled.
+What: /sys/kernel/slab/cache/cache_dma32
+Date: December 2018
+KernelVersion: 4.21
+Contact: Nicolas Boichat
+Description:
+ The cache_dma32 file is read-only and specifies whether objects
+ are from ZONE_DMA32.
+ Available when CONFIG_ZONE_DMA32 is enabled.
+
What: /sys/kernel/slab/cache/cpu_slabs
Date: May 2007
KernelVersion: 2.6.22
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 11b45f7ae4057c..9449b19c5f107a 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -32,6 +32,8 @@
#define SLAB_HWCACHE_ALIGN ((slab_flags_t __force)0x2000U)
/* Use GFP_DMA memory */
#define SLAB_CACHE_DMA ((slab_flags_t __force)0x4000U)
+/* Use GFP_DMA32 memory */
+#define SLAB_CACHE_DMA32 ((slab_flags_t __force)0x8000U)
/* DEBUG: Store the last owner for bug hunting */
#define SLAB_STORE_USER((slab_flags_t __force)0x0001U)
/* Panic if kmem_cache_create() fails */
diff --git a/mm/internal.h b/mm/internal.h
index a2ee82a0cd44ae..fd244ad716eaf8 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
/*
@@ -34,9 +35,12 @@
#define GFP_CONSTRAINT_MASK (__GFP_HARDWALL|__GFP_THISNODE)
/* Check for flags that must not be used with a slab allocator */
-static inline gfp_t check_slab_flags(gfp_t flags)
+static inline gfp_t check_slab_flags(gfp_t flags, slab_flags_t slab_flags)
{
- gfp_t bug_mask = __GFP_DMA32 | __GFP_HIGHMEM | ~__GFP_BITS_MASK;
+ gfp_t bug_mask = __GFP_HIGHMEM | ~__GFP_BITS_MASK;
+
+ if (!IS_ENABLED(CONFIG_ZONE_DMA32) || !(slab_flags & SLAB_CACHE_DMA32))
+ bug_mask |= __GFP_DMA32;
if (unlikely(flags & bug_mask)) {
gfp_t invalid_mask = flags & bug_mask;
diff --git a/mm/slab.c b/mm/slab.c
index 65a774f05e7836..2fd3b9a996cbe6 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2109,6 +2109,8 @@ int __kmem_cache_create(struct kmem_cache *cachep,
slab_flags_t flags)
cachep->allocflags = __GFP_COMP;
if (flags & SLAB_CACHE_DMA)
cachep->allocflags |= GFP_DMA;
+ if (flags & SLAB_CACHE_DMA32)
+ cachep->allocflags |= GFP_DMA32;
if (flags & SLAB_RECLAIM_ACCOUNT)
cachep->allocflags |= __GFP_RECLAIMABLE;
cachep->size = size;
@@ -2643,7 +2645,7 @@ static struct page *cache_grow_begin(struct kmem_cache
*cachep,
* Be lazy and only check for valid flags here, keeping it out of the
* critical path in kmem_cache_alloc().
*/
- flags = check_slab_flags(flags);
+ flags = check_slab_flags(flags, cachep->flags);
WARN_ON_ONCE(cachep->ctor && (flags & __GFP_ZERO));
local_flags = flags & (GFP_CONSTRAINT_MASK|GFP_RECLAIM_MASK);
diff --git a/mm/slab.h b/mm/slab.h
index 4190c24ef0e9df..fcf717e12f0a86 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -127,7 +127,8 @@ static inline slab_flags_t kmem_cache_flags(unsigned int
object_size,
/* Legal flag mask for kmem_cache_create(), for various configurations */
-#define SLAB_CORE_FLAGS (SLAB_HWCACHE_ALIGN | SLAB_CACHE_DMA | SLAB_PANIC | \
+#define SLAB_CORE_FLAGS (SLAB_HWCACHE_ALIGN | SLAB_CACHE_DMA | \
+SLAB_CACHE_DMA32 | SLAB_PANIC | \
SLAB_TYPESAFE_BY_RCU | SLAB_DEBUG_OBJECTS )
#if
