On Tue, Apr 10, 2018 at 06:53:04AM -0700, Eric Dumazet wrote:
> On 04/10/2018 05:53 AM, Matthew Wilcox wrote:
> > From: Matthew Wilcox <mawil...@microsoft.com>
> > 
> > __GFP_ZERO requests that the object be initialised to all-zeroes,
> > while the purpose of a constructor is to initialise an object to a
> > particular pattern.  We cannot do both.  Add a warning to catch any
> > users who mistakenly pass a __GFP_ZERO flag when allocating a slab with
> > a constructor.
> > 
> > Fixes: d07dbea46405 ("Slab allocators: support __GFP_ZERO in all 
> > allocators")
> > Signed-off-by: Matthew Wilcox <mawil...@microsoft.com>
> > Cc: sta...@vger.kernel.org
> Since there are probably no bug to fix, what about adding the extra check
> only for some DEBUG option ?
> How many caches are still using ctor these days ?

That's a really good question, and strangely hard to find out.  I settled
on "git grep -A4 kmem_cache_alloc" and then searching the 'less' output
with '[^L]);'.

arch/powerpc/kvm/book3s_64_mmu_radix.c: kvm_pte_cache = 
kmem_cache_create("kvm-pte", size, size, 0, pte_ctor);
arch/powerpc/mm/init-common.c:  new = kmem_cache_create(name, table_size, 
align, 0, ctor);
arch/powerpc/platforms/cell/spufs/inode.c:      spufs_inode_cache = 
arch/powerpc/platforms/cell/spufs/inode.c-                      sizeof(struct 
spufs_inode_info), 0,
arch/sh/mm/pgtable.c:   pgd_cachep = kmem_cache_create("pgd_cache",
arch/sh/mm/pgtable.c-                                  PTRS_PER_PGD * 
arch/sh/mm/pgtable.c-                                  PAGE_SIZE, SLAB_PANIC, 
arch/sparc/mm/tsb.c:    pgtable_cache = kmem_cache_create("pgtable_cache",
arch/sparc/mm/tsb.c-                                      PAGE_SIZE, PAGE_SIZE,
arch/sparc/mm/tsb.c-                                      0,
arch/sparc/mm/tsb.c-                                      _clear_page);
drivers/dax/super.c:    dax_cache = kmem_cache_create("dax_cache", sizeof(struct
 dax_device), 0,
drivers/dax/super.c-                    (SLAB_HWCACHE_ALIGN|SLAB_RECLAIM_ACCOUNT
drivers/dax/super.c-                     SLAB_MEM_SPREAD|SLAB_ACCOUNT),
drivers/dax/super.c-                    init_once);
drivers/staging/ncpfs/inode.c:  ncp_inode_cachep = kmem_cache_create("ncp_inode_
drivers/staging/ncpfs/inode.c-                                       sizeof(stru
ct ncp_inode_info),
drivers/staging/ncpfs/inode.c-                                       0, (SLAB_RE
drivers/staging/ncpfs/inode.c-                                          SLAB_MEM
drivers/staging/ncpfs/inode.c-                                       init_once);
drivers/usb/mon/mon_text.c:     rp->e_slab = kmem_cache_create(rp->slab_name,
drivers/usb/mon/mon_text.c-         sizeof(struct mon_event_text), sizeof(long),
drivers/usb/mon/mon_text.c-         mon_text_ctor);
fs/9p/v9fs.c:   v9fs_inode_cache = kmem_cache_create("v9fs_inode_cache",
fs/9p/v9fs.c-                                     sizeof(struct v9fs_inode),
fs/9p/v9fs.c-                                     0, (SLAB_RECLAIM_ACCOUNT|
fs/9p/v9fs.c-                                     v9fs_inode_init_once);
fs/adfs/super.c:        adfs_inode_cachep = 
fs/adfs/super.c-                                             sizeof(struct 
fs/adfs/super.c-                                             0, 
fs/adfs/super.c-                                             init_once);
... snip a huge number of filesystems ...
ipc/mqueue.c:   mqueue_inode_cachep = kmem_cache_create("mqueue_inode_cache",
ipc/mqueue.c-                           sizeof(struct mqueue_inode_info), 0,
ipc/mqueue.c-                           SLAB_HWCACHE_ALIGN|SLAB_ACCOUNT, 
kernel/fork.c:  sighand_cachep = kmem_cache_create("sighand_cache",
kernel/fork.c-                  sizeof(struct sighand_struct), 0,
kernel/fork.c-                  SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_TYPESAFE_BY_R
kernel/fork.c-                  SLAB_ACCOUNT, sighand_ctor);
lib/radix-tree.c:       radix_tree_node_cachep = kmem_cache_create("radix_tree_n
lib/radix-tree.c-                       sizeof(struct radix_tree_node), 0,
lib/radix-tree.c-                       SLAB_PANIC | SLAB_RECLAIM_ACCOUNT,
lib/radix-tree.c-                       radix_tree_node_ctor);
mm/rmap.c:      anon_vma_cachep = kmem_cache_create("anon_vma", sizeof(struct an
mm/rmap.c-                      0, SLAB_TYPESAFE_BY_RCU|SLAB_PANIC|SLAB_ACCOUNT,
mm/rmap.c-                      anon_vma_ctor);
mm/shmem.c:     shmem_inode_cachep = kmem_cache_create("shmem_inode_cache",
mm/shmem.c-                             sizeof(struct shmem_inode_info),
mm/shmem.c-                             0, SLAB_PANIC|SLAB_ACCOUNT, 
net/sunrpc/rpc_pipe.c:  rpc_inode_cachep = kmem_cache_create("rpc_inode_cache",
net/sunrpc/rpc_pipe.c-                          sizeof(struct rpc_inode),
net/sunrpc/rpc_pipe.c-                          0, (SLAB_HWCACHE_ALIGN|SLAB_RECL
net/sunrpc/rpc_pipe.c-                                          SLAB_MEM_SPREAD|
net/sunrpc/rpc_pipe.c-                          init_once);
security/integrity/iint.c:          kmem_cache_create("iint_cache", sizeof(struc
t integrity_iint_cache),
security/integrity/iint.c-                            0, SLAB_PANIC, init_once);

So aside from the filesystems, about fourteen places use it in the kernel.

If we want to get rid of the concept of constructors, it's doable,
but somebody needs to do the work to show what the effects will be.

For example, I took a quick look at the sighand_struct in kernel/fork.c.
That initialises the spinlock and waitqueue head which are at the end
of sighand_struct.  The caller who allocates sighand_struct touches
the head of the struct.  So if we removed the ctor, we'd touch two
cachelines on allocation instead of one ... but we could rearrange the
sighand_struct to put all the initialised bits in the first cacheline
(and we probably should).

Reply via email to