| Message ID | 20201102152037.963-1-brgl@bgdev.pl (mailing list archive) |
|---|---|
| Headers | show |
| Series | slab: provide and use krealloc_array() | expand |
On Mon, Nov 02, 2020 at 04:20:30PM +0100, Bartosz Golaszewski wrote: > +Chunks allocated with `kmalloc` can be resized with `krealloc`. Similarly > +to `kmalloc_array`: a helper for resising arrays is provided in the form of > +`krealloc_array`. Is there any reason you chose to `do_this` instead of do_this()? The automarkup script turns do_this() into a nice link to the documentation which you're adding below. Typo 'resising' resizing.
On Mon, Nov 2, 2020 at 4:41 PM Matthew Wilcox <willy@infradead.org> wrote: > > On Mon, Nov 02, 2020 at 04:20:30PM +0100, Bartosz Golaszewski wrote: > > +Chunks allocated with `kmalloc` can be resized with `krealloc`. Similarly > > +to `kmalloc_array`: a helper for resising arrays is provided in the form of > > +`krealloc_array`. > > Is there any reason you chose to `do_this` instead of do_this()? The > automarkup script turns do_this() into a nice link to the documentation > which you're adding below. > No, I just didn't know better. Thanks for bringing this to my attention. > Typo 'resising' resizing. Will fix in the next iteration. Bartosz
On Mon, Nov 02, 2020 at 04:20:37PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Use the helper that checks for overflows internally instead of manually > calculating the size of the new array. ... > + nfences = krealloc_array(fences, i, > + sizeof(*fences), GFP_KERNEL); On 80 position is closing parenthesis, which, I think, makes it okay to put on one line.
On Mon, 2020-11-02 at 16:20 +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Andy brought to my attention the fact that users allocating an array of > equally sized elements should check if the size multiplication doesn't > overflow. This is why we have helpers like kmalloc_array(). > > However we don't have krealloc_array() equivalent and there are many > users who do their own multiplication when calling krealloc() for arrays. > > This series provides krealloc_array() and uses it in a couple places. My concern about this is a possible assumption that __GFP_ZERO will work, and as far as I know, it will not.
On Tue, Nov 3, 2020 at 5:14 AM Joe Perches <joe@perches.com> wrote: > > On Mon, 2020-11-02 at 16:20 +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > Andy brought to my attention the fact that users allocating an array of > > equally sized elements should check if the size multiplication doesn't > > overflow. This is why we have helpers like kmalloc_array(). > > > > However we don't have krealloc_array() equivalent and there are many > > users who do their own multiplication when calling krealloc() for arrays. > > > > This series provides krealloc_array() and uses it in a couple places. > > My concern about this is a possible assumption that __GFP_ZERO will > work, and as far as I know, it will not. > Yeah so I had this concern for devm_krealloc() and even sent a patch that extended it to honor __GFP_ZERO before I noticed that regular krealloc() silently ignores __GFP_ZERO. I'm not sure if this is on purpose. Maybe we should either make krealloc() honor __GFP_ZERO or explicitly state in its documentation that it ignores it? This concern isn't really related to this patch as such - it's more of a general krealloc() inconsistency. Bartosz
On Tue, Nov 3, 2020 at 12:13 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > On Tue, Nov 3, 2020 at 5:14 AM Joe Perches <joe@perches.com> wrote: > > On Mon, 2020-11-02 at 16:20 +0100, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > Yeah so I had this concern for devm_krealloc() and even sent a patch > that extended it to honor __GFP_ZERO before I noticed that regular > krealloc() silently ignores __GFP_ZERO. I'm not sure if this is on > purpose. Maybe we should either make krealloc() honor __GFP_ZERO or > explicitly state in its documentation that it ignores it? And my voice here is to ignore for the same reasons: respect realloc(3) and making common sense with the idea of REallocating (capital letters on purpose).
From: Bartosz Golaszewski <bgolaszewski@baylibre.com> Andy brought to my attention the fact that users allocating an array of equally sized elements should check if the size multiplication doesn't overflow. This is why we have helpers like kmalloc_array(). However we don't have krealloc_array() equivalent and there are many users who do their own multiplication when calling krealloc() for arrays. This series provides krealloc_array() and uses it in a couple places. A separate series will follow adding devm_krealloc_array() which is needed in the xilinx adc driver. v1 -> v2: - added a kernel doc for krealloc_array() - mentioned krealloc et al in the docs - collected review tags Bartosz Golaszewski (8): mm: slab: provide krealloc_array() ALSA: pcm: use krealloc_array() vhost: vringh: use krealloc_array() pinctrl: use krealloc_array() edac: ghes: use krealloc_array() drm: atomic: use krealloc_array() hwtracing: intel: use krealloc_array() dma-buf: use krealloc_array() Documentation/core-api/memory-allocation.rst | 4 ++++ drivers/dma-buf/sync_file.c | 4 ++-- drivers/edac/ghes_edac.c | 4 ++-- drivers/gpu/drm/drm_atomic.c | 3 ++- drivers/hwtracing/intel_th/msu.c | 2 +- drivers/pinctrl/pinctrl-utils.c | 2 +- drivers/vhost/vringh.c | 3 ++- include/linux/slab.h | 18 ++++++++++++++++++ sound/core/pcm_lib.c | 4 ++-- 9 files changed, 34 insertions(+), 10 deletions(-)