Re: [PATCH] lib/kstrtox.c clean kstrtoll function
Dear Mr. Jeff, Thanks for the comments. Yes i think overflow check logic is wrong. So i think we can change the overflow logic - >From -- if ((long long)tmp < 0) + return -ERANGE; to - if (((long long)tmp < LLONG_MIN) || ((long long)tmp > LLONG_MAX) ) + return -ERANGE; Please give your views on this change.. If this change seems correct i will update overflow logic in my sent patch. Hope to hear from you soon. Thanks Anshul Garg On Thu, Jan 22, 2015 at 10:55 PM, Jeff Epler wrote: > On Thu, Jan 22, 2015 at 05:54:10AM -0800, Anshul Garg wrote: >> - if ((long long)(-tmp) >= 0) >> - return -ERANGE; >> - *res = -tmp; > ... >> + if ((long long)tmp < 0) >> + return -ERANGE; >> + *res = sign * tmp; > > I don't believe overflow handling is correct anymore with this patch. > Did you try with the input as the most negative possible unsigned long? > > Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] iio/inkern.c Use list_for_each_entry_safe
Use list_for_each_entry_safe instead of list_for_each_safe
and list_entry call.
Signed-off-by: Anshul Garg
---
drivers/iio/inkern.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c8bad3c..f764eb6 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -61,12 +61,10 @@ EXPORT_SYMBOL_GPL(iio_map_array_register);
int iio_map_array_unregister(struct iio_dev *indio_dev)
{
int ret = -ENODEV;
- struct iio_map_internal *mapi;
- struct list_head *pos, *tmp;
+ struct iio_map_internal *mapi, *next;
mutex_lock(&iio_map_list_lock);
- list_for_each_safe(pos, tmp, &iio_map_list) {
- mapi = list_entry(pos, struct iio_map_internal, l);
+ list_for_each_entry_safe(mapi, next, &iio_map_listi, list) {
if (indio_dev == mapi->indio_dev) {
list_del(&mapi->l);
kfree(mapi);
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] iio/inkern.c Use list_for_each_entry_safe
Use list_for_each_entry_safe instead of list_for_each_safe
and list_entry call.
Signed-off-by: Anshul Garg
---
drivers/iio/inkern.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c8bad3c..f764eb6 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -61,12 +61,10 @@ EXPORT_SYMBOL_GPL(iio_map_array_register);
int iio_map_array_unregister(struct iio_dev *indio_dev)
{
int ret = -ENODEV;
- struct iio_map_internal *mapi;
- struct list_head *pos, *tmp;
+ struct iio_map_internal *mapi, *next;
mutex_lock(&iio_map_list_lock);
- list_for_each_safe(pos, tmp, &iio_map_list) {
- mapi = list_entry(pos, struct iio_map_internal, l);
+ list_for_each_entry_safe(mapi, next, &iio_map_list, l) {
if (indio_dev == mapi->indio_dev) {
list_del(&mapi->l);
kfree(mapi);
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] lib/int_sqrt.c: Optimize square root function
Dear Mr. linus, Thanks for quick replies. Yes performance numbers are not conclusive enough. So its better to discard this patch as of now. I will try to explore more in this area. Thanks & regards Anshul Garg On Fri, Feb 6, 2015 at 1:07 AM, Linus Torvalds wrote: > On Thu, Feb 5, 2015 at 10:43 AM, Anshul Garg wrote: >> >> NOTE :: >> I have not used gcc optimizations while compilation. >> With O2 level optimization proposed solution is taking more time. > > The thing is, the kernel is compiled with -O2, so that's what matters. > > Also, for very tight loops like this, the major costs tend to be very > subtle microarchitectural details, particularly branch prediction. > Which in turn end up sometimes depending on just exactly where the > branches were placed, and even whether two conditional branches were > in the same 8-byte aligned region etc things (because the branch > prediction might be done ignoring the low bits of the EIP etc). So not > only does the exact microarchitecture matter, things that don't *seem* > like they should matter can change behavior a lot. > > My point is really that the performance numbers are very ambiguous. > The patch may well help in some situations, but hurt in others. > > Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] lib/int_sqrt.c: Optimize square root function
On Thu, Feb 5, 2015 at 10:33 AM, Linus Torvalds
wrote:
> On Thu, Feb 5, 2015 at 10:20 AM, Linus Torvalds
> wrote:
>>
>> Hmm. I did that too [..]
>
> Side note: one difference in our results (apart from possibly just CPU
> uarch details) is that my loop goes to 100M to make it easier to just
> time it. Which means that my load essentially had about three more
> iterations over nonzero data.
>
> Linus
I have also done the same testing on 100 million numbers.
Attaching source codes.
Below is the result ::
int_sqrt_old - current
int_sqrt_new - With proposed change
anshul@ubuntu:~/kernel_latest/sqrt$ time ./int_sqrt_old
real0m41.895s
user0m36.490s
sys0m0.365s
anshul@ubuntu:~/kernel_latest/sqrt$ time ./int_sqrt_new
real0m39.491s
user0m36.495s
sys0m0.338s
I have run this test on Intel(R) Core(TM) i3-4000M CPU @ 2.40GHz VMWare Machine.
Please check if i am doing anything wrong.
NOTE ::
I have not used gcc optimizations while compilation.
With O2 level optimization proposed solution is taking more time.
/*
* Copyright (C) 2013 Davidlohr Bueso
*
* Based on the shift-and-subtract algorithm for computing integer
* square root from Guy L. Steele.
*/
#include
/**
* int_sqrt - rough approximation to sqrt
* @x: integer of which to calculate the sqrt
*
* A very rough approximation to the sqrt() function.
*/
#define BITS_PER_LONG (8*sizeof(long))
unsigned long int_sqrt(unsigned long x)
{
unsigned long b, m, y = 0;
if (x <= 1)
return x;
m = 1UL << (BITS_PER_LONG - 2);
while (m != 0) {
b = y + m;
y >>= 1;
if (x >= b) {
x -= b;
y += m;
}
m >>= 2;
}
return y;
}
int main()
{
unsigned long n = 1;
for(;n<=1;++n)
int_sqrt(n);
return 0;
}
/*
* Copyright (C) 2013 Davidlohr Bueso
*
* Based on the shift-and-subtract algorithm for computing integer
* square root from Guy L. Steele.
*/
#include
#define BITS_PER_LONG (8*sizeof(long))
/**
* int_sqrt - rough approximation to sqrt
* @x: integer of which to calculate the sqrt
*
* A very rough approximation to the sqrt() function.
*/
unsigned long int_sqrt(unsigned long x)
{
unsigned long b, m, y = 0;
if (x <= 1)
return x;
m = 1UL << (BITS_PER_LONG - 2);
while(m > x )
m >>= 2;
while (m != 0) {
b = y + m;
y >>= 1;
if (x >= b) {
x -= b;
y += m;
}
m >>= 2;
}
return y;
}
int main()
{
unsigned long n = 1;
for(;n<=1;++n)
int_sqrt(n);
return 0;
}
[PATCH] lib/int_sqrt.c: Optimize square root function
From: Anshul Garg
Unnecessary instructions are executing even though m is
greater than x so added logic to make m less than equal to
x before performing these operations.
Signed-off-by: Anshul Garg
---
lib/int_sqrt.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/int_sqrt.c b/lib/int_sqrt.c
index 1ef4cc3..64ae722 100644
--- a/lib/int_sqrt.c
+++ b/lib/int_sqrt.c
@@ -22,6 +22,9 @@ unsigned long int_sqrt(unsigned long x)
return x;
m = 1UL << (BITS_PER_LONG - 2);
+
+ while (m > x)
+ m >>= 2;
while (m != 0) {
b = y + m;
y >>= 1;
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] lib/int_sqrt.c: Optimize square root function
On Tue, Feb 3, 2015 at 10:17 AM, Davidlohr Bueso wrote: > On Mon, 2015-02-02 at 11:00 -0800, Linus Torvalds wrote: >> Hmm. I don't disagree, but would like some more feedback. >> >> Davidlohr - you were the person to touch this function last (commit >> 30493cc9dddb: "lib/int_sqrt.c: optimize square root algorithm"), and >> you did so for performance reasons. And in fact, when you did that, >> you removed that initial loop: >> >> - one = 1UL << (BITS_PER_LONG - 2); >> - while (one > op) >> - one >>= 2; >> >> but I'm not sure that was actually all that conscious, I think the >> real optimization was the changes inside the loop to make the final >> real loop faster and simpler. > > I missed that. And yes, the real optimization should be in the loop. > >> >> Also, you had performance numbers, so presumably a test harness for it >> all. It probably depends a lot on the actual distribution of argument >> values, of course, but it would be good to accompany the patch with >> actual real numbers like lasty time. > > Aha. In my case I recall I ran a usersapce program using each function > from 1 to a million, and throwing perf at it for 10 times. > I have done profiling of int_sqrt function using perf tool for 10 times. For this purpose i have created a userspace program which uses sqrt function from 1 to a million. int_sqrt_old -> current algorithm version int_sqrt_new -> with proposed change these results are for BITS_PER_LONG=64. Performance counter stats for './int_sqrt_old' (10 runs): 460.944061 task-clock (msec) #0.969 CPUs utilized ( +- 1.72% ) 64 context-switches #0.139 K/sec ( +- 2.27% ) 0 cpu-migrations#0.000 K/sec 132 page-faults #0.286 K/sec cycles stalled-cycles-frontend stalled-cycles-backend instructions branches branch-misses 0.475795341 seconds time elapsed( +- 3.20% ) Performance counter stats for './int_sqrt_new' (10 runs): 401.782119 task-clock (msec) #0.974 CPUs utilized( +- 1.55% ) 57 context-switches #0.141 K/sec( +- 1.92% ) 0 cpu-migrations#0.000 K/sec 132 page-faults #0.329 K/sec cycles stalled-cycles-frontend stalled-cycles-backend instructions branches branch-misses 0.412593296 seconds time elapsed( +- 2.03% ) As per profiling definitely there is improvement in algorithm timing. >> (I'm also not entirely sure what uses int_sqrt() that ends up being so >> performance-critical, so it would be good to document that too, since >> that probably also matters for the "what's the normal argument range" >> question..) > > It's not a big deal afaik. > > Thanks, > Davidlohr > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] lib/int_sqrt.c: Optimize square root function
On Tue, Feb 3, 2015 at 10:11 AM, Davidlohr Bueso wrote:
> On Mon, 2015-02-02 at 11:13 -0800, Linus Torvalds wrote:
>> IOW, instead of
>>
>> m = 1UL << (BITS_PER_LONG - 2);
>>
>> perhaps something like
>>
>>m = 1UL << (BITS_PER_LONG/2- 2);
>>if (m < x)
>>m <<= BITS_PER_LONG/2;
On doing this change also extra instruction(shown below) will execute
if x is small(say 1).
b = y + m;
y >>= 1;
if (x >= b) {
Although i understand your point that we can set value of m according
to range of x.
But in future also range of x can change if apart from memory some
other module starts
using this function more extensively.
So in worst case it will be almost same as current implementation.
So i think its better to scale m according to x by doing
while(m > x)
m>>=2;
rather than according to range of x.
Thanks
Anshul Garg
>>
>> (assuming gcc can change that code into a "cmov") might cut down the
>> "lots of empty loops" case in half for small values of 'x'.
>
> Makes a lot of sense.
>
>> There's probably some other better cheap initial guess value estimator.
>
> Just to get a feeling for the normal arg range in the non-driver parts
> that use this thing:
>
> fs/ceph/super.h:congestion_kb = (16*int_sqrt(totalram_pages)) <<
> (PAGE_SHIFT-10);
> fs/nfs/write.c: nfs_congestion_kb = (16*int_sqrt(totalram_pages)) <<
> (PAGE_SHIFT-10);
> fs/nfsd/nfscache.c: limit = (16 * int_sqrt(low_pages)) << (PAGE_SHIFT-10);
> kernel/rcu/tree_plugin.h: ls = int_sqrt(nr_cpu_ids);
> mm/memcontrol.c:inactive_ratio = int_sqrt(10 * gb);
> mm/page_alloc.c:ratio = int_sqrt(10 * gb);
> mm/page_alloc.c:new_min_free_kbytes = int_sqrt(lowmem_kbytes * 16);
>
> So mostly values that scale according to mem or cpu.
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] lib/int_sqrt.c: Optimize square root function
From: Anshul Garg
Unnecessary instructions are executing even though m is
greater than x so added logic to make m less than equal to
x before performing these operations.
Signed-off-by: Anshul Garg
---
lib/int_sqrt.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/int_sqrt.c b/lib/int_sqrt.c
index 1ef4cc3..64ae722 100644
--- a/lib/int_sqrt.c
+++ b/lib/int_sqrt.c
@@ -22,6 +22,9 @@ unsigned long int_sqrt(unsigned long x)
return x;
m = 1UL << (BITS_PER_LONG - 2);
+
+ while (m > x)
+ m >>= 2;
while (m != 0) {
b = y + m;
y >>= 1;
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RESEND PATCH] lib/int_sqrt.c: Optimize square root function
From: Anshul Garg
Unnecessary instructions are executing even though m is
greater than x so added logic to make m less than equal to
x before performing these operations.
Signed-off-by: Anshul Garg
---
lib/int_sqrt.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/int_sqrt.c b/lib/int_sqrt.c
index 1ef4cc3..64ae722 100644
--- a/lib/int_sqrt.c
+++ b/lib/int_sqrt.c
@@ -22,6 +22,9 @@ unsigned long int_sqrt(unsigned long x)
return x;
m = 1UL << (BITS_PER_LONG - 2);
+
+ while (m > x)
+ m >>= 2;
while (m != 0) {
b = y + m;
y >>= 1;
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] lib/kstrtox.c break if overflow is detected
From: Anshul Garg
1. While converting string representation to integer
break the loop if overflow is detected.
2. Clean kstrtoll function
Signed-off-by: Anshul Garg
---
lib/kstrtox.c | 28 +---
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index ec8da78..8cbe5ca 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -70,8 +70,10 @@ unsigned int _parse_integer(const char *s, unsigned int
base, unsigned long long
* it in the max base we support (16)
*/
if (unlikely(res & (~0ull << 60))) {
- if (res > div_u64(ULLONG_MAX - val, base))
+ if (res > div_u64(ULLONG_MAX - val, base)) {
overflow = 1;
+ break;
+ }
}
res = res * base + val;
rv++;
@@ -146,23 +148,19 @@ EXPORT_SYMBOL(kstrtoull);
int kstrtoll(const char *s, unsigned int base, long long *res)
{
unsigned long long tmp;
- int rv;
+ int rv, sign = 1;
if (s[0] == '-') {
- rv = _kstrtoull(s + 1, base, &tmp);
- if (rv < 0)
- return rv;
- if ((long long)(-tmp) >= 0)
- return -ERANGE;
- *res = -tmp;
- } else {
- rv = kstrtoull(s, base, &tmp);
- if (rv < 0)
- return rv;
- if ((long long)tmp < 0)
- return -ERANGE;
- *res = tmp;
+ sign = -1;
+ s++;
}
+
+ rv = kstrtoull(s, base, &tmp);
+ if (rv < 0)
+ return rv;
+ if ((long long)tmp < 0)
+ return -ERANGE;
+ *res = sign * tmp;
return 0;
}
EXPORT_SYMBOL(kstrtoll);
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] lib/kstrtox.c break if overflow is detected
Dear Mr. Levente ,
Thanks for the reply.
I will segregate this patch in two different patches.
I will send these patches again.
Thanks
Anshul Garg
On Thu, Jan 22, 2015 at 1:09 AM, Levente Kurusa wrote:
> Hi,
>
> On Wed, Jan 21, 2015 at 11:26:26AM -0800, Anshul Garg wrote:
>> From: Anshul Garg
>>
>> 1. While converting string representation to integer
>> break the loop if overflow is detected.
>> 2. Clean kstrtoll function
>>
>> Signed-off-by: Anshul Garg
>> ---
>> lib/kstrtox.c | 28 +---
>> 1 file changed, 13 insertions(+), 15 deletions(-)
>>
>> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
>> index ec8da78..8cbe5ca 100644
>> --- a/lib/kstrtox.c
>> +++ b/lib/kstrtox.c
>> @@ -70,8 +70,10 @@ unsigned int _parse_integer(const char *s, unsigned int
>> base, unsigned long long
>>* it in the max base we support (16)
>>*/
>> if (unlikely(res & (~0ull << 60))) {
>> - if (res > div_u64(ULLONG_MAX - val, base))
>> + if (res > div_u64(ULLONG_MAX - val, base)) {
>> overflow = 1;
>> + break;
>> + }
>> }
>> res = res * base + val;
>> rv++;
>> @@ -146,23 +148,19 @@ EXPORT_SYMBOL(kstrtoull);
>> int kstrtoll(const char *s, unsigned int base, long long *res)
>> {
>> unsigned long long tmp;
>> - int rv;
>> + int rv, sign = 1;
>>
>> if (s[0] == '-') {
>> - rv = _kstrtoull(s + 1, base, &tmp);
>> - if (rv < 0)
>> - return rv;
>> - if ((long long)(-tmp) >= 0)
>> - return -ERANGE;
>> - *res = -tmp;
>> - } else {
>> - rv = kstrtoull(s, base, &tmp);
>> - if (rv < 0)
>> - return rv;
>> - if ((long long)tmp < 0)
>> - return -ERANGE;
>> - *res = tmp;
>> + sign = -1;
>> + s++;
>> }
>> +
>> + rv = kstrtoull(s, base, &tmp);
>> + if (rv < 0)
>> + return rv;
>> + if ((long long)tmp < 0)
>> + return -ERANGE;
>> + *res = sign * tmp;
>> return 0;
>> }
>> EXPORT_SYMBOL(kstrtoll);
>
> Looks correct to me, so:
>
> Reviewed-by: Levente Kurusa
>
> But I believe the two hunks are completely unrelated to
> eachother and hence should split into a patch series.
>
> Could you please do that and resend? Or if Andrew is OK
> with picking it up as is, then it's fine.
>
> Thanks,
> Levente.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] lib/kstrtox.c clean kstrtoll function
From: Anshul Garg
Instead of having same code for negative and postive
integer, use sign variable for integer parsing.
Signed-off-by: Anshul Garg
---
lib/kstrtox.c | 24 ++--
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index ec8da78..744cbb0 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -146,23 +146,19 @@ EXPORT_SYMBOL(kstrtoull);
int kstrtoll(const char *s, unsigned int base, long long *res)
{
unsigned long long tmp;
- int rv;
+ int rv, sign = 1;
if (s[0] == '-') {
- rv = _kstrtoull(s + 1, base, &tmp);
- if (rv < 0)
- return rv;
- if ((long long)(-tmp) >= 0)
- return -ERANGE;
- *res = -tmp;
- } else {
- rv = kstrtoull(s, base, &tmp);
- if (rv < 0)
- return rv;
- if ((long long)tmp < 0)
- return -ERANGE;
- *res = tmp;
+ sign = -1;
+ s++;
}
+
+ rv = kstrtoull(s, base, &tmp);
+ if (rv < 0)
+ return rv;
+ if ((long long)tmp < 0)
+ return -ERANGE;
+ *res = sign * tmp;
return 0;
}
EXPORT_SYMBOL(kstrtoll);
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] lib/kstrtox.c Stop parsing integer on overflow
From: Anshul Garg
While converting string representation to integer
break the loop if overflow is detected.
Signed-off-by: Anshul Garg
---
lib/kstrtox.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index ec8da78..6f30209 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -70,8 +70,10 @@ unsigned int _parse_integer(const char *s, unsigned int
base, unsigned long long
* it in the max base we support (16)
*/
if (unlikely(res & (~0ull << 60))) {
- if (res > div_u64(ULLONG_MAX - val, base))
+ if (res > div_u64(ULLONG_MAX - val, base)) {
overflow = 1;
+ break;
+ }
}
res = res * base + val;
rv++;
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Fw: [PATCH] lib/kstrtox.c Stop parsing integer on overflow
On Wed, Feb 18, 2015 at 1:14 PM, Alexey Dobriyan wrote:
> On Tue, Feb 17, 2015 at 04:17:24PM -0800, Andrew Morton wrote:
>> ?
>>
>> Begin forwarded message:
>>
>> Date: Mon, 16 Feb 2015 10:48:50 -0800
>> From: Anshul Garg
>> To: linux-kernel@vger.kernel.org
>> Cc: aksgarg1...@gmail.com, anshu...@samsung.com,
>> torva...@linux-foundation.org
>> Subject: [PATCH] lib/kstrtox.c Stop parsing integer on overflow
>>
>>
>> From: Anshul Garg
>>
>> While converting string representation to integer
>> break the loop if overflow is detected.
>>
>> Signed-off-by: Anshul Garg
>> ---
>> lib/kstrtox.c |4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
>> index ec8da78..6f30209 100644
>> --- a/lib/kstrtox.c
>> +++ b/lib/kstrtox.c
>> @@ -70,8 +70,10 @@ unsigned int _parse_integer(const char *s, unsigned int
>> base, unsigned long long
>>* it in the max base we support (16)
>>*/
>> if (unlikely(res & (~0ull << 60))) {
>> - if (res > div_u64(ULLONG_MAX - val, base))
>> + if (res > div_u64(ULLONG_MAX - val, base)) {
>> overflow = 1;
>> + break;
>> + }
>> }
>> res = res * base + val;
>> rv++;
>
> The _notion_ of a patch is OK if you want EVERY simple_strtoull() call
> to stop parsing past overflow right now. It SHOULD have done so from day 1,
> but it doesn't do that.
>
> When I wrote kstrto*() code I deliberatedly didn't break this bug
> because of the sheer number of call sites.
>
> If you are OK with changing bug-for-bug compatibility,
> then patch simply need to delete overflow detection code.
>
> Alexey
I think this patch won't break any existing module using this function.
because this function sets KSTRTOX_OVERFLOW as error status.
which is checked by calling function to determine whether value is correct
or not.
If this flag is set we can simply discard the parsed value.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] lib/kstrtox.c Stop parsing integer on overflow
From: Anshul Garg
While converting string representation to integer
break the loop if overflow is detected.
Signed-off-by: Anshul Garg
---
lib/kstrtox.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index ec8da78..6f30209 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -70,8 +70,10 @@ unsigned int _parse_integer(const char *s, unsigned int
base, unsigned long long
* it in the max base we support (16)
*/
if (unlikely(res & (~0ull << 60))) {
- if (res > div_u64(ULLONG_MAX - val, base))
+ if (res > div_u64(ULLONG_MAX - val, base)) {
overflow = 1;
+ break;
+ }
}
res = res * base + val;
rv++;
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] lib/vsprintf.c:Avoid extra operation in dentry_name
From: Anshul Garg
Remove unnecessary increment and decrement operation
in dentry_name function as after increment operation
loop is breaked and then decrement operation is
performed. So remove increment and decrement operation
from the function.
Signed-off-by: Anshul Garg
---
lib/vsprintf.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index ec337f6..2a38105 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -576,11 +576,10 @@ char *dentry_name(char *buf, char *end, const struct
dentry *d, struct printf_sp
if (p == d) {
if (i)
array[i] = "";
- i++;
break;
}
}
- s = array[--i];
+ s = array[i];
for (n = 0; n != spec.precision; n++, buf++) {
char c = *s++;
if (!c) {
--
1.7.9.5
---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] lib/vsprintf.c:Avoid extra operation in dentry_name
Yes suggested code modification will break if (p==d) branch is not taken.
Thanks for pointing out this point.
So its better to keep code as it is.
On Tue, Feb 17, 2015 at 12:44 AM, Richard Weinberger
wrote:
> On Mon, Feb 16, 2015 at 7:49 PM, Anshul Garg wrote:
>> From: Anshul Garg
>>
>> Remove unnecessary increment and decrement operation
>> in dentry_name function as after increment operation
>> loop is breaked and then decrement operation is
>> performed. So remove increment and decrement operation
>> from the function.
>>
>> Signed-off-by: Anshul Garg
>> ---
>> lib/vsprintf.c |3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>> index ec337f6..2a38105 100644
>> --- a/lib/vsprintf.c
>> +++ b/lib/vsprintf.c
>> @@ -576,11 +576,10 @@ char *dentry_name(char *buf, char *end, const struct
>> dentry *d, struct printf_sp
>> if (p == d) {
>> if (i)
>> array[i] = "";
>> - i++;
>> break;
>> }
>> }
>> - s = array[--i];
>> + s = array[i];
>> for (n = 0; n != spec.precision; n++, buf++) {
>> char c = *s++;
>> if (!c) {
>
> What if the if (d == d) branch is not taken?
> Does the code then really behave exactly as before?
>
> --
> Thanks,
> //richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Sched/rt:Fix memory leak in alloc_rt_sched_group
From: Anshul Garg Added code to free allocated memory by function alloc_rt_sched_group in case kzalloc api fails. Signed-off-by: Anshul Garg --- kernel/sched/rt.c |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index f4d4b07..47213e9 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -177,7 +177,7 @@ int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent) goto err; tg->rt_se = kzalloc(sizeof(rt_se) * nr_cpu_ids, GFP_KERNEL); if (!tg->rt_se) - goto err; + goto err_free_rt_rq; init_rt_bandwidth(&tg->rt_bandwidth, ktime_to_ns(def_rt_bandwidth.rt_period), 0); @@ -186,7 +186,7 @@ int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent) rt_rq = kzalloc_node(sizeof(struct rt_rq), GFP_KERNEL, cpu_to_node(i)); if (!rt_rq) - goto err; + goto err_free_rt_se; rt_se = kzalloc_node(sizeof(struct sched_rt_entity), GFP_KERNEL, cpu_to_node(i)); @@ -202,6 +202,10 @@ int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent) err_free_rq: kfree(rt_rq); +err_free_rt_se: + kfree(tg->rt_se); +err_free_rt_rq: + kfree(tg->rt_rq); err: return 0; } -- 1.7.9.5 --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [tip:sched/core] sched/rt: Fix memory leak in alloc_rt_sched_group()
Dear linux community, I think this patch" sched/rt: Fix memory leak in alloc_rt_sched_group()" with Commit-ID: ecdd6804b7c9e15fe8fc836ba0233d9912834e8b is not correct. As it will create panic in case of kzalloc failure because of doingkfree twice. As if alloc_rt_sched_group fails it will return zero then core.c will call free_sched_group in core.c which subsequently calls free_rt_sched_group . In free_rt_sched_group memory is getting freed properly. so it seems patch sent by me is not correct. Please help to review patch once again as it might break some things. Sorry for inconvenience. Thanks Anshul Garg On Mon, Jul 6, 2015 at 9:05 PM, tip-bot for Anshul Garg wrote: > Commit-ID: ecdd6804b7c9e15fe8fc836ba0233d9912834e8b > Gitweb: http://git.kernel.org/tip/ecdd6804b7c9e15fe8fc836ba0233d9912834e8b > Author: Anshul Garg > AuthorDate: Wed, 1 Jul 2015 12:29:26 -0700 > Committer: Ingo Molnar > CommitDate: Mon, 6 Jul 2015 14:17:14 +0200 > > sched/rt: Fix memory leak in alloc_rt_sched_group() > > Added code to free allocated memory by function > alloc_rt_sched_group() in case the kzalloc() API fails. > > Signed-off-by: Anshul Garg > Signed-off-by: Peter Zijlstra (Intel) > Cc: Linus Torvalds > Cc: Peter Zijlstra > Cc: Thomas Gleixner > Link: > http://lkml.kernel.org/r/1435778966-36415-1-git-send-email-aksgarg1...@gmail.com > Signed-off-by: Ingo Molnar > --- > kernel/sched/rt.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c > index 0d193a24..cfa907d 100644 > --- a/kernel/sched/rt.c > +++ b/kernel/sched/rt.c > @@ -193,7 +193,7 @@ int alloc_rt_sched_group(struct task_group *tg, struct > task_group *parent) > goto err; > tg->rt_se = kzalloc(sizeof(rt_se) * nr_cpu_ids, GFP_KERNEL); > if (!tg->rt_se) > - goto err; > + goto err_free_rt_rq; > > init_rt_bandwidth(&tg->rt_bandwidth, > ktime_to_ns(def_rt_bandwidth.rt_period), 0); > @@ -202,7 +202,7 @@ int alloc_rt_sched_group(struct task_group *tg, struct > task_group *parent) > rt_rq = kzalloc_node(sizeof(struct rt_rq), > GFP_KERNEL, cpu_to_node(i)); > if (!rt_rq) > - goto err; > + goto err_free_rt_se; > > rt_se = kzalloc_node(sizeof(struct sched_rt_entity), > GFP_KERNEL, cpu_to_node(i)); > @@ -218,6 +218,10 @@ int alloc_rt_sched_group(struct task_group *tg, struct > task_group *parent) > > err_free_rq: > kfree(rt_rq); > +err_free_rt_se: > + kfree(tg->rt_se); > +err_free_rt_rq: > + kfree(tg->rt_rq); > err: > return 0; > } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[tip:sched/core] sched/rt: Fix memory leak in alloc_rt_sched_group()
Commit-ID: ecdd6804b7c9e15fe8fc836ba0233d9912834e8b Gitweb: http://git.kernel.org/tip/ecdd6804b7c9e15fe8fc836ba0233d9912834e8b Author: Anshul Garg AuthorDate: Wed, 1 Jul 2015 12:29:26 -0700 Committer: Ingo Molnar CommitDate: Mon, 6 Jul 2015 14:17:14 +0200 sched/rt: Fix memory leak in alloc_rt_sched_group() Added code to free allocated memory by function alloc_rt_sched_group() in case the kzalloc() API fails. Signed-off-by: Anshul Garg Signed-off-by: Peter Zijlstra (Intel) Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/1435778966-36415-1-git-send-email-aksgarg1...@gmail.com Signed-off-by: Ingo Molnar --- kernel/sched/rt.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 0d193a24..cfa907d 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -193,7 +193,7 @@ int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent) goto err; tg->rt_se = kzalloc(sizeof(rt_se) * nr_cpu_ids, GFP_KERNEL); if (!tg->rt_se) - goto err; + goto err_free_rt_rq; init_rt_bandwidth(&tg->rt_bandwidth, ktime_to_ns(def_rt_bandwidth.rt_period), 0); @@ -202,7 +202,7 @@ int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent) rt_rq = kzalloc_node(sizeof(struct rt_rq), GFP_KERNEL, cpu_to_node(i)); if (!rt_rq) - goto err; + goto err_free_rt_se; rt_se = kzalloc_node(sizeof(struct sched_rt_entity), GFP_KERNEL, cpu_to_node(i)); @@ -218,6 +218,10 @@ int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent) err_free_rq: kfree(rt_rq); +err_free_rt_se: + kfree(tg->rt_se); +err_free_rt_rq: + kfree(tg->rt_rq); err: return 0; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
