* + fix-mult_frac-multiple-argument-evaluation-bug.patch added to mm-nonmm-unstable branch
@ 2023-05-19 20:53 Andrew Morton
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2023-05-19 20:53 UTC (permalink / raw)
To: mm-commits, adobriyan, akpm
The patch titled
Subject: include/linux/math.h: fix mult_frac() multiple argument evaluation bug
has been added to the -mm mm-nonmm-unstable branch. Its filename is
fix-mult_frac-multiple-argument-evaluation-bug.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/fix-mult_frac-multiple-argument-evaluation-bug.patch
This patch will later appear in the mm-nonmm-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days
------------------------------------------------------
From: Alexey Dobriyan <adobriyan@gmail.com>
Subject: include/linux/math.h: fix mult_frac() multiple argument evaluation bug
Date: Fri, 19 May 2023 23:24:54 +0300
mult_frac() evaluates _all_ arguments multiple times in the body.
Clarify comment while I'm at it.
Link: https://lkml.kernel.org/r/f522ad25-f899-4526-abc4-da35868b6a8b@p183
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
include/linux/math.h | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
--- a/include/linux/math.h~fix-mult_frac-multiple-argument-evaluation-bug
+++ a/include/linux/math.h
@@ -118,17 +118,17 @@ __STRUCT_FRACT(s32)
__STRUCT_FRACT(u32)
#undef __STRUCT_FRACT
-/*
- * Multiplies an integer by a fraction, while avoiding unnecessary
- * overflow or loss of precision.
- */
-#define mult_frac(x, numer, denom)( \
-{ \
- typeof(x) quot = (x) / (denom); \
- typeof(x) rem = (x) % (denom); \
- (quot * (numer)) + ((rem * (numer)) / (denom)); \
-} \
-)
+/* Calculate "x * n / d" without unnecessary overflow or loss of precision. */
+#define mult_frac(x, n, d) \
+({ \
+ typeof(x) x_ = (x); \
+ typeof(n) n_ = (n); \
+ typeof(d) d_ = (d); \
+ \
+ typeof(x) q = x_ / d_; \
+ typeof(x) r = x_ % d_; \
+ q * n_ + r * n_ / d_; \
+})
#define sector_div(a, b) do_div(a, b)
_
Patches currently in -mm which might be from adobriyan@gmail.com are
add-intptr_t.patch
fix-mult_frac-multiple-argument-evaluation-bug.patch
^ permalink raw reply [flat|nested] 5+ messages in thread
* + fix-mult_frac-multiple-argument-evaluation-bug.patch added to mm-nonmm-unstable branch
@ 2023-05-22 21:15 Andrew Morton
2023-05-22 23:45 ` Thomas Gleixner
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2023-05-22 21:15 UTC (permalink / raw)
To: mm-commits, adobriyan, akpm
The patch titled
Subject: include/linux/math.h: fix mult_frac() multiple argument evaluation bug
has been added to the -mm mm-nonmm-unstable branch. Its filename is
fix-mult_frac-multiple-argument-evaluation-bug.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/fix-mult_frac-multiple-argument-evaluation-bug.patch
This patch will later appear in the mm-nonmm-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days
------------------------------------------------------
From: Alexey Dobriyan <adobriyan@gmail.com>
Subject: include/linux/math.h: fix mult_frac() multiple argument evaluation bug
Date: Sat, 20 May 2023 21:25:19 +0300
mult_frac() evaluates _all_ arguments multiple times in the body.
Clarify comment while I'm at it.
Link: https://lkml.kernel.org/r/f9f9fdbb-ec8e-4f5e-a998-2a58627a1a43@p183
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
include/linux/math.h | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
--- a/include/linux/math.h~fix-mult_frac-multiple-argument-evaluation-bug
+++ a/include/linux/math.h
@@ -118,17 +118,17 @@ __STRUCT_FRACT(s32)
__STRUCT_FRACT(u32)
#undef __STRUCT_FRACT
-/*
- * Multiplies an integer by a fraction, while avoiding unnecessary
- * overflow or loss of precision.
- */
-#define mult_frac(x, numer, denom)( \
-{ \
- typeof(x) quot = (x) / (denom); \
- typeof(x) rem = (x) % (denom); \
- (quot * (numer)) + ((rem * (numer)) / (denom)); \
-} \
-)
+/* Calculate "x * n / d" without unnecessary overflow or loss of precision. */
+#define mult_frac(x, n, d) \
+({ \
+ typeof(x) x_ = (x); \
+ typeof(n) n_ = (n); \
+ typeof(d) d_ = (d); \
+ \
+ typeof(x_) q = x_ / d_; \
+ typeof(x_) r = x_ % d_; \
+ q * n_ + r * n_ / d_; \
+})
#define sector_div(a, b) do_div(a, b)
_
Patches currently in -mm which might be from adobriyan@gmail.com are
add-intptr_t.patch
fix-mult_frac-multiple-argument-evaluation-bug.patch
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: + fix-mult_frac-multiple-argument-evaluation-bug.patch added to mm-nonmm-unstable branch
2023-05-22 21:15 + fix-mult_frac-multiple-argument-evaluation-bug.patch added to mm-nonmm-unstable branch Andrew Morton
@ 2023-05-22 23:45 ` Thomas Gleixner
2023-05-23 8:48 ` Alexey Dobriyan
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Gleixner @ 2023-05-22 23:45 UTC (permalink / raw)
To: linux-kernel, mm-commits, adobriyan, akpm
On Mon, May 22 2023 at 14:15, Andrew Morton wrote:
> ------------------------------------------------------
> From: Alexey Dobriyan <adobriyan@gmail.com>
> Subject: include/linux/math.h: fix mult_frac() multiple argument evaluation bug
> Date: Sat, 20 May 2023 21:25:19 +0300
>
> mult_frac() evaluates _all_ arguments multiple times in the body.
I'm not opposed to the patch, but to the description.
Multiple evaluation is not a bug per se.
Unless there is a reasonable explanation for the alleged bug this is
just a cosmetic exercise.
Changelogs have to be self explanatory and if the shortlog, aka
$subject, claims "bug" then there has to be a reasonable explanation
what the actual bug is.
Seriously.
All this is documented, but obviously documention for changelogs and the
acceptance of patches is just there to be ignored, right?
Thanks,
tglx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: + fix-mult_frac-multiple-argument-evaluation-bug.patch added to mm-nonmm-unstable branch
2023-05-22 23:45 ` Thomas Gleixner
@ 2023-05-23 8:48 ` Alexey Dobriyan
2023-05-23 10:41 ` Thomas Gleixner
0 siblings, 1 reply; 5+ messages in thread
From: Alexey Dobriyan @ 2023-05-23 8:48 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: linux-kernel, mm-commits, akpm
On Tue, May 23, 2023 at 01:45:40AM +0200, Thomas Gleixner wrote:
> On Mon, May 22 2023 at 14:15, Andrew Morton wrote:
> > ------------------------------------------------------
> > From: Alexey Dobriyan <adobriyan@gmail.com>
> > Subject: include/linux/math.h: fix mult_frac() multiple argument evaluation bug
> > Date: Sat, 20 May 2023 21:25:19 +0300
> >
> > mult_frac() evaluates _all_ arguments multiple times in the body.
>
> I'm not opposed to the patch, but to the description.
>
> Multiple evaluation is not a bug per se.
It is kind of a bug if a macro pretends to be a function and is spelled in
lowercase.
> Unless there is a reasonable explanation for the alleged bug this is
> just a cosmetic exercise.
Most usages looks OK, and compiler tend to merge loads so even more
usages are OK. But formally this is not OK:
static inline unsigned long vfs_pressure_ratio(unsigned long val)
{
return mult_frac(val, sysctl_vfs_cache_pressure, 100);
}
> Changelogs have to be self explanatory and if the shortlog, aka
> $subject, claims "bug" then there has to be a reasonable explanation
> what the actual bug is.
>
> Seriously.
>
> All this is documented, but obviously documention for changelogs and the
> acceptance of patches is just there to be ignored, right?
I don't want to return to kindergarten and document problem which every
C programmer learns exploring MIN(a, b).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: + fix-mult_frac-multiple-argument-evaluation-bug.patch added to mm-nonmm-unstable branch
2023-05-23 8:48 ` Alexey Dobriyan
@ 2023-05-23 10:41 ` Thomas Gleixner
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Gleixner @ 2023-05-23 10:41 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: linux-kernel, mm-commits, akpm
On Tue, May 23 2023 at 11:48, Alexey Dobriyan wrote:
> On Tue, May 23, 2023 at 01:45:40AM +0200, Thomas Gleixner wrote:
>> Changelogs have to be self explanatory and if the shortlog, aka
>> $subject, claims "bug" then there has to be a reasonable explanation
>> what the actual bug is.
>>
>> Seriously.
>>
>> All this is documented, but obviously documention for changelogs and the
>> acceptance of patches is just there to be ignored, right?
>
> I don't want to return to kindergarten and document problem which every
> C programmer learns exploring MIN(a, b).
A quick summary what the bug is, is _not_ kindergarten level.
Why does a reviewer have to do his own analysis at the patch level to
figure out what this solves and fixes?
It's 20 seconds of courtesy on the submitter side which saves a lot of
time on the reviewer and maintainer side.
Thanks,
tglx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-05-23 10:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-22 21:15 + fix-mult_frac-multiple-argument-evaluation-bug.patch added to mm-nonmm-unstable branch Andrew Morton
2023-05-22 23:45 ` Thomas Gleixner
2023-05-23 8:48 ` Alexey Dobriyan
2023-05-23 10:41 ` Thomas Gleixner
-- strict thread matches above, loose matches on Subject: below --
2023-05-19 20:53 Andrew Morton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.