public inbox for linux-iio@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: buffer: hw-consumer: free scan_mask on buffer release
@ 2026-04-27 11:11 Felix Gu
  2026-04-27 15:19 ` Andy Shevchenko
  2026-04-28 17:28 ` Jonathan Cameron
  0 siblings, 2 replies; 5+ messages in thread
From: Felix Gu @ 2026-04-27 11:11 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko
  Cc: linux-iio, linux-kernel, Jonathan Cameron, Felix Gu

The scan_mask lifetime changed in commit 9a2e1233d38c ("iio: buffer:
hw-consumer: remove redundant scan_mask flexible array").

Before that change, the scan mask storage was embedded in struct
hw_consumer_buffer, so iio_hw_buf_release() could free the whole
allocation with a single kfree(hw_buf).

That commit moved the scan mask to a separate bitmap_zalloc() allocation
stored in buffer.scan_mask, but left iio_hw_buf_release() unchanged.

Free the scan mask in iio_hw_buf_release() before freeing the buffer
wrapper.

Fixes: 9a2e1233d38c ("iio: buffer: hw-consumer: remove redundant scan_mask flexible array")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/iio/buffer/industrialio-hw-consumer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/buffer/industrialio-hw-consumer.c b/drivers/iio/buffer/industrialio-hw-consumer.c
index 24d7df603760..d1ab2cbf8c88 100644
--- a/drivers/iio/buffer/industrialio-hw-consumer.c
+++ b/drivers/iio/buffer/industrialio-hw-consumer.c
@@ -40,6 +40,8 @@ static void iio_hw_buf_release(struct iio_buffer *buffer)
 {
 	struct hw_consumer_buffer *hw_buf =
 		iio_buffer_to_hw_consumer_buffer(buffer);
+
+	bitmap_free(buffer->scan_mask);
 	kfree(hw_buf);
 }
 

---
base-commit: 7080e32d3f09d8688c4a87d81bdcc71f7f606b16
change-id: 20260427-iio_buf-0459b3fa3de8

Best regards,
-- 
Felix Gu <ustc.gu@gmail.com>


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] iio: buffer: hw-consumer: free scan_mask on buffer release
  2026-04-27 11:11 [PATCH] iio: buffer: hw-consumer: free scan_mask on buffer release Felix Gu
@ 2026-04-27 15:19 ` Andy Shevchenko
  2026-04-28 17:28 ` Jonathan Cameron
  1 sibling, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2026-04-27 15:19 UTC (permalink / raw)
  To: Felix Gu
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

On Mon, Apr 27, 2026 at 07:11:39PM +0800, Felix Gu wrote:
> The scan_mask lifetime changed in commit 9a2e1233d38c ("iio: buffer:
> hw-consumer: remove redundant scan_mask flexible array").
> 
> Before that change, the scan mask storage was embedded in struct
> hw_consumer_buffer, so iio_hw_buf_release() could free the whole
> allocation with a single kfree(hw_buf).
> 
> That commit moved the scan mask to a separate bitmap_zalloc() allocation
> stored in buffer.scan_mask, but left iio_hw_buf_release() unchanged.
> 
> Free the scan mask in iio_hw_buf_release() before freeing the buffer
> wrapper.

Good catch!

Seems that struct iio_buffer is a foundation class that is used in several
objects and hence each of them is responsible to allocate/free scan_mask
(and others, if required). That's why it's easy to forget to clean up that
properly. I was thinking of a common ground to have that being done in one
place, but it seems unfeasible.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] iio: buffer: hw-consumer: free scan_mask on buffer release
  2026-04-27 11:11 [PATCH] iio: buffer: hw-consumer: free scan_mask on buffer release Felix Gu
  2026-04-27 15:19 ` Andy Shevchenko
@ 2026-04-28 17:28 ` Jonathan Cameron
  2026-04-29  7:04   ` Andy Shevchenko
  1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2026-04-28 17:28 UTC (permalink / raw)
  To: Felix Gu
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

On Mon, 27 Apr 2026 19:11:39 +0800
Felix Gu <ustc.gu@gmail.com> wrote:

> The scan_mask lifetime changed in commit 9a2e1233d38c ("iio: buffer:
> hw-consumer: remove redundant scan_mask flexible array").
> 
> Before that change, the scan mask storage was embedded in struct
> hw_consumer_buffer, so iio_hw_buf_release() could free the whole
> allocation with a single kfree(hw_buf).

> 
> That commit moved the scan mask to a separate bitmap_zalloc() allocation

> stored in buffer.scan_mask, but left iio_hw_buf_release() unchanged.

Hmm. This reuse of a pointer that is otherwise set via very different
paths sent me down a wild goose chase.

Patch seems fine, but I'd like Nuno to take a quick look before I pick it up.

Jonathan

> 
> Free the scan mask in iio_hw_buf_release() before freeing the buffer
> wrapper.
> 
> Fixes: 9a2e1233d38c ("iio: buffer: hw-consumer: remove redundant scan_mask flexible array")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
>  drivers/iio/buffer/industrialio-hw-consumer.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/buffer/industrialio-hw-consumer.c b/drivers/iio/buffer/industrialio-hw-consumer.c
> index 24d7df603760..d1ab2cbf8c88 100644
> --- a/drivers/iio/buffer/industrialio-hw-consumer.c
> +++ b/drivers/iio/buffer/industrialio-hw-consumer.c
> @@ -40,6 +40,8 @@ static void iio_hw_buf_release(struct iio_buffer *buffer)
>  {
>  	struct hw_consumer_buffer *hw_buf =
>  		iio_buffer_to_hw_consumer_buffer(buffer);
> +
> +	bitmap_free(buffer->scan_mask);
>  	kfree(hw_buf);
>  }
>  
> 
> ---
> base-commit: 7080e32d3f09d8688c4a87d81bdcc71f7f606b16
> change-id: 20260427-iio_buf-0459b3fa3de8
> 
> Best regards,


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] iio: buffer: hw-consumer: free scan_mask on buffer release
  2026-04-28 17:28 ` Jonathan Cameron
