* [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:28 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).