linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] memtest cleanups
@ 2015-06-19 14:58 Vladimir Murzin
  2015-06-19 14:58 ` [PATCH 1/3] memtest: use kstrtouint instead of simple_strtoul Vladimir Murzin
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Vladimir Murzin @ 2015-06-19 14:58 UTC (permalink / raw)
  To: linux-mm; +Cc: akpm

Hi,

This patch set does simple cleanup in mm/memtest.c code.

There is no changes in functionality, but some logging may slightly differ
after patch 2/3 is applied.

It was generated against 4.1-rc8

Thanks!

Vladimir Murzin (3):
  memtest: use kstrtouint instead of simple_strtoul
  memtest: cleanup log messages
  memtest: remove unused header files

 mm/memtest.c |   33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] memtest: use kstrtouint instead of simple_strtoul
  2015-06-19 14:58 [PATCH 0/3] memtest cleanups Vladimir Murzin
@ 2015-06-19 14:58 ` Vladimir Murzin
  2015-06-20  6:55   ` Leon Romanovsky
  2015-06-19 14:58 ` [PATCH 2/3] memtest: cleanup log messages Vladimir Murzin
  2015-06-19 14:58 ` [PATCH 3/3] memtest: remove unused header files Vladimir Murzin
  2 siblings, 1 reply; 10+ messages in thread
From: Vladimir Murzin @ 2015-06-19 14:58 UTC (permalink / raw)
  To: linux-mm; +Cc: akpm

Since simple_strtoul is obsolete and memtest_pattern is type of int, use
kstrtouint instead.

Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
---
 mm/memtest.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/mm/memtest.c b/mm/memtest.c
index 1997d93..895a43c 100644
--- a/mm/memtest.c
+++ b/mm/memtest.c
@@ -88,14 +88,18 @@ static void __init do_one_pass(u64 pattern, phys_addr_t start, phys_addr_t end)
 }
 
 /* default is disabled */
-static int memtest_pattern __initdata;
+static unsigned int memtest_pattern __initdata;
 
 static int __init parse_memtest(char *arg)
 {
-	if (arg)
-		memtest_pattern = simple_strtoul(arg, NULL, 0);
-	else
-		memtest_pattern = ARRAY_SIZE(patterns);
+	if (arg) {
+		int err = kstrtouint(arg, 0, &memtest_pattern);
+
+		if (!err)
+			return 0;
+	}
+
+	memtest_pattern = ARRAY_SIZE(patterns);
 
 	return 0;
 }
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/3] memtest: cleanup log messages
  2015-06-19 14:58 [PATCH 0/3] memtest cleanups Vladimir Murzin
  2015-06-19 14:58 ` [PATCH 1/3] memtest: use kstrtouint instead of simple_strtoul Vladimir Murzin
