From: Christophe JAILLET <christophe.jaillet@wanadoo.fr> To: Yury Norov <yury.norov@gmail.com>, Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: 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 22:41:27 +0200 [thread overview] Message-ID: <effcd39c-7f86-5f69-0663-4b81015cfc7c@wanadoo.fr> (raw) In-Reply-To: <YsHqRDfCQVwyA2m1@yury-laptop> 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 >
WARNING: multiple messages have this Message-ID (diff)
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr> To: Yury Norov <yury.norov@gmail.com>, 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, 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 22:41:27 +0200 [thread overview] Message-ID: <effcd39c-7f86-5f69-0663-4b81015cfc7c@wanadoo.fr> (raw) In-Reply-To: <YsHqRDfCQVwyA2m1@yury-laptop> 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 > -- 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 20:41 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 2022-07-03 19:13 ` [dm-devel] " Yury Norov 2022-07-03 20:41 ` Christophe JAILLET [this message] 2022-07-03 20:41 ` 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=effcd39c-7f86-5f69-0663-4b81015cfc7c@wanadoo.fr \ --to=christophe.jaillet@wanadoo.fr \ --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=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 \ --cc=yury.norov@gmail.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.