Re: [PATCH 2/2] kfree_rcu() should use kfree_bulk() interface

From: Rao Shoaib
Date: Wed Apr 04 2018 - 03:18:21 EST




On 04/03/2018 07:23 PM, Matthew Wilcox wrote:
On Tue, Apr 03, 2018 at 05:55:55PM -0700, Rao Shoaib wrote:
On 04/03/2018 01:58 PM, Matthew Wilcox wrote:
I think you might be better off with an IDR. The IDR can always
contain one entry, so there's no need for this 'rbf_list_head' or
__rcu_bulk_schedule_list. The IDR contains its first 64 entries in
an array (if that array can be allocated), so it's compatible with the
kfree_bulk() interface.

I have just familiarized myself with what IDR is by reading your article. If
I am incorrect please correct me.

The list and head you have pointed are only used if the container can not
be allocated. That could happen with IDR as well. Note that the containers
are allocated at boot time and are re-used.
No, it can't happen with the IDR. The IDR can always contain one entry
without allocating anything. If you fail to allocate the second entry,
just free the first entry.

IDR seems to have some overhead, such as I have to specifically add the
pointer and free the ID, plus radix tree maintenance.
... what? Adding a pointer is simply idr_alloc(), and you get back an
integer telling you which index it has. Your data structure has its
own set of overhead.
The only overhead is a pointer that points to the head and an int to keep count. If I use idr, I would have to allocate an struct idr which is much larger. idr_alloc()/idr_destroy() operations are much more costly than updating two pointers. As the pointers are stored in slots/nodes corresponding to the id, I would have to retrieve the pointers by calling idr_remove() to pass them to be freed, the slots/nodes would constantly be allocated and freed.

IDR is a very useful interface for allocating/managing ID's but I really do not see the justification for using it over here, perhaps you can elaborate more on the benefits and also on how I can just pass the array to be freed.

Shoaib