@ 2015-06-19 14:58 ` Vladimir Murzin
  2015-06-20  6:59   ` Leon Romanovsky
  2015-06-19 14:58 ` [PATCH 3/3] memtest: remove unused header files Vladimir Murzin
  2 siblings, 1 reply; 10+ messages in thread
From: Vladimir Murzin @ 2015-06-19 14:58 UTC (permalink / raw)
  To: linux-mm; +Cc: akpm

- prefer pr_info(...  to printk(KERN_INFO ...
- use %pa for phys_addr_t
- use cpu_to_be64 while printing pattern in reserve_bad_mem()

Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
---
 mm/memtest.c |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/mm/memtest.c b/mm/memtest.c
index 895a43c..ccaed3e 100644
--- a/mm/memtest.c
+++ b/mm/memtest.c
@@ -31,10 +31,8 @@ static u64 patterns[] __initdata = {
 
 static void __init reserve_bad_mem(u64 pattern, phys_addr_t start_bad, phys_addr_t end_bad)
 {
-	printk(KERN_INFO "  %016llx bad mem addr %010llx - %010llx reserved\n",
-	       (unsigned long long) pattern,
-	       (unsigned long long) start_bad,
-	       (unsigned long long) end_bad);
+	pr_info("%016llx bad mem addr %pa - %pa reserved\n",
+		cpu_to_be64(pattern), &start_bad, &end_bad);
 	memblock_reserve(start_bad, end_bad - start_bad);
 }
 
@@ -78,10 +76,8 @@ static void __init do_one_pass(u64 pattern, phys_addr_t start, phys_addr_t end)
 		this_start = clamp(this_start, start, end);
 		this_end = clamp(this_end, start, end);
 		if (this_start < this_end) {
-			printk(KERN_INFO "  %010llx - %010llx pattern %016llx\n",
-			       (unsigned long long)this_start,
-			       (unsigned long long)this_end,
-			       (unsigned long long)cpu_to_be64(pattern));
+			pr_info("  %pa - %pa pattern %016llx\n",
+				&this_start, &this_end, cpu_to_be64(pattern));
 			memtest(pattern, this_start, this_end - this_start);
 		}
 	}
@@ -114,7 +110,7 @@ void __init early_memtest(phys_addr_t start, phys_addr_t end)
 	if (!memtest_pattern)
 		return;
 
-	printk(KERN_INFO "early_memtest: # of tests: %d\n", memtest_pattern);
+	pr_info("early_memtest: # of tests: %u\n", memtest_pattern);
 	for (i = memtest_pattern-1; i < UINT_MAX; --i) {
 		idx = i % ARRAY_SIZE(patterns);
 		do_one_pass(patterns[idx], start, end);
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/3] memtest: remove unused header files
  2015-06-19 14:58 [PATCH 0/3] memtest cleanups Vladimir Murzin
  2015-06-19 14:58 ` [PATCH 1/3] memtest: use kstrtouint instead of simple_strtoul Vladimir Murzin
  2015-06-19 14:58 ` [PATCH 2/3] memtest: cleanup log messages Vladimir Murzin
@ 2015-06-19 14:58 ` Vladimir Murzin
  2 siblings, 0 replies; 10+ messages in thread
From: Vladimir Murzin @ 2015-06-19 14:58 UTC (permalink / raw)
  To: linux-mm; +Cc: akpm

memtest does not require these headers to be included.

Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
---
 mm/memtest.c |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/mm/memtest.c b/mm/memtest.c
index ccaed3e..fe08f70 100644
--- a/mm/memtest.c
+++ b/mm/memtest.c
@@ -1,11 +1,6 @@
 #include <linux/kernel.h>
-#include <linux/errno.h>
-#include <linux/string.h>
 #include <linux/types.h>
-#include <linux/mm.h>
-#include <linux/smp.h>
 #include <linux/init.h>
-#include <linux/pfn.h>
 #include <linux/memblock.h>
 
 static u64 patterns[] __initdata = {
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] memtest: use kstrtouint instead of simple_strtoul
  2015-06-19 14:58 ` [PATCH 1/3] memtest: use kstrtouint instead of simple_strtoul Vladimir Murzin
@ 2015-06-20  6:55   ` Leon Romanovsky
  2015-06-22  8:39     ` Vladimir Murzin
  0 siblings, 1 reply; 10+ messages in thread
From: Leon Romanovsky @ 2015-06-20  6:55 UTC (permalink / raw)
  To: Vladimir Murzin; +Cc: Linux-MM, Andrew Morton

On Fri, Jun 19, 2015 at 5:58 PM, Vladimir Murzin
<vladimir.murzin@arm.com> wrote:
> Since simple_strtoul is obsolete and memtest_pattern is type of int, use
> kstrtouint instead.
>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  mm/memtest.c |   14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/mm/memtest.c b/mm/memtest.c
> index 1997d93..895a43c 100644
> --- a/mm/memtest.c
> +++ b/mm/memtest.c
> @@ -88,14 +88,18 @@ static void __init do_one_pass(u64 pattern, phys_addr_t start, phys_addr_t end)
>  }
>
>  /* default is disabled */
> -static int memtest_pattern __initdata;
> +static unsigned int memtest_pattern __initdata;
>
>  static int __init parse_memtest(char *arg)
>  {
> -       if (arg)
> -               memtest_pattern = simple_strtoul(arg, NULL, 0);
> -       else
> -               memtest_pattern = ARRAY_SIZE(patterns);
> +       if (arg) {
> +               int err = kstrtouint(arg, 0, &memtest_pattern);
> +
> +               if (!err)
> +                       return 0;
kstrtouint returns 0 for success, in case of error you will fallback
and execute following line. It is definetely change of behaviour.
> +       }
> +
> +       memtest_pattern = ARRAY_SIZE(patterns);
>
>         return 0;
>  }
> --
> 1.7.9.5
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a hrefmailto:"dont@kvack.org"> email@kvack.org </a>



-- 
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon@leon.nu

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] memtest: cleanup log messages
  2015-06-19 14:58 ` [PATCH 2/3] memtest: cleanup log messages Vladimir Murzin
