| Message ID | 20250320185238.447458-30-jim.cromie@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y | expand |
Le 20/03/2025 à 19:52, Jim Cromie a écrit : > The Xe driver's XE_IOCTL_DBG macro calls drm_dbg() from inside an if > (expression). This breaks when CONFIG_DRM_USE_DYNAMIC_DEBUG=y because > the invoked macro has a do-while-0 wrapper. > > if (cond && (drm_dbg("expr-form"),1)) { > ... do some more stuff > } > > Fix for this usage by changing __dynamic_func_call_cls{,_no_desc} > macros into expressions, by replacing the do-while-0s with a ({ }) > wrapper. In the common usage, the trailing ';' converts the > expression into a statement. > > drm_dbg("statement form"); > > Signed-off-by: Jim Cromie <jim.cromie@gmail.com> > --- > --- > include/linux/dynamic_debug.h | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h > index 8043966a0fd6..80bcaad03400 100644 > --- a/include/linux/dynamic_debug.h > +++ b/include/linux/dynamic_debug.h > @@ -339,20 +339,20 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > * (|_cls): adds in _DPRINT_CLASS_DFLT as needed > * (|_no_desc): former gets callsite descriptor as 1st arg (for prdbgs) > */ > -#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do { \ > - DEFINE_DYNAMIC_DEBUG_METADATA_CLS((id), cls, fmt); \ > +#define __dynamic_func_call_cls(id, cls, fmt, func, ...) ({ \ > + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ Is it normal to remove the parenthesis around id? Or the other way around, why did you add parenthesis in PATCH 17? > if (DYNAMIC_DEBUG_BRANCH(id)) \ > - func(&id, ##__VA_ARGS__); \ > -} while (0) > + func(&(id), ##__VA_ARGS__); \ > +}) > #define __dynamic_func_call(id, fmt, func, ...) \ > __dynamic_func_call_cls(id, _DPRINTK_CLASS_DFLT, fmt, \ > func, ##__VA_ARGS__) > > -#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) do { \ > +#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) ({ \ > DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ I expect the same constraints around id here, both with parenthesis, or no parenthesis at all. > if (DYNAMIC_DEBUG_BRANCH(id)) \ > func(__VA_ARGS__); \ > -} while (0) > +}) > #define __dynamic_func_call_no_desc(id, fmt, func, ...) \ > __dynamic_func_call_cls_no_desc(id, _DPRINTK_CLASS_DFLT, \ > fmt, func, ##__VA_ARGS__)
On Mon, Mar 24, 2025 at 9:19 AM Louis Chauvet <louis.chauvet@bootlin.com> wrote: > > > > Le 20/03/2025 à 19:52, Jim Cromie a écrit : > > The Xe driver's XE_IOCTL_DBG macro calls drm_dbg() from inside an if > > (expression). This breaks when CONFIG_DRM_USE_DYNAMIC_DEBUG=y because > > the invoked macro has a do-while-0 wrapper. > > > > if (cond && (drm_dbg("expr-form"),1)) { > > ... do some more stuff > > } > > > > Fix for this usage by changing __dynamic_func_call_cls{,_no_desc} > > macros into expressions, by replacing the do-while-0s with a ({ }) > > wrapper. In the common usage, the trailing ';' converts the > > expression into a statement. > > > > drm_dbg("statement form"); > > > > Signed-off-by: Jim Cromie <jim.cromie@gmail.com> > > --- > > --- > > include/linux/dynamic_debug.h | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h > > index 8043966a0fd6..80bcaad03400 100644 > > --- a/include/linux/dynamic_debug.h > > +++ b/include/linux/dynamic_debug.h > > @@ -339,20 +339,20 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > > * (|_cls): adds in _DPRINT_CLASS_DFLT as needed > > * (|_no_desc): former gets callsite descriptor as 1st arg (for prdbgs) > > */ > > -#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do { \ > > - DEFINE_DYNAMIC_DEBUG_METADATA_CLS((id), cls, fmt); \ > > +#define __dynamic_func_call_cls(id, cls, fmt, func, ...) ({ \ > > + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ > > Is it normal to remove the parenthesis around id? Or the other way > around, why did you add parenthesis in PATCH 17? > heisen-thinking ? noisy inputs ? historically, checkpatch warnings on macros have given me difficulty so I tend toward defense. I think this one was a red-herring. > > if (DYNAMIC_DEBUG_BRANCH(id)) \ > > - func(&id, ##__VA_ARGS__); \ > > -} while (0) > > + func(&(id), ##__VA_ARGS__); \ > > +}) > > #define __dynamic_func_call(id, fmt, func, ...) \ > > __dynamic_func_call_cls(id, _DPRINTK_CLASS_DFLT, fmt, \ > > func, ##__VA_ARGS__) > > > > -#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) do { \ > > +#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) ({ \ > > DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ > > I expect the same constraints around id here, both with parenthesis, or > no parenthesis at all. > yes, inconsistent. > > if (DYNAMIC_DEBUG_BRANCH(id)) \ > > func(__VA_ARGS__); \ > > -} while (0) > > +}) > > #define __dynamic_func_call_no_desc(id, fmt, func, ...) \ > > __dynamic_func_call_cls_no_desc(id, _DPRINTK_CLASS_DFLT, \ > > fmt, func, ##__VA_ARGS__) > > -- > Louis Chauvet, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > >
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h index 8043966a0fd6..80bcaad03400 100644 --- a/include/linux/dynamic_debug.h +++ b/include/linux/dynamic_debug.h @@ -339,20 +339,20 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, * (|_cls): adds in _DPRINT_CLASS_DFLT as needed * (|_no_desc): former gets callsite descriptor as 1st arg (for prdbgs) */ -#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do { \ - DEFINE_DYNAMIC_DEBUG_METADATA_CLS((id), cls, fmt); \ +#define __dynamic_func_call_cls(id, cls, fmt, func, ...) ({ \ + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ if (DYNAMIC_DEBUG_BRANCH(id)) \ - func(&id, ##__VA_ARGS__); \ -} while (0) + func(&(id), ##__VA_ARGS__); \ +}) #define __dynamic_func_call(id, fmt, func, ...) \ __dynamic_func_call_cls(id, _DPRINTK_CLASS_DFLT, fmt, \ func, ##__VA_ARGS__) -#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) do { \ +#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) ({ \ DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ if (DYNAMIC_DEBUG_BRANCH(id)) \ func(__VA_ARGS__); \ -} while (0) +}) #define __dynamic_func_call_no_desc(id, fmt, func, ...) \ __dynamic_func_call_cls_no_desc(id, _DPRINTK_CLASS_DFLT, \ fmt, func, ##__VA_ARGS__)
The Xe driver's XE_IOCTL_DBG macro calls drm_dbg() from inside an if (expression). This breaks when CONFIG_DRM_USE_DYNAMIC_DEBUG=y because the invoked macro has a do-while-0 wrapper. if (cond && (drm_dbg("expr-form"),1)) { ... do some more stuff } Fix for this usage by changing __dynamic_func_call_cls{,_no_desc} macros into expressions, by replacing the do-while-0s with a ({ }) wrapper. In the common usage, the trailing ';' converts the expression into a statement. drm_dbg("statement form"); Signed-off-by: Jim Cromie <jim.cromie@gmail.com> --- --- include/linux/dynamic_debug.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)