* [PATCH 1/2] lib/lcm.c: Ensure correct result whenever it fits
@ 2013-12-17 14:46 Rasmus Villemoes
2013-12-17 14:46 ` [PATCH 2/2] lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not n Rasmus Villemoes
2014-01-14 11:43 ` [PATCH 1/2] lib/lcm.c: Ensure correct result whenever it fits Rasmus Villemoes
0 siblings, 2 replies; 7+ messages in thread
From: Rasmus Villemoes @ 2013-12-17 14:46 UTC (permalink / raw)
To: linux-kernel; +Cc: Rasmus Villemoes
Ensure that lcm(a,b) returns the mathematically correct result,
provided it fits in an unsigned long. The current version returns
garbage if a*b overflows, even if the final result would fit.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
There are of course still plenty of cases where the return value is
wrong.
lib/lcm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/lcm.c b/lib/lcm.c
index b9c8de4..01b3aa9 100644
--- a/lib/lcm.c
+++ b/lib/lcm.c
@@ -7,7 +7,7 @@
unsigned long lcm(unsigned long a, unsigned long b)
{
if (a && b)
- return (a * b) / gcd(a, b);
+ return (a / gcd(a, b)) * b;
else if (b)
return b;
--
1.8.4.rc3.2.g61bff3f
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not n
2013-12-17 14:46 [PATCH 1/2] lib/lcm.c: Ensure correct result whenever it fits Rasmus Villemoes
@ 2013-12-17 14:46 ` Rasmus Villemoes
2014-01-14 11:43 ` [PATCH 1/2] lib/lcm.c: Ensure correct result whenever it fits Rasmus Villemoes
1 sibling, 0 replies; 7+ messages in thread
From: Rasmus Villemoes @ 2013-12-17 14:46 UTC (permalink / raw)
To: linux-kernel; +Cc: Rasmus Villemoes
Return the mathematically correct answer when an argument is 0.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
I don't think there is any instance of lcm(0,n) in the kernel, but at
least this reduces code size a little.
lib/lcm.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/lcm.c b/lib/lcm.c
index 01b3aa9..51cc6b1 100644
--- a/lib/lcm.c
+++ b/lib/lcm.c
@@ -8,9 +8,7 @@ unsigned long lcm(unsigned long a, unsigned long b)
{
if (a && b)
return (a / gcd(a, b)) * b;
- else if (b)
- return b;
-
- return a;
+ else
+ return 0;
}
EXPORT_SYMBOL_GPL(lcm);
--
1.8.4.rc3.2.g61bff3f
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] lib/lcm.c: Ensure correct result whenever it fits
2013-12-17 14:46 [PATCH 1/2] lib/lcm.c: Ensure correct result whenever it fits Rasmus Villemoes
2013-12-17 14:46 ` [PATCH 2/2] lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not n Rasmus Villemoes
@ 2014-01-14 11:43 ` Rasmus Villemoes
1 sibling, 0 replies; 7+ messages in thread
From: Rasmus Villemoes @ 2014-01-14 11:43 UTC (permalink / raw)
To: linux-kernel
Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:
Ping...
> Ensure that lcm(a,b) returns the mathematically correct result,
> provided it fits in an unsigned long. The current version returns
> garbage if a*b overflows, even if the final result would fit.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> There are of course still plenty of cases where the return value is
> wrong.
>
> lib/lcm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/lcm.c b/lib/lcm.c
> index b9c8de4..01b3aa9 100644
> --- a/lib/lcm.c
> +++ b/lib/lcm.c
> @@ -7,7 +7,7 @@
> unsigned long lcm(unsigned long a, unsigned long b)
> {
> if (a && b)
> - return (a * b) / gcd(a, b);
> + return (a / gcd(a, b)) * b;
> else if (b)
> return b;
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] lib/lcm.c: Ensure correct result whenever it fits
@ 2014-12-09 21:03 Rasmus Villemoes
2014-12-09 21:03 ` [PATCH 2/2] lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not n Rasmus Villemoes
0 siblings, 1 reply; 7+ messages in thread
From: Rasmus Villemoes @ 2014-12-09 21:03 UTC (permalink / raw)
To: Andrew Morton; +Cc: Rasmus Villemoes, linux-kernel
Ensure that lcm(a,b) returns the mathematically correct result,
provided it fits in an unsigned long. The current version returns
garbage if a*b overflows, even if the final result would fit.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
lib/lcm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/lcm.c b/lib/lcm.c
index b9c8de461e9e..01b3aa922dda 100644
--- a/lib/lcm.c
+++ b/lib/lcm.c
@@ -7,7 +7,7 @@
unsigned long lcm(unsigned long a, unsigned long b)
{
if (a && b)
- return (a * b) / gcd(a, b);
+ return (a / gcd(a, b)) * b;
else if (b)
return b;
--
2.1.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not n
2014-12-09 21:03 Rasmus Villemoes
@ 2014-12-09 21:03 ` Rasmus Villemoes
2015-03-29 2:44 ` Mike Snitzer
0 siblings, 1 reply; 7+ messages in thread
From: Rasmus Villemoes @ 2014-12-09 21:03 UTC (permalink / raw)
To: Andrew Morton; +Cc: Rasmus Villemoes, linux-kernel
Return the mathematically correct answer when an argument is 0.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
lib/lcm.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/lcm.c b/lib/lcm.c
index 01b3aa922dda..51cc6b13cd52 100644
--- a/lib/lcm.c
+++ b/lib/lcm.c
@@ -8,9 +8,7 @@ unsigned long lcm(unsigned long a, unsigned long b)
{
if (a && b)
return (a / gcd(a, b)) * b;
- else if (b)
- return b;
-
- return a;
+ else
+ return 0;
}
EXPORT_SYMBOL_GPL(lcm);
--
2.1.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not n
2014-12-09 21:03 ` [PATCH 2/2] lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not n Rasmus Villemoes
@ 2015-03-29 2:44 ` Mike Snitzer
2015-03-30 19:38 ` Rasmus Villemoes
0 siblings, 1 reply; 7+ messages in thread
From: Mike Snitzer @ 2015-03-29 2:44 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Andrew Morton, linux-kernel, Martin K. Petersen,
device-mapper development, Jens Axboe
On Tue, Dec 9, 2014 at 4:03 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> Return the mathematically correct answer when an argument is 0.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> lib/lcm.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/lib/lcm.c b/lib/lcm.c
> index 01b3aa922dda..51cc6b13cd52 100644
> --- a/lib/lcm.c
> +++ b/lib/lcm.c
> @@ -8,9 +8,7 @@ unsigned long lcm(unsigned long a, unsigned long b)
> {
> if (a && b)
> return (a / gcd(a, b)) * b;
> - else if (b)
> - return b;
> -
> - return a;
> + else
> + return 0;
> }
> EXPORT_SYMBOL_GPL(lcm);
This change is the source of 3.19 regression for stacking device
limits, via commit 69c953c ("lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not
n").
Test:
# modprobe scsi_debug dev_size_mb=10 num_tgts=1 opt_blks=1536
# cat /sys/block/sde/queue/optimal_io_size
786432
# dmsetup create node --table "0 100 linear /dev/sde 0"
Before commit 69c953c:
# cat /sys/block/dm-5/queue/optimal_io_size
786432
After commit 69c953c:
# cat /sys/block/dm-5/queue/optimal_io_size
0
Rasmus, mathematical purity of lcm() aside, it'd have been nice if you
looked at the lcm() callers to determine whether you'd be breaking
them.
It looks like we need a new lcm_not_zero() and blk_stack_limits()
should be changed to use it, and the patch needs to cc stable. Martin
and/or Jens, what do you think?
Mike
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not n
2015-03-29 2:44 ` Mike Snitzer
@ 2015-03-30 19:38 ` Rasmus Villemoes
0 siblings, 0 replies; 7+ messages in thread
From: Rasmus Villemoes @ 2015-03-30 19:38 UTC (permalink / raw)
To: Mike Snitzer
Cc: Andrew Morton, linux-kernel, Martin K. Petersen,
device-mapper development, Jens Axboe
On Sun, Mar 29 2015, Mike Snitzer <snitzer@redhat.com> wrote:
> On Tue, Dec 9, 2014 at 4:03 PM, Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
>> Return the mathematically correct answer when an argument is 0.
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>
> This change is the source of 3.19 regression for stacking device
> limits, via commit 69c953c ("lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not
> n").
>
>
> Rasmus, mathematical purity of lcm() aside, it'd have been nice if you
> looked at the lcm() callers to determine whether you'd be breaking
> them.
I'm sorry about this. I thought I did check the callers, but evidently
not well enough.
Rasmus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not n
@ 2015-03-30 19:38 ` Rasmus Villemoes
0 siblings, 0 replies; 7+ messages in thread
From: Rasmus Villemoes @ 2015-03-30 19:38 UTC (permalink / raw)
To: Mike Snitzer
Cc: Andrew Morton, linux-kernel, Martin K. Petersen,
device-mapper development, Jens Axboe
On Sun, Mar 29 2015, Mike Snitzer <snitzer@redhat.com> wrote:
> On Tue, Dec 9, 2014 at 4:03 PM, Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
>> Return the mathematically correct answer when an argument is 0.
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>
> This change is the source of 3.19 regression for stacking device
> limits, via commit 69c953c ("lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not
> n").
>
>
> Rasmus, mathematical purity of lcm() aside, it'd have been nice if you
> looked at the lcm() callers to determine whether you'd be breaking
> them.
I'm sorry about this. I thought I did check the callers, but evidently
not well enough.
Rasmus
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-03-30 19:38 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-17 14:46 [PATCH 1/2] lib/lcm.c: Ensure correct result whenever it fits Rasmus Villemoes
2013-12-17 14:46 ` [PATCH 2/2] lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not n Rasmus Villemoes
2014-01-14 11:43 ` [PATCH 1/2] lib/lcm.c: Ensure correct result whenever it fits Rasmus Villemoes
2014-12-09 21:03 Rasmus Villemoes
2014-12-09 21:03 ` [PATCH 2/2] lib/lcm.c: lcm(n,0)=lcm(0,n) is 0, not n Rasmus Villemoes
2015-03-29 2:44 ` Mike Snitzer
2015-03-30 19:38 ` Rasmus Villemoes
2015-03-30 19:38 ` Rasmus Villemoes
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.