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(-)
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>
---
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>
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.
> ---
> 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
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.
Peter Zijlstra:
Do you have chance to check this?
Frank
>
>
> > ---
> > 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
>
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
>
>
On Sun, Oct 19, 2025 at 03:03:04PM +0300, Leon Romanovsky wrote:
> 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.
Yes, DMA API is much complicated. Actually macro CLASS() is using
__cleanup().
#define CLASS(_name, var) \
class_##_name##_t var __cleanup(class_##_name##_destructor) = \
class_##_name##_constructor
Frank
>
> 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
> >
> >
© 2016 - 2025 Red Hat, Inc.