Re: [PATCH v2] iio: adc: ti-ads7138: replace kmalloc() with stack allocation in i2c_write_block

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 28 Apr 2026 10:36:17 +0100
David Laight <david.laight.linux@gmail.com> wrote:

> 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().

That is a problem for some buses that define their basic bus access functions
to require DMA (and regmap on top of those).  E.g. SPI where your comment
would have been absolutely correct.

I2C does the opposite. Everything is bounced through DMA safe buffers unless
you explicitly opt in to say you are providing a DMA safe buffer.
That is only worth doing if the buffers are large and the bounce therefore
expensive.

Whilst we are here - something that has me doubting myself so I'll just
lay it out.

There is a fun corner where Sashiko is giving a false positive (I think
anyway!) in that a pair of rx/tx buffers can sit in the same cacheline
as long as no one modifies either whilst a bus transaction is in flight
and so the DMA hasn't yet occurred in both directions. This has the fun
side effect of part of the cacheline in the non coherent DMA management
code being marked for each direction. However, the mechanism for corruption
doesn't apply so it should be fine.  A common situation is where rx and
tx are the same buffer which is an extreme case of this and absolutely fine.

If I have this wrong we have a lot of broken drivers - so hopefully not!

> One of the solutions to that is to use kmalloc() for buffers that
> could be on stack.

Correct, just not needed here.

Jonathan

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