Re: [PATCH v4] lib: add size unit t/p/e to memparse

2014-06-17 Thread Satoru Takeuchi

Hi Gui,

(2014/06/18 7:21), David Rientjes wrote:

On Fri, 13 Jun 2014, Gui Hecheng wrote:


For modern filesystems such as btrfs, t/p/e size level operations
are common.
add size unit t/p/e parsing to memparse

Signed-off-by: Gui Hecheng 


Acked-by: David Rientjes 


It looks good to me.

Reviewed-by: Satoru Takeuchi 

Thanks,
Satoru



Sorry, didn't do this before.  Good luck with the simple_strto*() work!
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
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 v4] lib: add size unit t/p/e to memparse

2014-06-17 Thread David Rientjes
On Fri, 13 Jun 2014, Gui Hecheng wrote:

> For modern filesystems such as btrfs, t/p/e size level operations
> are common.
> add size unit t/p/e parsing to memparse
> 
> Signed-off-by: Gui Hecheng 

Acked-by: David Rientjes 

Sorry, didn't do this before.  Good luck with the simple_strto*() work!
--
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 v4] lib: add size unit t/p/e to memparse

2014-06-17 Thread David Rientjes
On Fri, 13 Jun 2014, Gui Hecheng wrote:

 For modern filesystems such as btrfs, t/p/e size level operations
 are common.
 add size unit t/p/e parsing to memparse
 
 Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com

Acked-by: David Rientjes rient...@google.com

Sorry, didn't do this before.  Good luck with the simple_strto*() work!
--
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 v4] lib: add size unit t/p/e to memparse

2014-06-17 Thread Satoru Takeuchi

Hi Gui,

(2014/06/18 7:21), David Rientjes wrote:

On Fri, 13 Jun 2014, Gui Hecheng wrote:


For modern filesystems such as btrfs, t/p/e size level operations
are common.
add size unit t/p/e parsing to memparse

Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com


Acked-by: David Rientjes rient...@google.com


It looks good to me.

Reviewed-by: Satoru Takeuchi takeuchi_sat...@jp.fujitsu.com

Thanks,
Satoru



Sorry, didn't do this before.  Good luck with the simple_strto*() work!
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
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 v4] lib: add size unit t/p/e to memparse

2014-06-16 Thread Gui Hecheng
On Mon, 2014-06-16 at 18:29 -0700, David Rientjes wrote:
> On Tue, 17 Jun 2014, Gui Hecheng wrote:
> 
> > > > diff --git a/lib/cmdline.c b/lib/cmdline.c
> > > > index d4932f7..76a712e 100644
> > > > --- a/lib/cmdline.c
> > > > +++ b/lib/cmdline.c
> > > > @@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
> > > >   * @retptr: (output) Optional pointer to next char after parse 
> > > > completes
> > > >   *
> > > >   * Parses a string into a number.  The number stored at @ptr is
> > > > - * potentially suffixed with %K (for kilobytes, or 1024 bytes),
> > > > - * %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
> > > > - * 1073741824).  If the number is suffixed with K, M, or G, then
> > > > - * the return value is the number multiplied by one kilobyte, one
> > > > - * megabyte, or one gigabyte, respectively.
> > > > + * potentially suffixed with K, M, G, T, P, E.
> > > >   */
> > > >  
> > > >  unsigned long long memparse(const char *ptr, char **retptr)
> > > > @@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char 
> > > > **retptr)
> > > > unsigned long long ret = simple_strtoull(ptr, , 0);
> > > >  
> > > > switch (*endptr) {
> > > > +   case 'E':
> > > > +   case 'e':
> > > > +   ret <<= 10;
> > > > +   case 'P':
> > > > +   case 'p':
> > > > +   ret <<= 10;
> > > > +   case 'T':
> > > > +   case 't':
> > > > +   ret <<= 10;
> > > > case 'G':
> > > > case 'g':
> > > > ret <<= 10;
> > > 
> > > Seems fine since unsigned long long is always at least 64 bits, but 
> > > perhaps also change simple_strtoull() to use kstrtoull() at the same time 
> > > since the former is deprecated?
> > 
> > Yes, that is a point. But the deprecated function is a separate problem
> > and may not be included in this patch.
> > Also, I find that simple_strtoull is used in many places in the kernel
> > code, it is better to replace it globally?
> > 
> 
> If you're going to have a go at replacing the simple_strto*() functions 
> throughout the kernel, it's probably better to do it per subsystem (as 
> defined by the separate sections of MAINTAINERS) and propose the patches 
> individually to those maintainers.  Once it has been removed entirely, you 
> can submit a patch to remove the functions themselves.
> 
> Be aware that there are many callers to the deprecated functions so it may 
> take a significant amount of time.