@ 2026-04-29  7:04   ` Andy Shevchenko
  2026-04-29  9:22     ` Jonathan Cameron
  0 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2026-04-29  7:04 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Felix Gu, David Lechner, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

On Tue, Apr 28, 2026 at 06:28:58PM +0100, Jonathan Cameron wrote:
> On Mon, 27 Apr 2026 19:11:39 +0800
> Felix Gu <ustc.gu@gmail.com> wrote:
> 
> > The scan_mask lifetime changed in commit 9a2e1233d38c ("iio: buffer:
> > hw-consumer: remove redundant scan_mask flexible array").
> > 
> > Before that change, the scan mask storage was embedded in struct
> > hw_consumer_buffer, so iio_hw_buf_release() could free the whole
> > allocation with a single kfree(hw_buf).
> 
> > That commit moved the scan mask to a separate bitmap_zalloc() allocation
> 
> > stored in buffer.scan_mask, but left iio_hw_buf_release() unchanged.
> 
> Hmm. This reuse of a pointer that is otherwise set via very different
> paths sent me down a wild goose chase.
> 
> Patch seems fine, but I'd like Nuno to take a quick look before I pick it up.

Yeah, more eyeballs is better here. I spent like an hour to understand
mysterious ways of scan_mask in IIO (we have a few of them and we have a few
objects based on struct iio_buffer), and we have constructors/destructors for
those objects which of that is responsible to handle scan_mask as appropriate.

May be all of this is documented somewhere?.. Dunno.

> > Free the scan mask in iio_hw_buf_release() before freeing the buffer
> > wrapper.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] iio: buffer: hw-consumer: free scan_mask on buffer release
  2026-04-29  7:04   ` Andy Shevchenko
@ 2026-04-29  9:22     ` Jonathan Cameron
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2026-04-29  9:22 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Felix Gu, David Lechner, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

On Wed, 29 Apr 2026 10:04:48 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Apr 28, 2026 at 06:28:58PM +0100, Jonathan Cameron wrote:
> > On Mon, 27 Apr 2026 19:11:39 +0800
> > Felix Gu <ustc.gu@gmail.com> wrote:
> >   
> > > The scan_mask lifetime changed in commit 9a2e1233d38c ("iio: buffer:
> > > hw-consumer: remove redundant scan_mask flexible array").
> > > 
> > > Before that change, the scan mask storage was embedded in struct
> > > hw_consumer_buffer, so iio_hw_buf_release() could free the whole
> > > allocation with a single kfree(hw_buf).  
> >   
> > > That commit moved the scan mask to a separate bitmap_zalloc() allocation  
> >   
> > > stored in buffer.scan_mask, but left iio_hw_buf_release() unchanged.  
> > 
> > Hmm. This reuse of a pointer that is otherwise set via very different
> > paths sent me down a wild goose chase.
> > 
> > Patch seems fine, but I'd like Nuno to take a quick look before I pick it up.  
> 
> Yeah, more eyeballs is better here. I spent like an hour to understand
> mysterious ways of scan_mask in IIO (we have a few of them and we have a few
> objects based on struct iio_buffer), and we have constructors/destructors for
> those objects which of that is responsible to handle scan_mask as appropriate.

Likewise on time to understand this and I wrote some of them :(

> 
> May be all of this is documented somewhere?.. Dunno.

Have a guess :(  Nope.  This one is downright odd and only really exists
to leverage some helper functions.   With hindsight I think maybe we'd
be better of separating it out so it doesn't use the same structure.

But for now we need a fix.


> 
> > > Free the scan mask in iio_hw_buf_release() before freeing the buffer
> > > wrapper.  
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-29  9:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-27 11:11 [PATCH] iio: buffer: hw-consumer: free scan_mask on buffer release Felix Gu
2026-04-27 15:19 ` Andy Shevchenko
2026-04-28 17:28 ` Jonathan Cameron
2026-04-29  7:04   ` Andy Shevchenko
2026-04-29  9:22     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox