All of lore.kernel.org
 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 related	[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 related	[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 related	[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 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.