Messages in this thread |  | | Date | Sun, 3 Jul 2022 22:41:27 +0200 | Subject | Re: [PATCH 3/4] bitmap: Introduce bitmap_size() | From | Christophe JAILLET <> |
| |
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 >
|  |