H...It may really take a long time. But I am not sure whether this
is really a good idea to do this big replacement. Let's see whether
anyone would like to share his opinions.

-Gui

--
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 v4] lib: add size unit t/p/e to memparse

2014-06-16 Thread David Rientjes
On Tue, 17 Jun 2014, Gui Hecheng wrote:

> > > diff --git a/lib/cmdline.c b/lib/cmdline.c
> > > index d4932f7..76a712e 100644
> > > --- a/lib/cmdline.c
> > > +++ b/lib/cmdline.c
> > > @@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
> > >   *   @retptr: (output) Optional pointer to next char after parse 
> > > completes
> > >   *
> > >   *   Parses a string into a number.  The number stored at @ptr is
> > > - *   potentially suffixed with %K (for kilobytes, or 1024 bytes),
> > > - *   %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
> > > - *   1073741824).  If the number is suffixed with K, M, or G, then
> > > - *   the return value is the number multiplied by one kilobyte, one
> > > - *   megabyte, or one gigabyte, respectively.
> > > + *   potentially suffixed with K, M, G, T, P, E.
> > >   */
> > >  
> > >  unsigned long long memparse(const char *ptr, char **retptr)
> > > @@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char 
> > > **retptr)
> > >   unsigned long long ret = simple_strtoull(ptr, , 0);
> > >  
> > >   switch (*endptr) {
> > > + case 'E':
> > > + case 'e':
> > > + ret <<= 10;
> > > + case 'P':
> > > + case 'p':
> > > + ret <<= 10;
> > > + case 'T':
> > > + case 't':
> > > + ret <<= 10;
> > >   case 'G':
> > >   case 'g':
> > >   ret <<= 10;
> > 
> > Seems fine since unsigned long long is always at least 64 bits, but 
> > perhaps also change simple_strtoull() to use kstrtoull() at the same time 
> > since the former is deprecated?
> 
> Yes, that is a point. But the deprecated function is a separate problem
> and may not be included in this patch.
> Also, I find that simple_strtoull is used in many places in the kernel
> code, it is better to replace it globally?
> 

If you're going to have a go at replacing the simple_strto*() functions 
throughout the kernel, it's probably better to do it per subsystem (as 
defined by the separate sections of MAINTAINERS) and propose the patches 
individually to those maintainers.  Once it has been removed entirely, you 
can submit a patch to remove the functions themselves.

Be aware that there are many callers to the deprecated functions so it may 
take a significant amount of time.
--
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 v4] lib: add size unit t/p/e to memparse

2014-06-16 Thread Gui Hecheng
On Fri, 2014-06-13 at 14:58 -0700, David Rientjes wrote:
> On Fri, 13 Jun 2014, Gui Hecheng wrote:
> 
> > diff --git a/lib/cmdline.c b/lib/cmdline.c
> > index d4932f7..76a712e 100644
> > --- a/lib/cmdline.c
> > +++ b/lib/cmdline.c
> > @@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
> >   * @retptr: (output) Optional pointer to next char after parse completes
> >   *
> >   * Parses a string into a number.  The number stored at @ptr is
> > - * potentially suffixed with %K (for kilobytes, or 1024 bytes),
> > - * %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
> > - * 1073741824).  If the number is suffixed with K, M, or G, then
> > - * the return value is the number multiplied by one kilobyte, one
> > - * megabyte, or one gigabyte, respectively.
> > + * potentially suffixed with K, M, G, T, P, E.
> >   */
> >  
> >  unsigned long long memparse(const char *ptr, char **retptr)
> > @@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char 
> > **retptr)
> > unsigned long long ret = simple_strtoull(ptr, , 0);
> >  
> > switch (*endptr) {
> > +   case 'E':
> > +   case 'e':
> > +   ret <<= 10;
> > +   case 'P':
> > +   case 'p':
> > +   ret <<= 10;
> > +   case 'T':
> > +   case 't':
> > +   ret <<= 10;
> > case 'G':
> > case 'g':
> > ret <<= 10;
> 
> Seems fine since unsigned long long is always at least 64 bits, but 
> perhaps also change simple_strtoull() to use kstrtoull() at the same time 
> since the former is deprecated?

Yes, that is a point. But the deprecated function is a separate problem
and may not be included in this patch.
Also, I find that simple_strtoull is used in many places in the kernel
code, it is better to replace it globally?

-Gui

--
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 v4] lib: add size unit t/p/e to memparse

2014-06-16 Thread Gui Hecheng
On Fri, 2014-06-13 at 14:58 -0700, David Rientjes wrote:
 On Fri, 13 Jun 2014, Gui Hecheng wrote:
 
  diff --git a/lib/cmdline.c b/lib/cmdline.c
  index d4932f7..76a712e 100644
  --- a/lib/cmdline.c
  +++ b/lib/cmdline.c
  @@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
* @retptr: (output) Optional pointer to next char after parse completes
*
* Parses a string into a number.  The number stored at @ptr is
  - * potentially suffixed with %K (for kilobytes, or 1024 bytes),
  - * %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
  - * 1073741824).  If the number is suffixed with K, M, or G, then
  - * the return value is the number multiplied by one kilobyte, one
  - * megabyte, or one gigabyte, respectively.
  + * potentially suffixed with K, M, G, T, P, E.
*/
   
   unsigned long long memparse(const char *ptr, char **retptr)
  @@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char 
  **retptr)
  unsigned long long ret = simple_strtoull(ptr, endptr, 0);
   
  switch (*endptr) {
  +   case 'E':
  +   case 'e':
  +   ret = 10;
  +   case 'P':
  +   case 'p':
  +   ret = 10;
  +   case 'T':
  +   case 't':
  +   ret = 10;
  case 'G':
  case 'g':
  ret = 10;
 
 Seems fine since unsigned long long is always at least 64 bits, but 
 perhaps also change simple_strtoull() to use kstrtoull() at the same time 
 since the former is deprecated?

Yes, that is a point. But the deprecated function is a separate problem
and may not be included in this patch.
Also, I find that simple_strtoull is used in many places in the kernel
code, it is better to replace it globally?

-Gui

--
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 v4] lib: add size unit t/p/e to memparse

2014-06-16 Thread David Rientjes
On Tue, 17 Jun 2014, Gui Hecheng wrote:

   diff --git a/lib/cmdline.c b/lib/cmdline.c
   index d4932f7..76a712e 100644
   --- a/lib/cmdline.c
   +++ b/lib/cmdline.c
   @@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
 *   @retptr: (output) Optional pointer to next char after parse 
   completes
 *
 *   Parses a string into a number.  The number stored at @ptr is
   - *   potentially suffixed with %K (for kilobytes, or 1024 bytes),
   - *   %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
   - *   1073741824).  If the number is suffixed with K, M, or G, then
   - *   the return value is the number multiplied by one kilobyte, one
   - *   megabyte, or one gigabyte, respectively.
   + *   potentially suffixed with K, M, G, T, P, E.
 */

unsigned long long memparse(const char *ptr, char **retptr)
   @@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char 
   **retptr)
 unsigned long long ret = simple_strtoull(ptr, endptr, 0);

 switch (*endptr) {
   + case 'E':
   + case 'e':
   + ret = 10;
   + case 'P':
   + case 'p':
   + ret = 10;
   + case 'T':
   + case 't':
   + ret = 10;
 case 'G':
 case 'g':
 ret = 10;
  
  Seems fine since unsigned long long is always at least 64 bits, but 
  perhaps also change simple_strtoull() to use kstrtoull() at the same time 
  since the former is deprecated?
 
 Yes, that is a point. But the deprecated function is a separate problem
 and may not be included in this patch.
 Also, I find that simple_strtoull is used in many places in the kernel
 code, it is better to replace it globally?
 

If you're going to have a go at replacing the simple_strto*() functions 
throughout the kernel, it's probably better to do it per subsystem (as 
defined by the separate sections of MAINTAINERS) and propose the patches 
individually to those maintainers.  Once it has been removed entirely, you 
can submit a patch to remove the functions themselves.

Be aware that there are many callers to the deprecated functions so it may 
take a significant amount of time.
--
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 v4] lib: add size unit t/p/e to memparse

2014-06-16 Thread Gui Hecheng
On Mon, 2014-06-16 at 18:29 -0700, David Rientjes wrote:
 On Tue, 17 Jun 2014, Gui Hecheng wrote:
 
diff --git a/lib/cmdline.c b/lib/cmdline.c
index d4932f7..76a712e 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
  * @retptr: (output) Optional pointer to next char after parse 
completes
  *
  * Parses a string into a number.  The number stored at @ptr is
- * potentially suffixed with %K (for kilobytes, or 1024 bytes),
- * %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
- * 1073741824).  If the number is suffixed with K, M, or G, then
- * the return value is the number multiplied by one kilobyte, one
- * megabyte, or one gigabyte, respectively.
+ * potentially suffixed with K, M, G, T, P, E.
  */
 
 unsigned long long memparse(const char *ptr, char **retptr)
@@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char 
**retptr)
unsigned long long ret = simple_strtoull(ptr, endptr, 0);
 
switch (*endptr) {
+   case 'E':
+   case 'e':
+   ret = 10;
+   case 'P':
+   case 'p':
+   ret = 10;
+   case 'T':
+   case 't':
+   ret = 10;
case 'G':
case 'g':
ret = 10;
   
   Seems fine since unsigned long long is always at least 64 bits, but 
   perhaps also change simple_strtoull() to use kstrtoull() at the same time 
   since the former is deprecated?
  
  Yes, that is a point. But the deprecated function is a separate problem
  and may not be included in this patch.
  Also, I find that simple_strtoull is used in many places in the kernel
  code, it is better to replace it globally?
  
 
 If you're going to have a go at replacing the simple_strto*() functions 
 throughout the kernel, it's probably better to do it per subsystem (as 
 defined by the separate sections of MAINTAINERS) and propose the patches 
 individually to those maintainers.  Once it has been removed entirely, you 
 can submit a patch to remove the functions themselves.
 
 Be aware that there are many callers to the deprecated functions so it may 
 take a significant amount of time.

H...It may really take a long time. But I am not sure whether this
is really a good idea to do this big replacement. Let's see whether
anyone would like to share his opinions.

-Gui

--
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 v4] lib: add size unit t/p/e to memparse

2014-06-13 Thread David Rientjes
On Fri, 13 Jun 2014, Gui Hecheng wrote:

> diff --git a/lib/cmdline.c b/lib/cmdline.c
> index d4932f7..76a712e 100644
> --- a/lib/cmdline.c
> +++ b/lib/cmdline.c
> @@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
>   *   @retptr: (output) Optional pointer to next char after parse completes
>   *
>   *   Parses a string into a number.  The number stored at @ptr is
> - *   potentially suffixed with %K (for kilobytes, or 1024 bytes),
> - *   %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
> - *   1073741824).  If the number is suffixed with K, M, or G, then
> - *   the return value is the number multiplied by one kilobyte, one
> - *   megabyte, or one gigabyte, respectively.
> + *   potentially suffixed with K, M, G, T, P, E.
>   */
>  
>  unsigned long long memparse(const char *ptr, char **retptr)
> @@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char 
> **retptr)
>   unsigned long long ret = simple_strtoull(ptr, , 0);
>  
>   switch (*endptr) {
> + case 'E':
> + case 'e':
> + ret <<= 10;
> + case 'P':
> + case 'p':
> + ret <<= 10;
> + case 'T':
> + case 't':
> + ret <<= 10;
>   case 'G':
>   case 'g':
>   ret <<= 10;

Seems fine since unsigned long long is always at least 64 bits, but 
perhaps also change simple_strtoull() to use kstrtoull() at the same time 
since the former is deprecated?
--
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 v4] lib: add size unit t/p/e to memparse

