Re: [PATCH 2/2] drm/i915: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 18/07/2023 22: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:

   drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
     189 |         if (NSEC_PER_SEC % HZ &&
         |             ~~~~~~~~~~~~~~~~~ ^
   drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: note: use '&' for a bitwise operation
     189 |         if (NSEC_PER_SEC % HZ &&
         |                               ^~
         |                               &
   drivers/gpu/drm/i915/gem/i915_gem_wait.c:189: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.

So -Wconstant-logical-operand only triggers when it is a constant but not zero constant? Why does that make sense is not a kludge to avoid too much noise?

Personally, it all feels a bit over the top as a warning, since code in both cases should optimise away. And we may end up papering over it if it becomes a default.

Then again this patch IMO does make the code more readable, so I am happy to take this one via our tree. Or either give ack to bring it in via drm-misc-next:

Acked-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Let me know which route works best.

Regards,

Tvrtko

Link: https://reviews.llvm.org/D142609
Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx>
---
  drivers/gpu/drm/i915/gem/i915_gem_wait.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
index 4a33ad2d122b..d4b918fb11ce 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
@@ -186,7 +186,7 @@ i915_gem_object_wait(struct drm_i915_gem_object *obj,
  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;



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux