From: Yury Norov <yury.norov@gmail.com> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>, agk@redhat.com, snitzer@kernel.org, dm-devel@redhat.com, vneethv@linux.ibm.com, oberpar@linux.ibm.com, hca@linux.ibm.com, gor@linux.ibm.com, agordeev@linux.ibm.com, borntraeger@linux.ibm.com, svens@linux.ibm.com, almaz.alexandrovich@paragon-software.com, linux@rasmusvillemoes.dk, linux-s390@vger.kernel.org, ntfs3@lists.linux.dev, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [PATCH 3/4] bitmap: Introduce bitmap_size() Date: Sun, 3 Jul 2022 12:13:08 -0700 [thread overview] Message-ID: <YsHqRDfCQVwyA2m1@yury-laptop> (raw) In-Reply-To: <YsGz1Xp0RDM5ZhVY@smile.fi.intel.com> 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. Thanks, Yury
WARNING: multiple messages have this Message-ID (diff)
From: Yury Norov <yury.norov@gmail.com> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: linux-s390@vger.kernel.org, kernel-janitors@vger.kernel.org, ntfs3@lists.linux.dev, gor@linux.ibm.com, linux@rasmusvillemoes.dk, hca@linux.ibm.com, snitzer@kernel.org, oberpar@linux.ibm.com, linux-kernel@vger.kernel.org, almaz.alexandrovich@paragon-software.com, dm-devel@redhat.com, Christophe JAILLET <christophe.jaillet@wanadoo.fr>, svens@linux.ibm.com, vneethv@linux.ibm.com, agordeev@linux.ibm.com, borntraeger@linux.ibm.com, agk@redhat.com Subject: Re: [dm-devel] [PATCH 3/4] bitmap: Introduce bitmap_size() Date: Sun, 3 Jul 2022 12:13:08 -0700 [thread overview] Message-ID: <YsHqRDfCQVwyA2m1@yury-laptop> (raw) In-Reply-To: <YsGz1Xp0RDM5ZhVY@smile.fi.intel.com> 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. Thanks, Yury -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
next prev parent reply other threads:[~2022-07-03 19:13 UTC|newest] Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-07-02 18:28 [PATCH 0/4] Introduce bitmap_size() Christophe JAILLET 2022-07-02 18:28 ` [dm-devel] " Christophe JAILLET 2022-07-02 18:29 ` [PATCH 1/4] s390/cio: Rename bitmap_size() as idset_bitmap_size() Christophe JAILLET 2022-07-02 18:29 ` [dm-devel] " Christophe JAILLET 2022-07-02 18:54 ` Andy Shevchenko 2022-07-02 18:54 ` [dm-devel] " Andy Shevchenko 2022-07-02 19:24 ` Christophe JAILLET 2022-07-02 19:24 ` [dm-devel] " Christophe JAILLET 2022-07-02 19:24 ` Christophe JAILLET 2022-07-02 19:32 ` Andy Shevchenko 2022-07-02 19:32 ` [dm-devel] " Andy Shevchenko 2022-07-02 19:42 ` Christophe JAILLET 2022-07-02 19:42 ` [dm-devel] " Christophe JAILLET 2022-07-02 19:42 ` Christophe JAILLET 2022-07-02 19:42 ` Christophe JAILLET 2022-07-02 20:46 ` Yury Norov 2022-07-02 20:46 ` [dm-devel] " Yury Norov 2022-07-04 4:28 ` Vineeth Vijayan 2022-07-04 4:28 ` [dm-devel] " Vineeth Vijayan 2022-07-02 18:29 ` [PATCH 2/4] fs/ntfs3: Rename bitmap_size() as ntfs3_bitmap_size() Christophe JAILLET 2022-07-02 18:29 ` [dm-devel] " Christophe JAILLET 2022-07-02 18:58 ` Andy Shevchenko 2022-07-02 18:58 ` [dm-devel] " Andy Shevchenko 2022-07-02 19:37 ` Christophe JAILLET 2022-07-02 19:37 ` [dm-devel] " Christophe JAILLET 2022-07-02 19:37 ` Christophe JAILLET 2022-07-02 20:57 ` Yury Norov 2022-07-02 20:57 ` [dm-devel] " Yury Norov 2022-07-02 18:29 ` [PATCH 3/4] bitmap: Introduce bitmap_size() Christophe JAILLET 2022-07-02 18:29 ` [dm-devel] " Christophe JAILLET 2022-07-02 18:59 ` Andy Shevchenko 2022-07-02 18:59 ` [dm-devel] " Andy Shevchenko 2022-07-02 21:09 ` Yury Norov 2022-07-02 21:09 ` [dm-devel] " Yury Norov 2022-07-03 6:50 ` Christophe JAILLET 2022-07-03 6:50 ` [dm-devel] " Christophe JAILLET 2022-07-03 6:50 ` Christophe JAILLET 2022-07-03 15:20 ` Andy Shevchenko 2022-07-03 15:20 ` [dm-devel] " Andy Shevchenko 2022-07-03 19:13 ` Yury Norov [this message] 2022-07-03 19:13 ` Yury Norov 2022-07-03 20:41 ` Christophe JAILLET 2022-07-03 20:41 ` [dm-devel] " Christophe JAILLET 2022-07-02 18:29 ` [PATCH 4/4] bitmap: Use bitmap_size() Christophe JAILLET 2022-07-02 18:29 ` [dm-devel] " Christophe JAILLET 2022-07-02 20:44 ` [PATCH 0/4] Introduce bitmap_size() Yury Norov 2022-07-02 20:44 ` [dm-devel] " Yury Norov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YsHqRDfCQVwyA2m1@yury-laptop \ --to=yury.norov@gmail.com \ --cc=agk@redhat.com \ --cc=agordeev@linux.ibm.com \ --cc=almaz.alexandrovich@paragon-software.com \ --cc=andriy.shevchenko@linux.intel.com \ --cc=borntraeger@linux.ibm.com \ --cc=christophe.jaillet@wanadoo.fr \ --cc=dm-devel@redhat.com \ --cc=gor@linux.ibm.com \ --cc=hca@linux.ibm.com \ --cc=kernel-janitors@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-s390@vger.kernel.org \ --cc=linux@rasmusvillemoes.dk \ --cc=ntfs3@lists.linux.dev \ --cc=oberpar@linux.ibm.com \ --cc=snitzer@kernel.org \ --cc=svens@linux.ibm.com \ --cc=vneethv@linux.ibm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.