Re: Bulk kmalloc

Re: Bulk kmalloc

[Jesper Dangaard Brouer]

On Thu, 09 May 2019 16:28:58 +0100 David Howells <dhowells@redhat.com> wrote:

> Is it possible to use kmem_cache_alloc_bulk() with kmalloc slabs to
> effect a bulk kmalloc?

Well, we have kfree_bulk() which is a simple wrapper around
kmem_cache_free_bulk() (as Christoph make me handle that case).

We/I didn't code the kmalloc_bulk() variant.

What is you use case?

(p.s. fixed the MM-email address)
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


static __always_inline void kfree_bulk(size_t size, void **p)
{
	kmem_cache_free_bulk(NULL, size, p);
}


Handling code for kfree_bulk case:

	page = virt_to_head_page(object);
	if (!s) {
		/* Handle kalloc'ed objects */
		if (unlikely(!PageSlab(page))) {
			BUG_ON(!PageCompound(page));
			kfree_hook(object);
			__free_pages(page, compound_order(page));
			p[size] = NULL; /* mark object processed */
			return size;
		}
		/* Derive kmem_cache from object */
		df->s = page->slab_cache;
	} else {
		df->s = cache_from_obj(s, object); /* Support for memcg */
	}

Re: Bulk kmalloc

[David Howells]

Jesper Dangaard Brouer <brouer@redhat.com> wrote:

> > Is it possible to use kmem_cache_alloc_bulk() with kmalloc slabs to
> > effect a bulk kmalloc?
> 
> Well, we have kfree_bulk() which is a simple wrapper around
> kmem_cache_free_bulk() (as Christoph make me handle that case).
> 
> We/I didn't code the kmalloc_bulk() variant.
> 
> What is you use case?

afs_do_lookup() allocates an array of file status records and an array of
callback records:

	/* Need space for examining all the selected files */
	inode = ERR_PTR(-ENOMEM);
	cookie->statuses = kcalloc(cookie->nr_fids, sizeof(struct afs_file_status),
				   GFP_KERNEL);
	if (!cookie->statuses)
		goto out;

	cookie->callbacks = kcalloc(cookie->nr_fids, sizeof(struct afs_callback),
				    GFP_KERNEL);
	if (!cookie->callbacks)
		goto out_s;

These, however, may go to order-1 allocations or higher if nr_fids > 39, say,
and it may be as many as 50 for AFS3 or 1024 for YFS.

Also, I'd like to combine the afs_file_status record with the afs_callback
record inside another struct so that I can pass these around in more places
and fix the locking over applying them to the relevant inodes.

So what I want to do is to allocate an array of pointers to {status,callback}
records and then bulk allocate those records.  As it happens, the tuple is
just shy of 128 bytes, so they should fit into that slab very nicely.

Note also that the records are transient - they're freed at the end of the
operation.

David

Re: Bulk kmalloc

[Matthew Wilcox]

On Fri, May 10, 2019 at 05:23:23PM +0100, David Howells wrote:
> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> > What is you use case?
> 
> afs_do_lookup() allocates an array of file status records and an array of
> callback records:
> 
> 	/* Need space for examining all the selected files */
> 	inode = ERR_PTR(-ENOMEM);
> 	cookie->statuses = kcalloc(cookie->nr_fids, sizeof(struct afs_file_status),
> 				   GFP_KERNEL);
> 	if (!cookie->statuses)
> 		goto out;
> 
> 	cookie->callbacks = kcalloc(cookie->nr_fids, sizeof(struct afs_callback),
> 				    GFP_KERNEL);
> 	if (!cookie->callbacks)
> 		goto out_s;
> 
> These, however, may go to order-1 allocations or higher if nr_fids > 39, say,
> and it may be as many as 50 for AFS3 or 1024 for YFS.

kvmalloc() is the normal solution here.  Usual reasons for not being
able to do that would be that you do DMA to the memory or that you need
to be able to free each of these objects individually.

> Also, I'd like to combine the afs_file_status record with the afs_callback
> record inside another struct so that I can pass these around in more places
> and fix the locking over applying them to the relevant inodes.
> 
> So what I want to do is to allocate an array of pointers to {status,callback}
> records and then bulk allocate those records.  As it happens, the tuple is
> just shy of 128 bytes, so they should fit into that slab very nicely.
> 
> Note also that the records are transient - they're freed at the end of the
> operation.

sounds like you free them all together?

Re: Bulk kmalloc

[Christoph Hellwig]

On Fri, May 10, 2019 at 09:50:01AM -0700, Matthew Wilcox wrote:
> kvmalloc() is the normal solution here.  Usual reasons for not being
> able to do that would be that you do DMA to the memory or that you need
> to be able to free each of these objects individually.

Note that you absolutely can do DMA to vmalloced buffers.  It just is
very painful due to the manual coherency management..

Re: Bulk kmalloc

[Matthew Wilcox]

On Fri, May 10, 2019 at 10:11:10AM -0700, Christoph Hellwig wrote:
> On Fri, May 10, 2019 at 09:50:01AM -0700, Matthew Wilcox wrote:
> > kvmalloc() is the normal solution here.  Usual reasons for not being
> > able to do that would be that you do DMA to the memory or that you need
> > to be able to free each of these objects individually.
> 
> Note that you absolutely can do DMA to vmalloced buffers.  It just is
> very painful due to the manual coherency management..

... and we don't have proper APIs for it.  The last time we discussed
it was January, I think.

https://lore.kernel.org/patchwork/patch/1033921/

I haven't made any effort to try to come up with decent APIs for this.

Re: Bulk kmalloc

[David Howells]

Matthew Wilcox <willy@infradead.org> wrote:

> kvmalloc() is the normal solution here.  Usual reasons for not being
> able to do that would be that you do DMA to the memory or that you need
> to be able to free each of these objects individually.

I don't need to DMA to the memory - but it might be worth my while removing
status, cb_expires_at, cb_version and cb_type from struct afs_vnode and
replacing them with an RCU-managed pointer to an afs_status_cb struct.

I could then use plain RCU rather than a seqlock to manage reading from the
record (the problem being that updating these records cannot be done
atomically).  That would allow me to make afs_d_revalidate() more efficient by
doing the checks first in LOOKUP_RCU mode.

I'm not sure it's worth the extra RCU load, though.

David