2014-06-13 Thread Gui Hecheng
On Fri, 2014-06-13 at 07:55 +0200, Brendan Hide wrote:
> On 13/06/14 03:42, Gui Hecheng wrote:
> > For modern filesystems such as btrfs, t/p/e size level operations
> > are common.
> > add size unit t/p/e parsing to memparse
> >
> > Signed-off-by: Gui Hecheng 
> > ---
> > changelog
> > v1->v2: replace kilobyte with kibibyte, and others
> > v2->v3: add missing unit "bytes" in comment
> > v3->v4: remove idiotic name for K,M,G,P,T,E
> > ---
> >   lib/cmdline.c | 15 ++-
> >   1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/cmdline.c b/lib/cmdline.c
> > index d4932f7..76a712e 100644
> > --- a/lib/cmdline.c
> > +++ b/lib/cmdline.c
> > @@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
> >*@retptr: (output) Optional pointer to next char after parse 
> > completes
> >*
> >*Parses a string into a number.  The number stored at @ptr is
> > - * potentially suffixed with %K (for kilobytes, or 1024 bytes),
> > - * %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
> > - * 1073741824).  If the number is suffixed with K, M, or G, then
> > - * the return value is the number multiplied by one kilobyte, one
> > - * megabyte, or one gigabyte, respectively.
> > + * potentially suffixed with K, M, G, T, P, E.
> >*/
> >   
> >   unsigned long long memparse(const char *ptr, char **retptr)
> > @@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char 
> > **retptr)
> > unsigned long long ret = simple_strtoull(ptr, , 0);
> >   
> > switch (*endptr) {
> > +   case 'E':
> > +   case 'e':
> > +   ret <<= 10;
> > +   case 'P':
> > +   case 'p':
> > +   ret <<= 10;
> > +   case 'T':
> > +   case 't':
> > +   ret <<= 10;
> > case 'G':
> > case 'g':
> > ret <<= 10;
> Ah, I see - you've removed all reference to their names. That's good too. :)
> 
Thank you for your review!
I think maybe more people would like cleaner things in the kernel.

-Gui

--
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 v4] lib: add size unit t/p/e to memparse

2014-06-13 Thread Brendan Hide

On 13/06/14 03:42, Gui Hecheng wrote:

For modern filesystems such as btrfs, t/p/e size level operations
are common.
add size unit t/p/e parsing to memparse

Signed-off-by: Gui Hecheng 
---
changelog
v1->v2: replace kilobyte with kibibyte, and others
v2->v3: add missing unit "bytes" in comment
v3->v4: remove idiotic name for K,M,G,P,T,E
---
  lib/cmdline.c | 15 ++-
  1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index d4932f7..76a712e 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
   *@retptr: (output) Optional pointer to next char after parse completes
   *
   *Parses a string into a number.  The number stored at @ptr is
- * potentially suffixed with %K (for kilobytes, or 1024 bytes),
- * %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
- * 1073741824).  If the number is suffixed with K, M, or G, then
- * the return value is the number multiplied by one kilobyte, one
- * megabyte, or one gigabyte, respectively.
+ * potentially suffixed with K, M, G, T, P, E.
   */
  
  unsigned long long memparse(const char *ptr, char **retptr)

@@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char **retptr)
unsigned long long ret = simple_strtoull(ptr, , 0);
  
  	switch (*endptr) {

+   case 'E':
+   case 'e':
+   ret <<= 10;
+   case 'P':
+   case 'p':
+   ret <<= 10;
+   case 'T':
+   case 't':
+   ret <<= 10;
case 'G':
case 'g':
ret <<= 10;

Ah, I see - you've removed all reference to their names. That's good too. :)

--
__
Brendan Hide
http://swiftspirit.co.za/
http://www.webafrica.co.za/?AFF1E97

--
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 v4] lib: add size unit t/p/e to memparse

2014-06-13 Thread Brendan Hide

On 13/06/14 03:42, Gui Hecheng wrote:

For modern filesystems such as btrfs, t/p/e size level operations
are common.
add size unit t/p/e parsing to memparse

Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
---
changelog
v1-v2: replace kilobyte with kibibyte, and others
v2-v3: add missing unit bytes in comment
v3-v4: remove idiotic name for K,M,G,P,T,E
---
  lib/cmdline.c | 15 ++-
  1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index d4932f7..76a712e 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
   *@retptr: (output) Optional pointer to next char after parse completes
   *
   *Parses a string into a number.  The number stored at @ptr is
- * potentially suffixed with %K (for kilobytes, or 1024 bytes),
- * %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
- * 1073741824).  If the number is suffixed with K, M, or G, then
- * the return value is the number multiplied by one kilobyte, one
- * megabyte, or one gigabyte, respectively.
+ * potentially suffixed with K, M, G, T, P, E.
   */
  
  unsigned long long memparse(const char *ptr, char **retptr)

@@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char **retptr)
unsigned long long ret = simple_strtoull(ptr, endptr, 0);
  
  	switch (*endptr) {

+   case 'E':
+   case 'e':
+   ret = 10;
+   case 'P':
+   case 'p':
+   ret = 10;
+   case 'T':
+   case 't':
+   ret = 10;
case 'G':
case 'g':
ret = 10;

Ah, I see - you've removed all reference to their names. That's good too. :)

--
__
Brendan Hide
http://swiftspirit.co.za/
http://www.webafrica.co.za/?AFF1E97

--
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 v4] lib: add size unit t/p/e to memparse

2014-06-13 Thread Gui Hecheng
On Fri, 2014-06-13 at 07:55 +0200, Brendan Hide wrote:
 On 13/06/14 03:42, Gui Hecheng wrote:
  For modern filesystems such as btrfs, t/p/e size level operations
  are common.
  add size unit t/p/e parsing to memparse
 
  Signed-off-by: Gui Hecheng guihc.f...@cn.fujitsu.com
  ---
  changelog
  v1-v2: replace kilobyte with kibibyte, and others
  v2-v3: add missing unit bytes in comment
  v3-v4: remove idiotic name for K,M,G,P,T,E
  ---
lib/cmdline.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
 
  diff --git a/lib/cmdline.c b/lib/cmdline.c
  index d4932f7..76a712e 100644
  --- a/lib/cmdline.c
  +++ b/lib/cmdline.c
  @@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
 *@retptr: (output) Optional pointer to next char after parse 
  completes
 *
 *Parses a string into a number.  The number stored at @ptr is
  - * potentially suffixed with %K (for kilobytes, or 1024 bytes),
  - * %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
  - * 1073741824).  If the number is suffixed with K, M, or G, then
  - * the return value is the number multiplied by one kilobyte, one
  - * megabyte, or one gigabyte, respectively.
  + * potentially suffixed with K, M, G, T, P, E.
 */

unsigned long long memparse(const char *ptr, char **retptr)
  @@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char 
  **retptr)
  unsigned long long ret = simple_strtoull(ptr, endptr, 0);

  switch (*endptr) {
  +   case 'E':
  +   case 'e':
  +   ret = 10;
  +   case 'P':
  +   case 'p':
  +   ret = 10;
  +   case 'T':
  +   case 't':
  +   ret = 10;
  case 'G':
  case 'g':
  ret = 10;
 Ah, I see - you've removed all reference to their names. That's good too. :)
 
Thank you for your review!
I think maybe more people would like cleaner things in the kernel.

-Gui

--
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 v4] lib: add size unit t/p/e to memparse

2014-06-13 Thread David Rientjes
On Fri, 13 Jun 2014, Gui Hecheng wrote:

 diff --git a/lib/cmdline.c b/lib/cmdline.c
 index d4932f7..76a712e 100644
 --- a/lib/cmdline.c
 +++ b/lib/cmdline.c
 @@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
   *   @retptr: (output) Optional pointer to next char after parse completes
   *
   *   Parses a string into a number.  The number stored at @ptr is
 - *   potentially suffixed with %K (for kilobytes, or 1024 bytes),
 - *   %M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
 - *   1073741824).  If the number is suffixed with K, M, or G, then
 - *   the return value is the number multiplied by one kilobyte, one
 - *   megabyte, or one gigabyte, respectively.
 + *   potentially suffixed with K, M, G, T, P, E.
   */
  
  unsigned long long memparse(const char *ptr, char **retptr)
 @@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char 
 **retptr)
   unsigned long long ret = simple_strtoull(ptr, endptr, 0);
  
   switch (*endptr) {
 + case 'E':
 + case 'e':
 + ret = 10;
 + case 'P':
 + case 'p':
 + ret = 10;
 + case 'T':
 + case 't':
 + ret = 10;
   case 'G':
   case 'g':
   ret = 10;

Seems fine since unsigned long long is always at least 64 bits, but 
perhaps also change simple_strtoull() to use kstrtoull() at the same time 
since the former is deprecated?
--
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/