| Message ID | 20230718-nsecs_to_jiffies_timeout-constant-logical-operand-v1-1-36ed8fc8faea@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout() | expand |
On Tue, Jul 18, 2023 at 2:44 PM Nathan Chancellor <nathan@kernel.org> wrote: > > A proposed update to clang's -Wconstant-logical-operand to warn when the > left hand side is a constant shows the following instance in > nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ, > such as CONFIG_HZ=300: > > In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12: > drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand] > 343 | if (NSEC_PER_SEC % HZ && > | ~~~~~~~~~~~~~~~~~ ^ > drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation > 343 | if (NSEC_PER_SEC % HZ && > | ^~ > | & > drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this warning > 1 warning generated. > > Turn this into an explicit comparison against zero to make the > expression a boolean to make it clear this should be a logical check, > not a bitwise one. > > Link: https://reviews.llvm.org/D142609 > Signed-off-by: Nathan Chancellor <nathan@kernel.org> Thanks for the patch! Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > drivers/gpu/drm/v3d/v3d_drv.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h > index b74b1351bfc8..7f664a4b2a75 100644 > --- a/drivers/gpu/drm/v3d/v3d_drv.h > +++ b/drivers/gpu/drm/v3d/v3d_drv.h > @@ -340,7 +340,7 @@ struct v3d_submit_ext { > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n) > { > /* nsecs_to_jiffies64() does not guard against overflow */ > - if (NSEC_PER_SEC % HZ && > + if ((NSEC_PER_SEC % HZ) != 0 && > div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ) > return MAX_JIFFY_OFFSET; > > > -- > 2.41.0 >
Hi Nathan, On 7/18/23 18:44, Nathan Chancellor wrote: > A proposed update to clang's -Wconstant-logical-operand to warn when the > left hand side is a constant shows the following instance in > nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ, > such as CONFIG_HZ=300: > > In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12: > drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand] > 343 | if (NSEC_PER_SEC % HZ && > | ~~~~~~~~~~~~~~~~~ ^ > drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation > 343 | if (NSEC_PER_SEC % HZ && > | ^~ > | & > drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this warning > 1 warning generated. > > Turn this into an explicit comparison against zero to make the > expression a boolean to make it clear this should be a logical check, > not a bitwise one. > > Link: https://reviews.llvm.org/D142609 > Signed-off-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Maíra Canal <mcanal@igalia.com> Thanks for all the hard work with clang! Let me know if you need someone to push it to drm-misc-next. Best Regards, - Maíra > --- > drivers/gpu/drm/v3d/v3d_drv.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h > index b74b1351bfc8..7f664a4b2a75 100644 > --- a/drivers/gpu/drm/v3d/v3d_drv.h > +++ b/drivers/gpu/drm/v3d/v3d_drv.h > @@ -340,7 +340,7 @@ struct v3d_submit_ext { > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n) > { > /* nsecs_to_jiffies64() does not guard against overflow */ > - if (NSEC_PER_SEC % HZ && > + if ((NSEC_PER_SEC % HZ) != 0 && > div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ) > return MAX_JIFFY_OFFSET; > >
Hi Maira, On Thu, Jul 27, 2023 at 11:01:27AM -0300, Maira Canal wrote: > Hi Nathan, > > On 7/18/23 18:44, Nathan Chancellor wrote: > > A proposed update to clang's -Wconstant-logical-operand to warn when the > > left hand side is a constant shows the following instance in > > nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ, > > such as CONFIG_HZ=300: > > > > In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12: > > drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand] > > 343 | if (NSEC_PER_SEC % HZ && > > | ~~~~~~~~~~~~~~~~~ ^ > > drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation > > 343 | if (NSEC_PER_SEC % HZ && > > | ^~ > > | & > > drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this warning > > 1 warning generated. > > > > Turn this into an explicit comparison against zero to make the > > expression a boolean to make it clear this should be a logical check, > > not a bitwise one. > > > > Link: https://reviews.llvm.org/D142609 > > Signed-off-by: Nathan Chancellor <nathan@kernel.org> > > Reviewed-by: Maíra Canal <mcanal@igalia.com> > > Thanks for all the hard work with clang! Let me know if you need someone > to push it to drm-misc-next. Yes I will, I do not have drm commit rights. I think the i915 patch can go to drm-misc as well with Tvrtko's ack. Thanks a lot for taking a look! Cheers, Nathan > Best Regards, > - Maíra > > > --- > > drivers/gpu/drm/v3d/v3d_drv.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h > > index b74b1351bfc8..7f664a4b2a75 100644 > > --- a/drivers/gpu/drm/v3d/v3d_drv.h > > +++ b/drivers/gpu/drm/v3d/v3d_drv.h > > @@ -340,7 +340,7 @@ struct v3d_submit_ext { > > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n) > > { > > /* nsecs_to_jiffies64() does not guard against overflow */ > > - if (NSEC_PER_SEC % HZ && > > + if ((NSEC_PER_SEC % HZ) != 0 && > > div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ) > > return MAX_JIFFY_OFFSET; > >
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h index b74b1351bfc8..7f664a4b2a75 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.h +++ b/drivers/gpu/drm/v3d/v3d_drv.h @@ -340,7 +340,7 @@ struct v3d_submit_ext { static inline unsigned long nsecs_to_jiffies_timeout(const u64 n) { /* nsecs_to_jiffies64() does not guard against overflow */ - if (NSEC_PER_SEC % HZ && + if ((NSEC_PER_SEC % HZ) != 0 && div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ) return MAX_JIFFY_OFFSET;
A proposed update to clang's -Wconstant-logical-operand to warn when the left hand side is a constant shows the following instance in nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ, such as CONFIG_HZ=300: In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12: drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand] 343 | if (NSEC_PER_SEC % HZ && | ~~~~~~~~~~~~~~~~~ ^ drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation 343 | if (NSEC_PER_SEC % HZ && | ^~ | & drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this warning 1 warning generated. Turn this into an explicit comparison against zero to make the expression a boolean to make it clear this should be a logical check, not a bitwise one. Link: https://reviews.llvm.org/D142609 Signed-off-by: Nathan Chancellor <nathan@kernel.org> --- drivers/gpu/drm/v3d/v3d_drv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)