Le 03/07/2022 à 21:13, Yury Norov a écrit :
On Sun, Jul 03, 2022 at 06:20:53PM +0300, Andy Shevchenko wrote:
On Sun, Jul 03, 2022 at 08:50:19AM +0200, Christophe JAILLET wrote:
Le 02/07/2022 à 23:09, Yury Norov a écrit :
On Sat, Jul 02, 2022 at 08:29:36PM +0200, Christophe JAILLET wrote:
...
This should be dropped, for sure, and kmalloc() at line 128 should be
replaced with bitmap_alloc().
This kmalloc() is for a structure and a flexible array.
You mean re-arranging the code to allocate the structure alone at first,
then the bitmap?
We can change struct primes to:
struct primes {
struct rcu_head rcu;
unsigned long last, sz;
unsigned long *primes;
};
And then either allocate twice:
new = kmalloc(sizeof(struct primes);
new->primes = bitmap_alloc(sz);
Or keep the same struct primes for all expansions, and just allocate
new bitmap for ->primes when needed. This is what I meant.
This a bit deeper rework, but it addresses Andy's concern about excessive
fragmentation. (Did anyone before complain? Is it measurable?)
It's one way, but it will increase fragmentation of memory. The other one
as it seems to me is to name a new API properly, i.e. bitmap_size_to_bytes().
In such case you won't need renames to begin with. And then would be able
to convert driver-by-driver in cases of duplicated code.
I think that's what confused Yuri and I kinda agree that bitmap_size() should
return bits, and not bytes. Also argument for pure bitmap_size() would be
bitmap itself, but we have no way to detect the length of bitmap because we
are using POD and not a specific data structure for it.
bitmap_size_to_bytes() sounds better. How many places in the kernel
do we have where we can't simply use bitmap_alloc(), and need this
machinery? If this is the only one, I'd prefer to switch it to
bitmap_alloc() instead.
I'll spot some places that would require a bitmap_size_to_bytes().
This way, we'll have some more information to decide if:
- bitmap_size_to_bytes() makes sense or not
- other helper functions are better suited
- these places need some rework to use the existing API
CJ
Thanks,
Yury
