* [PATCH 0/3] dma_mapping: Add auto cleanup support
@ 2025-10-10 18:50 ` Frank Li
2025-10-10 18:50 ` [PATCH 1/3] cleanup: Add DEFINE_GUARD_ARGS_CLASS macro for resource alloc/free functions with multiple arguments Frank Li
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Frank Li @ 2025-10-10 18:50 UTC (permalink / raw)
To: Marek Szyprowski, Robin Murphy, Dong Aisheng, Andi Shyti,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: linux-kernel, iommu, linux-i2c, imx, linux-arm-kernel, Frank Li
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>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] cleanup: Add DEFINE_GUARD_ARGS_CLASS macro for resource alloc/free functions with multiple arguments
2025-10-10 18:50 ` [PATCH 0/3] dma_mapping: Add auto cleanup support Frank Li
@ 2025-10-10 18:50 ` Frank Li
2025-10-10 18:50 ` [PATCH 2/3] dma-mapping: Add auto cleanup support dma_map_single() Frank Li
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Frank Li @ 2025-10-10 18:50 UTC (permalink / raw)
To: Marek Szyprowski, Robin Murphy, Dong Aisheng, Andi Shyti,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: linux-kernel, iommu, linux-i2c, imx, linux-arm-kernel, Frank Li
Some resource alloc/free functions require passing more than one argument,
such as dma_map_single() and dma_unmap_single(). Add
DEFINE_GUARD_ARGS_CLASS() to support these cases by introducing a helper
structure to store all arguments for the resource cleanup function.
DEFINE_GUARD_ARGS_CLASS(..., _list_class_fields, _list_func_args, _list_args)
@_list_class_fields: argument list with types, separated by ';'
e.g. (struct device *dev; void *ptr; size_t size; enum dma_data_direction dir; int attr)
@_list_func_args: argument list with types, separated by ','
e.g. (struct device *dev, void *ptr, size_t size, enum dma_data_direction dir, int attr)
@_list_args: argument list without types, separated by ','
e.g. (dev, ptr, size, dir, attr)
Three lists are needed because C syntax differs between struct fields (';'),
function parameters (','), and variable arguments (no types).
Example usage for dma_map_single():
DEFINE_GUARD_ARGS_CLASS(dma_map_single, dma_addr_t,
dma_mapping_error(_T.args.dev, _T.dma_addr),
dma_unmap_single(_T->args.dev, _T->ret,
_T->args.size, _T->args.dir),
dma_map_single,
(struct device *dev; void *ptr; size_t size; enum dma_data_direction dir),
(struct device *dev, void *ptr, size_t size, enum dma_data_direction dir),
(dev, ptr, size, dir))
Example:
fun()
{
CLASS(dma_map_single, dma)(dev, ...);
...
if (error)
return -EIO; // dma_unmap_single() will be auto-called
}
Add retain_and_empty() to keep resource when functions.
Example:
fun()
{
CLASS(dma_map_single, dma)(dev, ...);
...
if (error)
return -EIO; // dma_unmap_single() will be auto-called
retain_and_empty(dma); // dma_umap_single() will NOT called.
return 0;
}
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
There are some checkpatch.pl error, but these is correct and better
readablity.
ERROR: Macros with complex values should be enclosed in parentheses
+#define DEFINE_GUARD_ARGS_CLASS(_name, _return_type, _is_err, _exit, _init, _list_class_fields, _list_func_args, _list_args) \
+__DEFINE_GUARD_ARGS_CLASS(_name, _return_type, _list_class_fields) \
+__DEFINE_GUARD_ARGS_ENTRY(_name, _is_err, _init, _list_func_args, _list_args) \
+__DEFINE_GUARD_ARGS_EXIT(_name, _exit)
ERROR: Macros with complex values should be enclosed in parentheses
+#define __REMOVE_PAREN(x) __REMOVE_PAREN_HELP x
---
include/linux/cleanup.h | 73 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index 2573585b7f068abe992af1ac05f478fef7b34306..84b27dd374f2200382b6dc1c450320b79406fd87 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -518,4 +518,77 @@ __DEFINE_LOCK_GUARD_0(_name, _lock)
#define DEFINE_LOCK_GUARD_1_COND(X...) CONCATENATE(DEFINE_LOCK_GUARD_1_COND_, COUNT_ARGS(X))(X)
+/*
+ * Many resource allocation/free function pairs require passing multiple
+ * arguments — for example, dma_map_single() and dma_unmap_single()
+ *
+ * DEFINE_GUARD_ARGS_CLASS(dma_map_single, dma_addr_t,
+ * dma_mapping_error(_T.args.dev, _T.ret),
+ * dma_unmap_single(_T->args.dev, _T->ret, _T->args.size, _T->args.dir),
+ * dma_map_single,
+ * (struct device *dev; void *ptr; size_t size; enum dma_data_direction dir),
+ * (struct device *dev, void *ptr, size_t size, enum dma_data_direction dir),
+ * (dev, ptr, size, dir))
+ * Example usage:
+ *
+ * int fun() {
+ * CLASS(dma_map_single, dma)(dev, ptr, size, dir);
+ * if (!dma.okay)
+ * return -EIO
+ * ...
+ * if (condition)
+ * return -EIO // cleanup will auto unmap
+ *
+ * req->dma_addr = retain_and_empty(dma); //keep mapping
+ * return 0;
+ * }
+ */
+#define __REMOVE_PAREN_HELP(...) __VA_ARGS__
+#define __REMOVE_PAREN(x) __REMOVE_PAREN_HELP x
+
+#define __DEFINE_GUARD_ARGS_CLASS(_name, _return_type, _list_class_fields) \
+typedef struct { \
+ _return_type ret; \
+ bool okay; \
+ struct { \
+ __REMOVE_PAREN(_list_class_fields); \
+ } args; \
+} class_##_name##_t;
+
+#define __DEFINE_GUARD_ARGS_EXIT(_name, _exit) \
+static inline void class_##_name##_destructor(class_##_name##_t *_T) \
+{ if (_T->okay) { _exit; } }
+
+#define __DEFINE_GUARD_ARGS_ENTRY(_name, _is_err, _init, _list_func_args, _list__args) \
+static inline class_##_name##_t class_##_name##_constructor(__REMOVE_PAREN(_list_func_args)) \
+{ \
+ class_##_name##_t _T = { .args = { __REMOVE_PAREN(_list__args) } }; \
+ _T.ret = _init _list__args; \
+ _T.okay = !(_is_err); \
+ return _T; \
+}
+
+/**
+ * @_name: class name
+ * @_return_type: return data type
+ * @_is_err: macro to check init function return value
+ * @_exit: macro to do resource clean up
+ * @_init: macro/func name to alloc resource
+ * @_list_class_fields: (args list with type, use ; as split)
+ * @_list_func_args: (args list with type, use , as split)
+ * @_list_args: (args list without type, use , as split)
+ */
+#define DEFINE_GUARD_ARGS_CLASS(_name, _return_type, _is_err, _exit, _init, _list_class_fields, _list_func_args, _list_args) \
+__DEFINE_GUARD_ARGS_CLASS(_name, _return_type, _list_class_fields) \
+__DEFINE_GUARD_ARGS_ENTRY(_name, _is_err, _init, _list_func_args, _list_args) \
+__DEFINE_GUARD_ARGS_EXIT(_name, _exit)
+
+#define retain_and_empty(t) \
+ ({ \
+ __auto_type __ptr = &(t); typeof(t) empty = {}; \
+ __auto_type __val = *__ptr; \
+ *__ptr = empty; \
+ __val.ret; \
+ })
+
#endif /* _LINUX_CLEANUP_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] dma-mapping: Add auto cleanup support dma_map_single()
2025-10-10 18:50 ` [PATCH 0/3] dma_mapping: Add auto cleanup support Frank Li
2025-10-10 18:50 ` [PATCH 1/3] cleanup: Add DEFINE_GUARD_ARGS_CLASS macro for resource alloc/free functions with multiple arguments Frank Li
@ 2025-10-10 18:50 ` Frank Li
2025-10-10 18:50 ` [PATCH 3/3] i2c: lpi2c: Use auto cleanup for dma_map_single() Frank Li
2025-10-14 5:54 ` [PATCH 0/3] dma_mapping: Add auto cleanup support Marek Szyprowski
3 siblings, 0 replies; 8+ messages in thread
From: Frank Li @ 2025-10-10 18:50 UTC (permalink / raw)
To: Marek Szyprowski, Robin Murphy, Dong Aisheng, Andi Shyti,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: linux-kernel, iommu, linux-i2c, imx, linux-arm-kernel, Frank Li
Add auto cleanup support for dma_map_single().
fun()
{
CLASS(dma_map_single, dma)(dev, ...);
...
if (error)
return -EIO; // dma_unmap_single() will be auto-called
}
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
include/linux/dma-mapping.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 8248ff9363eed2ae5dc18e9eedb711b299201bea..89fc80e7cd85ba5a4068c61f58f3a578ab0a7a01 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -757,4 +757,12 @@ static inline int dma_mmap_wc(struct device *dev,
do { typeof(PTR) __p __maybe_unused = PTR; } while (0)
#endif
+DEFINE_GUARD_ARGS_CLASS(dma_map_single, dma_addr_t,
+ dma_mapping_error(_T.args.dev, _T.ret),
+ dma_unmap_single(_T->args.dev, _T->ret, _T->args.size, _T->args.dir),
+ dma_map_single,
+ (struct device *dev; void *ptr; size_t size; enum dma_data_direction dir),
+ (struct device *dev, void *ptr, size_t size, enum dma_data_direction dir),
+ (dev, ptr, size, dir))
+
#endif /* _LINUX_DMA_MAPPING_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] i2c: lpi2c: Use auto cleanup for dma_map_single()
2025-10-10 18:50 ` [PATCH 0/3] dma_mapping: Add auto cleanup support Frank Li
2025-10-10 18:50 ` [PATCH 1/3] cleanup: Add DEFINE_GUARD_ARGS_CLASS macro for resource alloc/free functions with multiple arguments Frank Li
2025-10-10 18:50 ` [PATCH 2/3] dma-mapping: Add auto cleanup support dma_map_single() Frank Li
@ 2025-10-10 18:50 ` Frank Li
2025-10-14 5:54 ` [PATCH 0/3] dma_mapping: Add auto cleanup support Marek Szyprowski
3 siblings, 0 replies; 8+ messages in thread
From: Frank Li @ 2025-10-10 18:50 UTC (permalink / raw)
To: Marek Szyprowski, Robin Murphy, Dong Aisheng, Andi Shyti,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: linux-kernel, iommu, linux-i2c, imx, linux-arm-kernel, Frank Li
Use auto cleanup for dma_map_single() to simple code.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Demostrate to how to use auto cleanup for dma map functiongs. It will
simple more if need map multi buffers at a functions.
---
drivers/i2c/busses/i2c-imx-lpi2c.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 03b5a7e8c361abe1d75fb4d31f9614bbc6387d93..2c89d441ab1b3e607b8a2b0a6589e5909fa39ef4 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -727,9 +727,11 @@ static int lpi2c_dma_rx_cmd_submit(struct lpi2c_imx_struct *lpi2c_imx)
struct dma_chan *txchan = dma->chan_tx;
dma_cookie_t cookie;
- dma->dma_tx_addr = dma_map_single(txchan->device->dev,
- dma->rx_cmd_buf, dma->rx_cmd_buf_len,
- DMA_TO_DEVICE);
+ CLASS(dma_map_single, dma_addr)(txchan->device->dev,
+ dma->rx_cmd_buf, dma->rx_cmd_buf_len,
+ DMA_TO_DEVICE);
+ dma->dma_tx_addr = dma_addr.ret;
+
if (dma_mapping_error(txchan->device->dev, dma->dma_tx_addr)) {
dev_err(&lpi2c_imx->adapter.dev, "DMA map failed, use pio\n");
return -EINVAL;
@@ -749,18 +751,15 @@ static int lpi2c_dma_rx_cmd_submit(struct lpi2c_imx_struct *lpi2c_imx)
goto submit_err_exit;
}
+ retain_and_empty(dma_addr);
dma_async_issue_pending(txchan);
return 0;
desc_prepare_err_exit:
- dma_unmap_single(txchan->device->dev, dma->dma_tx_addr,
- dma->rx_cmd_buf_len, DMA_TO_DEVICE);
return -EINVAL;
submit_err_exit:
- dma_unmap_single(txchan->device->dev, dma->dma_tx_addr,
- dma->rx_cmd_buf_len, DMA_TO_DEVICE);
dmaengine_desc_free(rx_cmd_desc);
return -EINVAL;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] dma_mapping: Add auto cleanup support
2025-10-10 18:50 ` [PATCH 0/3] dma_mapping: Add auto cleanup support Frank Li
` (2 preceding siblings ...)
2025-10-10 18:50 ` [PATCH 3/3] i2c: lpi2c: Use auto cleanup for dma_map_single() Frank Li
@ 2025-10-14 5:54 ` Marek Szyprowski
2025-10-19 12:03 ` Leon Romanovsky
2025-11-06 17:27 ` Frank Li
3 siblings, 2 replies; 8+ messages in thread
From: Marek Szyprowski @ 2025-10-14 5:54 UTC (permalink / raw)
To: Frank Li, Robin Murphy, Dong Aisheng, Andi Shyti, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Peter Zijlstra
Cc: linux-kernel, iommu, linux-i2c, imx, linux-arm-kernel,
Peter Zijlstra
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] dma_mapping: Add auto cleanup support
2025-10-14 5:54 ` [PATCH 0/3] dma_mapping: Add auto cleanup support Marek Szyprowski
@ 2025-10-19 12:03 ` Leon Romanovsky
2025-10-20 15:56 ` Frank Li
2025-11-06 17:27 ` Frank Li
1 sibling, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2025-10-19 12:03 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Frank Li, Robin Murphy, Dong Aisheng, Andi Shyti, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Peter Zijlstra, linux-kernel, iommu, linux-i2c, imx,
linux-arm-kernel
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
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] dma_mapping: Add auto cleanup support
2025-10-19 12:03 ` Leon Romanovsky
@ 2025-10-20 15:56 ` Frank Li
0 siblings, 0 replies; 8+ messages in thread
From: Frank Li @ 2025-10-20 15:56 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Marek Szyprowski, Robin Murphy, Dong Aisheng, Andi Shyti,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Peter Zijlstra, linux-kernel, iommu, linux-i2c, imx,
linux-arm-kernel
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
> >
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] dma_mapping: Add auto cleanup support
2025-10-14 5:54 ` [PATCH 0/3] dma_mapping: Add auto cleanup support Marek Szyprowski
2025-10-19 12:03 ` Leon Romanovsky
@ 2025-11-06 17:27 ` Frank Li
1 sibling, 0 replies; 8+ messages in thread
From: Frank Li @ 2025-11-06 17:27 UTC (permalink / raw)
To: Marek Szyprowski, Peter Zijlstra
Cc: Robin Murphy, Dong Aisheng, Andi Shyti, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Peter Zijlstra,
linux-kernel, iommu, linux-i2c, imx, linux-arm-kernel
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-11-06 17:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20251010185046eucas1p26868b540b74a96e36943066216525bed@eucas1p2.samsung.com>
2025-10-10 18:50 ` [PATCH 0/3] dma_mapping: Add auto cleanup support Frank Li
2025-10-10 18:50 ` [PATCH 1/3] cleanup: Add DEFINE_GUARD_ARGS_CLASS macro for resource alloc/free functions with multiple arguments Frank Li
2025-10-10 18:50 ` [PATCH 2/3] dma-mapping: Add auto cleanup support dma_map_single() Frank Li
2025-10-10 18:50 ` [PATCH 3/3] i2c: lpi2c: Use auto cleanup for dma_map_single() Frank Li
2025-10-14 5:54 ` [PATCH 0/3] dma_mapping: Add auto cleanup support Marek Szyprowski
2025-10-19 12:03 ` Leon Romanovsky
2025-10-20 15:56 ` Frank Li
2025-11-06 17:27 ` Frank Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).