All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: David Lechner <dlechner@baylibre.com>
Cc: Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com>,
	antoniu.miclaus@analog.com, lars@metafoo.de,
	Michael.Hennerich@analog.com, jic23@kernel.org,
	nuno.sa@analog.com, andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Andy Shevchenko <andriy.shevchenko@intel.com>
Subject: Re: [PATCH v2] iio: adc: ti-ads7138: replace kmalloc() with stack allocation in i2c_write_block
Date: Tue, 28 Apr 2026 10:36:17 +0100	[thread overview]
Message-ID: <20260428103617.148027c1@pumpkin> (raw)
In-Reply-To: <3512cce0-6660-42a9-91bb-07e78a002205@baylibre.com>

On Mon, 27 Apr 2026 10:21:37 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 4/27/26 8:31 AM, David Laight wrote:
> > On Mon, 27 Apr 2026 15:27:07 +0400
> > Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com> wrote:
> >   
> >> The ads7138_i2c_write_block() function currently utilizes kmalloc()
> >> to allocate a buffer for I2C transfers. However, the length
> >> parameter passed to this function is strictly 2 bytes across all
> >> driver invocations, making the total payload buffer size exactly 4 bytes.
> >> Invoking the heap allocator for a 4-byte buffer introduces
> >> unnecessary SLUB overhead.   
> > 
> > Have you confirmed that the buffer is never used for DMA?
> > 
> > Provided the lock that blocks concurrent access from two threads
> > is actually outside this code, a buffer for short transfers could
> > be allocated within 'struct i2c_client'.
> > 
> > 	David
> >   
> Giorgi,
> 
> This is why it is important to include a changelog and a link to the
> previous discussion [1] in the cover letter or after --- in a single patch
> when you submit a new revision. I think that would have answered David's
> question.
> 
> [1]: https://lore.kernel.org/linux-iio/20260424081809.61841-2-giorgitchankvetadze1997@gmail.com/
> 
> 

The thing is I remember issues with on-stack buffers being used for
dma - which worked before kernel stacks were allocated using vmalloc().
One of the solutions to that is to use kmalloc() for buffers that
could be on stack.

I think I found exactly 1 call to this function.
Indeed, just inlining the logic would make everything clearer.

	David

  parent reply	other threads:[~2026-04-28  9:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-27 11:27 [PATCH v2] iio: adc: ti-ads7138: replace kmalloc() with stack allocation in i2c_write_block Giorgi Tchankvetadze
2026-04-27 13:31 ` David Laight
2026-04-27 15:21   ` David Lechner
2026-04-28  6:08     ` Giorgi Tchankvetadze
2026-04-28  9:36     ` David Laight [this message]
2026-04-28 17:03       ` Jonathan Cameron
2026-04-27 14:56 ` Andy Shevchenko

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=20260428103617.148027c1@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=antoniu.miclaus@analog.com \
    --cc=dlechner@baylibre.com \
    --cc=giorgitchankvetadze1997@gmail.com \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.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: link
Be 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.