@ 2015-06-20  6:59   ` Leon Romanovsky
  2015-06-22  8:46     ` Vladimir Murzin
  0 siblings, 1 reply; 10+ messages in thread
From: Leon Romanovsky @ 2015-06-20  6:59 UTC (permalink / raw)
  To: Vladimir Murzin; +Cc: Linux-MM, Andrew Morton

On Fri, Jun 19, 2015 at 5:58 PM, Vladimir Murzin
<vladimir.murzin@arm.com> wrote:
> - prefer pr_info(...  to printk(KERN_INFO ...
> - use %pa for phys_addr_t
> - use cpu_to_be64 while printing pattern in reserve_bad_mem()
>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  mm/memtest.c |   14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/mm/memtest.c b/mm/memtest.c
> index 895a43c..ccaed3e 100644
> --- a/mm/memtest.c
> +++ b/mm/memtest.c
> @@ -31,10 +31,8 @@ static u64 patterns[] __initdata = {
>
>  static void __init reserve_bad_mem(u64 pattern, phys_addr_t start_bad, phys_addr_t end_bad)
>  {
> -       printk(KERN_INFO "  %016llx bad mem addr %010llx - %010llx reserved\n",
> -              (unsigned long long) pattern,
> -              (unsigned long long) start_bad,
> -              (unsigned long long) end_bad);
> +       pr_info("%016llx bad mem addr %pa - %pa reserved\n",
> +               cpu_to_be64(pattern), &start_bad, &end_bad);
>         memblock_reserve(start_bad, end_bad - start_bad);
>  }
>
> @@ -78,10 +76,8 @@ static void __init do_one_pass(u64 pattern, phys_addr_t start, phys_addr_t end)
>                 this_start = clamp(this_start, start, end);
>                 this_end = clamp(this_end, start, end);
>                 if (this_start < this_end) {
> -                       printk(KERN_INFO "  %010llx - %010llx pattern %016llx\n",
> -                              (unsigned long long)this_start,
> -                              (unsigned long long)this_end,
> -                              (unsigned long long)cpu_to_be64(pattern));
> +                       pr_info("  %pa - %pa pattern %016llx\n",
s/(" %pa/("%pa
> +                               &this_start, &this_end, cpu_to_be64(pattern));
>                         memtest(pattern, this_start, this_end - this_start);
>                 }
>         }
> @@ -114,7 +110,7 @@ void __init early_memtest(phys_addr_t start, phys_addr_t end)
>         if (!memtest_pattern)
>                 return;
>
> -       printk(KERN_INFO "early_memtest: # of tests: %d\n", memtest_pattern);
> +       pr_info("early_memtest: # of tests: %u\n", memtest_pattern);
>         for (i = memtest_pattern-1; i < UINT_MAX; --i) {
>                 idx = i % ARRAY_SIZE(patterns);
>                 do_one_pass(patterns[idx], start, end);
> --
> 1.7.9.5
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a hrefmailto:"dont@kvack.org"> email@kvack.org </a>



-- 
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon@leon.nu

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] memtest: use kstrtouint instead of simple_strtoul
  2015-06-20  6:55   ` Leon Romanovsky
@ 2015-06-22  8:39     ` Vladimir Murzin
  2015-06-22 10:43       ` Leon Romanovsky
  0 siblings, 1 reply; 10+ messages in thread
From: Vladimir Murzin @ 2015-06-22  8:39 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Linux-MM, Andrew Morton

On 20/06/15 07:55, Leon Romanovsky wrote:
> On Fri, Jun 19, 2015 at 5:58 PM, Vladimir Murzin
> <vladimir.murzin@arm.com> wrote:
>> Since simple_strtoul is obsolete and memtest_pattern is type of int, use
>> kstrtouint instead.
>>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>>  mm/memtest.c |   14 +++++++++-----
>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/memtest.c b/mm/memtest.c
>> index 1997d93..895a43c 100644
>> --- a/mm/memtest.c
>> +++ b/mm/memtest.c
>> @@ -88,14 +88,18 @@ static void __init do_one_pass(u64 pattern, phys_addr_t start, phys_addr_t end)
>>  }
>>
>>  /* default is disabled */
>> -static int memtest_pattern __initdata;
>> +static unsigned int memtest_pattern __initdata;
>>
>>  static int __init parse_memtest(char *arg)
>>  {
>> -       if (arg)
>> -               memtest_pattern = simple_strtoul(arg, NULL, 0);
>> -       else
>> -               memtest_pattern = ARRAY_SIZE(patterns);
>> +       if (arg) {
>> +               int err = kstrtouint(arg, 0, &memtest_pattern);
>> +
>> +               if (!err)
>> +                       return 0;
> kstrtouint returns 0 for success, in case of error you will fallback
> and execute following line. It is definetely change of behaviour.

I'd be glad if you can elaborate more on use cases dependent on this
change? I can only imagine providing garbage to the memtest option with
only intention to shut it up... but it looks like the interface abuse
since "memtest=0" does exactly the same.

Since memtest is debugging option and numeric parameter is optional I
thought it was not harmful to fallback to default in case something is
wrong with the parameter.

Thanks
Vladimir

>> +       }
>> +
>> +       memtest_pattern = ARRAY_SIZE(patterns);
>>
>>         return 0;
>>  }
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a hrefmailto:"dont@kvack.org"> email@kvack.org </a>
> 
> 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] memtest: cleanup log messages
  2015-06-20  6:59   ` Leon Romanovsky
@ 2015-06-22  8:46     ` Vladimir Murzin
  0 siblings, 0 replies; 10+ messages in thread
From: Vladimir Murzin @ 2015-06-22  8:46 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Linux-MM, Andrew Morton

On 20/06/15 07:59, Leon Romanovsky wrote:
> On Fri, Jun 19, 2015 at 5:58 PM, Vladimir Murzin
> <vladimir.murzin@arm.com> wrote:
>> - prefer pr_info(...  to printk(KERN_INFO ...
>> - use %pa for phys_addr_t
>> - use cpu_to_be64 while printing pattern in reserve_bad_mem()
>>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>>  mm/memtest.c |   14 +++++---------
>>  1 file changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/mm/memtest.c b/mm/memtest.c
>> index 895a43c..ccaed3e 100644
>> --- a/mm/memtest.c
>> +++ b/mm/memtest.c
>> @@ -31,10 +31,8 @@ static u64 patterns[] __initdata = {
>>
>>  static void __init reserve_bad_mem(u64 pattern, phys_addr_t start_bad, phys_addr_t end_bad)
>>  {
>> -       printk(KERN_INFO "  %016llx bad mem addr %010llx - %010llx reserved\n",
>> -              (unsigned long long) pattern,
>> -              (unsigned long long) start_bad,
>> -              (unsigned long long) end_bad);
>> +       pr_info("%016llx bad mem addr %pa - %pa reserved\n",
>> +               cpu_to_be64(pattern), &start_bad, &end_bad);
>>         memblock_reserve(start_bad, end_bad - start_bad);
>>  }
>>
>> @@ -78,10 +76,8 @@ static void __init do_one_pass(u64 pattern, phys_addr_t start, phys_addr_t end)
>>                 this_start = clamp(this_start, start, end);
>>                 this_end = clamp(this_end, start, end);
>>                 if (this_start < this_end) {
>> -                       printk(KERN_INFO "  %010llx - %010llx pattern %016llx\n",
>> -                              (unsigned long long)this_start,
>> -                              (unsigned long long)this_end,
>> -                              (unsigned long long)cpu_to_be64(pattern));
>> +                       pr_info("  %pa - %pa pattern %016llx\n",
> s/(" %pa/("%pa

I don't think so since these messages belong to the "early_memtest:" and
this whitespace highlights where these logs come from.

Thanks
Vladimir

>> +                               &this_start, &this_end, cpu_to_be64(pattern));
>>                         memtest(pattern, this_start, this_end - this_start);
>>                 }
>>         }
>> @@ -114,7 +110,7 @@ void __init early_memtest(phys_addr_t start, phys_addr_t end)
>>         if (!memtest_pattern)
>>                 return;
>>
>> -       printk(KERN_INFO "early_memtest: # of tests: %d\n", memtest_pattern);
>> +       pr_info("early_memtest: # of tests: %u\n", memtest_pattern);
>>         for (i = memtest_pattern-1; i < UINT_MAX; --i) {
>>                 idx = i % ARRAY_SIZE(patterns);
>>                 do_one_pass(patterns[idx], start, end);
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a hrefmailto:"dont@kvack.org"> email@kvack.org </a>
> 
> 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] memtest: use kstrtouint instead of simple_strtoul
  2015-06-22  8:39     ` Vladimir Murzin
@ 2015-06-22 10:43       ` Leon Romanovsky
  2015-06-22 11:39         ` Vladimir Murzin
  0 siblings, 1 reply; 10+ messages in thread
From: Leon Romanovsky @ 2015-06-22 10:43 UTC (permalink / raw)
  To: Vladimir Murzin; +Cc: Linux-MM, Andrew Morton

On Mon, Jun 22, 2015 at 11:39 AM, Vladimir Murzin
<vladimir.murzin@arm.com> wrote:
> On 20/06/15 07:55, Leon Romanovsky wrote:
>> On Fri, Jun 19, 2015 at 5:58 PM, Vladimir Murzin
>> <vladimir.murzin@arm.com> wrote:
>>> Since simple_strtoul is obsolete and memtest_pattern is type of int, use
>>> kstrtouint instead.
>>>
>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>>> ---
>>>  mm/memtest.c |   14 +++++++++-----
>>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/mm/memtest.c b/mm/memtest.c
>>> index 1997d93..895a43c 100644
>>> --- a/mm/memtest.c
>>> +++ b/mm/memtest.c
>>> @@ -88,14 +88,18 @@ static void __init do_one_pass(u64 pattern, phys_addr_t start, phys_addr_t end)
>>>  }
>>>
>>>  /* default is disabled */
>>> -static int memtest_pattern __initdata;
>>> +static unsigned int memtest_pattern __initdata;
>>>
>>>  static int __init parse_memtest(char *arg)
>>>  {
>>> -       if (arg)
>>> -               memtest_pattern = simple_strtoul(arg, NULL, 0);
>>> -       else
>>> -               memtest_pattern = ARRAY_SIZE(patterns);
>>> +       if (arg) {
>>> +               int err = kstrtouint(arg, 0, &memtest_pattern);
>>> +
>>> +               if (!err)
>>> +                       return 0;
>> kstrtouint returns 0 for success, in case of error you will fallback
>> and execute following line. It is definetely change of behaviour.
>
> I'd be glad if you can elaborate more on use cases dependent on this
> change? I can only imagine providing garbage to the memtest option with
> only intention to shut it up... but it looks like the interface abuse
> since "memtest=0" does exactly the same.
>
> Since memtest is debugging option and numeric parameter is optional I
> thought it was not harmful to fallback to default in case something is
> wrong with the parameter.
I would like to suggest you, in case of error, print warning and set
memtest_pattern to be zero and return back to if(arg)..else
construction.

>
> Thanks
> Vladimir
>
>>> +       }
>>> +
>>> +       memtest_pattern = ARRAY_SIZE(patterns);
>>>
>>>         return 0;
>>>  }
>>> --
>>> 1.7.9.5
>>>
>>> --
>>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>>> the body to majordomo@kvack.org.  For more info on Linux MM,
>>> see: http://www.linux-mm.org/ .
>>> Don't email: <a hrefmailto:"dont@kvack.org"> email@kvack.org </a>
>>
>>
>>
>



-- 
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon@leon.nu

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] memtest: use kstrtouint instead of simple_strtoul
  2015-06-22 10:43       ` Leon Romanovsky
@ 2015-06-22 11:39         ` Vladimir Murzin
  0 siblings, 0 replies; 10+ messages in thread
From: Vladimir Murzin @ 2015-06-22 11:39 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Linux-MM, Andrew Morton

On 22/06/15 11:43, Leon Romanovsky wrote:
> On Mon, Jun 22, 2015 at 11:39 AM, Vladimir Murzin
> <vladimir.murzin@arm.com> wrote:
>> On 20/06/15 07:55, Leon Romanovsky wrote:
>>> On Fri, Jun 19, 2015 at 5:58 PM, Vladimir Murzin
>>> <vladimir.murzin@arm.com> wrote:
>>>> Since simple_strtoul is obsolete and memtest_pattern is type of int, use
>>>> kstrtouint instead.
>>>>
>>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>>>> ---
>>>>  mm/memtest.c |   14 +++++++++-----
>>>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/mm/memtest.c b/mm/memtest.c
>>>> index 1997d93..895a43c 100644
>>>> --- a/mm/memtest.c
>>>> +++ b/mm/memtest.c
>>>> @@ -88,14 +88,18 @@ static void __init do_one_pass(u64 pattern, phys_addr_t start, phys_addr_t end)
>>>>  }
>>>>
>>>>  /* default is disabled */
>>>> -static int memtest_pattern __initdata;
>>>> +static unsigned int memtest_pattern __initdata;
>>>>
>>>>  static int __init parse_memtest(char *arg)
>>>>  {
>>>> -       if (arg)
>>>> -               memtest_pattern = simple_strtoul(arg, NULL, 0);
>>>> -       else
>>>> -               memtest_pattern = ARRAY_SIZE(patterns);
>>>> +       if (arg) {
>>>> +               int err = kstrtouint(arg, 0, &memtest_pattern);
>>>> +
>>>> +               if (!err)
>>>> +                       return 0;
>>> kstrtouint returns 0 for success, in case of error you will fallback
>>> and execute following line. It is definetely change of behaviour.
>>
>> I'd be glad if you can elaborate more on use cases dependent on this
>> change? I can only imagine providing garbage to the memtest option with
>> only intention to shut it up... but it looks like the interface abuse
>> since "memtest=0" does exactly the same.
>>
>> Since memtest is debugging option and numeric parameter is optional I
>> thought it was not harmful to fallback to default in case something is
>> wrong with the parameter.
> I would like to suggest you, in case of error, print warning and set
> memtest_pattern to be zero and return back to if(arg)..else
> construction.

In case of kstrtouint() failure memtest_pattern is not altered, so I
won't need extra assignment here... and I'm not sure I get your point
with returning back to if(arg)...else

Anyway, I'd wait for other comments on what should be done here if anything.

Thanks
Vladimir

> 
>>
>> Thanks
>> Vladimir
>>
>>>> +       }
>>>> +
>>>> +       memtest_pattern = ARRAY_SIZE(patterns);
>>>>
>>>>         return 0;
>>>>  }
>>>> --
>>>> 1.7.9.5
>>>>
>>>> --
>>>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>>>> the body to majordomo@kvack.org.  For more info on Linux MM,
>>>> see: http://www.linux-mm.org/ .
>>>> Don't email: <a hrefmailto:"dont@kvack.org"> email@kvack.org </a>
>>>
>>>
>>>
>>
> 
> 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-06-22 11:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-19 14:58 [PATCH 0/3] memtest cleanups Vladimir Murzin
2015-06-19 14:58 ` [PATCH 1/3] memtest: use kstrtouint instead of simple_strtoul Vladimir Murzin
2015-06-20  6:55   ` Leon Romanovsky
2015-06-22  8:39     ` Vladimir Murzin
2015-06-22 10:43       ` Leon Romanovsky
2015-06-22 11:39         ` Vladimir Murzin
2015-06-19 14:58 ` [PATCH 2/3] memtest: cleanup log messages Vladimir Murzin
2015-06-20  6:59   ` Leon Romanovsky
2015-06-22  8:46     ` Vladimir Murzin
2015-06-19 14:58 ` [PATCH 3/3] memtest: remove unused header files Vladimir Murzin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox