Re: [PATCH 0/3] dma_mapping: Add auto cleanup support

[Leon Romanovsky <leon@kernel.org>]

On Tue, Oct 14, 2025 at 07:54:46AM +0200, Marek Szyprowski wrote:
> Hi
> 
> On 10.10.2025 20:50, Frank Li wrote:
> > There are many below pattern
> >
> > 	fun()
> > 	{
> > 		...
> > 		dma_map_single();
> > 		if (dma_mapping_error)
> > 			goto err1;
> >
> > 		dmaengine_prep_slave_single()
> > 		if (...)
> > 			goto err2
> >
> > 		dmaengine_submit()
> > 		if (...)
> > 			goto err3
> >
> > 		wait_for_completion_timeout()
> > 		if (...)
> > 			goto err4
> >
> > 	err4:
> > 	err3:
> > 	err2:
> > 		dma_umap_single();
> > 	err1:
> > 	}
> >
> > Use cleanup can simple error handle like guard(), such as guard(mutex).
> > or __free(kfree) = kmalloc.
> >
> > But dma_umap_single() need more argurements. So situation below complex.
> >
> > It need pack argurments list into structure.
> >
> > 	#define __DEFINE_GUARD_CLASS(_name, _return_type, _list_class_fields)   \
> > 	typedef struct {                                                \
> > 	       	 _return_type ret;                                       \
> > 	        bool    okay;                                           \
> > 	        struct  {                                               \
> > 	                __REMOVE(_list_class_fields);                   \
> > 	        } args;                                                 \
> > 	} class_##_name##_t;
> >
> > So save all arugments to it.
> >
> > __DEFINE_GUARD_CLASS(dma_map_single, dma_addr_t, (struct device *dev; void *ptr; size_t size; enum dma_data_direction dir)
> > will expand to
> >
> > 	struct {
> > 		dma_addr_t ret;
> > 		bool	okay;
> > 		struct {
> > 			struct device *dev;
> > 			void *ptr;
> > 			size_t size;
> > 			enum dma_data_direction dir;
> > 		}
> > 	}
> >
> > So cleanup function can use saved argurement.
> >
> > The above fun will be
> >
> > 	fun()
> > 	{
> > 		CLASS(dma_map_single, dma)(dev, ...);
> >
> > 		...
> > 		if (...)
> > 			return err;
> > 	}
> >
> > if funtion return, which need keep map,
> >
> > 	submit()
> > 	{
> > 		dma_map_single();
> > 		...
> > 		dmaengine_submit();
> > 		if (...)
> > 			goto err1
> > 		return;
> >
> > 	goto err1:
> > 		dma_umap_single();
> > 	}
> >
> > Macro retain_and_empty() will clean varible to avoid unmap.
> >
> >          ({                                  \
> >                  __auto_type __ptr = &(t); typeof(t) empty= {};   \
> >                  __auto_type __val = *__ptr; \
> >                  __ptr->okay = 0;        \
> >                  __val.ret;              \
> >          })
> >
> > So
> >
> > 	submit()
> > 	{
> >         		CLASS(dma_map_single, dma)(dev,...;
> >
> > 	        ...
> > 	        dmaengine_submit();
> > 		if (...)
> > 			return err;
> >
> > 		//before return;
> >
> > 		retain_and_empty(dma)
> > 	}
> >
> > This series just show how to hanndle many agurement at resource alloc/free
> > functions. Only show dma_map_single. If the over all method is acceptable.
> > I will more define for dma mapping functions.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> 
> This looks fine from the DMA-mapping API perspective. I think we should 
> also get some feedback from Peter, who authored most of the __cleanup() 
> based infrastructure, so I've added him to the recipients list.

I may represent minority here, but patch "i2c: lpi2c: Use auto cleanup
for dma_map_single()" looks completely unreadable after this change.

It is perfectly valid to use __cleanup() for simple and scoped things
like kmalloc, but DMA API is much complicated than that.

Thanks

> 
> 
> > ---
> > Frank Li (3):
> >        cleanup: Add DEFINE_GUARD_ARGS_CLASS macro for resource alloc/free functions with multiple arguments
> >        dma-mapping: Add auto cleanup support dma_map_single()
> >        i2c: lpi2c: Use auto cleanup for dma_map_single()
> >
> >   drivers/i2c/busses/i2c-imx-lpi2c.c | 13 ++++---
> >   include/linux/cleanup.h            | 73 ++++++++++++++++++++++++++++++++++++++
> >   include/linux/dma-mapping.h        |  8 +++++
> >   3 files changed, 87 insertions(+), 7 deletions(-)
> > ---
> > base-commit: 7c3ba4249a3604477ea9c077e10089ba7ddcaa03
> > change-id: 20251008-dmamap_cleanup-d0a7f0525a3d
> >
> > Best regards,
> > --
> > Frank Li <Frank.Li@nxp.com>
> >
> >
> Best regards
> -- 
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
> 
>