[PATCH] staging: erofs: Add braces to do-while statements

[PATCH] staging: erofs: Add braces to do-while statements

[Thomas Jespersen]

This fixes warning reported by sparse (with -Wsparse-all).

Signed-off-by: Thomas Jespersen <laumann.thomas@gmail.com>
---
 drivers/staging/erofs/internal.h | 4 ++--
 drivers/staging/erofs/utils.c    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 57575c7f5635..bf180a803446 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
 	/* spin if it is temporarily locked at the reclaim path */
 	if (unlikely(o == locked)) {
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-		do
+		do {
 			cpu_relax();
-		while (atomic_read(&grp->refcount) == locked);
+		} while (atomic_read(&grp->refcount) == locked);
 #endif
 		goto repeat;
 	}
diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
index ea8a962e5c95..cd76df4a48c7 100644
--- a/drivers/staging/erofs/utils.c
+++ b/drivers/staging/erofs/utils.c
@@ -217,9 +217,9 @@ unsigned long erofs_shrink_scan(struct shrinker *shrink,
 	unsigned long freed = 0;
 
 	spin_lock(&erofs_sb_list_lock);
-	do
+	do {
 		run_no = ++shrinker_run_no;
-	while (run_no == 0);
+	} while (run_no == 0);
 
 	/* Iterate over all mounted superblocks and try to shrink them */
 	p = erofs_sb_list.next;
-- 
2.19.2

[PATCH] staging: erofs: Add braces to do-while statements

[Thomas Jespersen]

This fixes warning reported by sparse (with -Wsparse-all).

Signed-off-by: Thomas Jespersen <laumann.thomas at gmail.com>
---
 drivers/staging/erofs/internal.h | 4 ++--
 drivers/staging/erofs/utils.c    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 57575c7f5635..bf180a803446 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
 	/* spin if it is temporarily locked at the reclaim path */
 	if (unlikely(o == locked)) {
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-		do
+		do {
 			cpu_relax();
-		while (atomic_read(&grp->refcount) == locked);
+		} while (atomic_read(&grp->refcount) == locked);
 #endif
 		goto repeat;
 	}
diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
index ea8a962e5c95..cd76df4a48c7 100644
--- a/drivers/staging/erofs/utils.c
+++ b/drivers/staging/erofs/utils.c
@@ -217,9 +217,9 @@ unsigned long erofs_shrink_scan(struct shrinker *shrink,
 	unsigned long freed = 0;
 
 	spin_lock(&erofs_sb_list_lock);
-	do
+	do {
 		run_no = ++shrinker_run_no;
-	while (run_no == 0);
+	} while (run_no == 0);
 
 	/* Iterate over all mounted superblocks and try to shrink them */
 	p = erofs_sb_list.next;
-- 
2.19.2

Re: [PATCH] staging: erofs: Add braces to do-while statements

[Greg KH]

On Sun, Dec 09, 2018 at 04:59:00PM +0100, Thomas Jespersen wrote:
> This fixes warning reported by sparse (with -Wsparse-all).

Why is sparse warning about this?

> Signed-off-by: Thomas Jespersen <laumann.thomas@gmail.com>
> ---
>  drivers/staging/erofs/internal.h | 4 ++--
>  drivers/staging/erofs/utils.c    | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
> index 57575c7f5635..bf180a803446 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
>  	/* spin if it is temporarily locked at the reclaim path */
>  	if (unlikely(o == locked)) {
>  #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> -		do
> +		do {
>  			cpu_relax();
> -		while (atomic_read(&grp->refcount) == locked);
> +		} while (atomic_read(&grp->refcount) == locked);

That looks like valid code to me, why change this?

greg k-h

[PATCH] staging: erofs: Add braces to do-while statements

[Greg KH]

On Sun, Dec 09, 2018@04:59:00PM +0100, Thomas Jespersen wrote:
> This fixes warning reported by sparse (with -Wsparse-all).

Why is sparse warning about this?

> Signed-off-by: Thomas Jespersen <laumann.thomas at gmail.com>
> ---
>  drivers/staging/erofs/internal.h | 4 ++--
>  drivers/staging/erofs/utils.c    | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
> index 57575c7f5635..bf180a803446 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
>  	/* spin if it is temporarily locked at the reclaim path */
>  	if (unlikely(o == locked)) {
>  #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> -		do
> +		do {
>  			cpu_relax();
> -		while (atomic_read(&grp->refcount) == locked);
> +		} while (atomic_read(&grp->refcount) == locked);

That looks like valid code to me, why change this?

greg k-h

Re: [PATCH] staging: erofs: Add braces to do-while statements

[Joe Perches]

On Sun, 2018-12-09 at 17:20 +0100, Greg KH wrote:
> On Sun, Dec 09, 2018 at 04:59:00PM +0100, Thomas Jespersen wrote:
> > This fixes warning reported by sparse (with -Wsparse-all).
> 
> Why is sparse warning about this?

Probably because it's the kernel preferred style
to use single statement

	do {
		<foo>;
	} while (<bar>);

over

by about a 20:1 ratio.

> > Signed-off-by: Thomas Jespersen <laumann.thomas@gmail.com>obab
> > ---
> >  drivers/staging/erofs/internal.h | 4 ++--
> >  drivers/staging/erofs/utils.c    | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
> > index 57575c7f5635..bf180a803446 100644
> > --- a/drivers/staging/erofs/internal.h
> > +++ b/drivers/staging/erofs/internal.h
> > @@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
> >  	/* spin if it is temporarily locked at the reclaim path */
> >  	if (unlikely(o == locked)) {
> >  #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> > -		do
> > +		do {
> >  			cpu_relax();
> > -		while (atomic_read(&grp->refcount) == locked);
> > +		} while (atomic_read(&grp->refcount) == locked);
> 
> That looks like valid code to me, why change this?
> 
> greg k-h

[PATCH] staging: erofs: Add braces to do-while statements

[Joe Perches]

On Sun, 2018-12-09@17:20 +0100, Greg KH wrote:
> On Sun, Dec 09, 2018@04:59:00PM +0100, Thomas Jespersen wrote:
> > This fixes warning reported by sparse (with -Wsparse-all).
> 
> Why is sparse warning about this?

Probably because it's the kernel preferred style
to use single statement

	do {
		<foo>;
	} while (<bar>);

over

by about a 20:1 ratio.

> > Signed-off-by: Thomas Jespersen <laumann.thomas at gmail.com>obab
> > ---
> >  drivers/staging/erofs/internal.h | 4 ++--
> >  drivers/staging/erofs/utils.c    | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
> > index 57575c7f5635..bf180a803446 100644
> > --- a/drivers/staging/erofs/internal.h
> > +++ b/drivers/staging/erofs/internal.h
> > @@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
> >  	/* spin if it is temporarily locked at the reclaim path */
> >  	if (unlikely(o == locked)) {
> >  #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> > -		do
> > +		do {
> >  			cpu_relax();
> > -		while (atomic_read(&grp->refcount) == locked);
> > +		} while (atomic_read(&grp->refcount) == locked);
> 
> That looks like valid code to me, why change this?
> 
> greg k-h

Re: [PATCH] staging: erofs: Add braces to do-while statements

[Joe Perches]

(mrph.  premature send)

On Sun, 2018-12-09 at 17:20 +0100, Greg KH wrote:
> On Sun, Dec 09, 2018 at 04:59:00PM +0100, Thomas Jespersen wrote:
> > This fixes warning reported by sparse (with -Wsparse-all).
> 
> Why is sparse warning about this?

Probably because it's the kernel preferred style
to use single statement

	do {
		<foo>;
	} while (<bar>);

over

	do
		<foo>;
	while (<bar>);

by about a 20:1 ratio.

[PATCH] staging: erofs: Add braces to do-while statements

[Joe Perches]

(mrph.  premature send)

On Sun, 2018-12-09@17:20 +0100, Greg KH wrote:
> On Sun, Dec 09, 2018@04:59:00PM +0100, Thomas Jespersen wrote:
> > This fixes warning reported by sparse (with -Wsparse-all).
> 
> Why is sparse warning about this?

Probably because it's the kernel preferred style
to use single statement

	do {
		<foo>;
	} while (<bar>);

over

	do
		<foo>;
	while (<bar>);

by about a 20:1 ratio.

Re: [PATCH] staging: erofs: Add braces to do-while statements

[Greg KH]

On Sun, Dec 09, 2018 at 09:27:01AM -0800, Joe Perches wrote:
> On Sun, 2018-12-09 at 17:20 +0100, Greg KH wrote:
> > On Sun, Dec 09, 2018 at 04:59:00PM +0100, Thomas Jespersen wrote:
> > > This fixes warning reported by sparse (with -Wsparse-all).
> > 
> > Why is sparse warning about this?
> 
> Probably because it's the kernel preferred style
> to use single statement
> 
> 	do {
> 		<foo>;
> 	} while (<bar>);
> 
> over
> 
> by about a 20:1 ratio.

Sparse is spitting out coding style complaints now?

[PATCH] staging: erofs: Add braces to do-while statements

[Greg KH]

On Sun, Dec 09, 2018@09:27:01AM -0800, Joe Perches wrote:
> On Sun, 2018-12-09@17:20 +0100, Greg KH wrote:
> > On Sun, Dec 09, 2018@04:59:00PM +0100, Thomas Jespersen wrote:
> > > This fixes warning reported by sparse (with -Wsparse-all).
> > 
> > Why is sparse warning about this?
> 
> Probably because it's the kernel preferred style
> to use single statement
> 
> 	do {
> 		<foo>;
> 	} while (<bar>);
> 
> over
> 
> by about a 20:1 ratio.

Sparse is spitting out coding style complaints now?

Re: [PATCH] staging: erofs: Add braces to do-while statements

[Gao Xiang]

[oops... I found Thomas sent the exactly the same email again.]

On 2018/12/9 23:59, Thomas Jespersen wrote:
> This fixes warning reported by sparse (with -Wsparse-all).
>
> Signed-off-by: Thomas Jespersen <laumann.thomas@gmail.com>
> ---
>  drivers/staging/erofs/internal.h | 4 ++--
>  drivers/staging/erofs/utils.c    | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
> index 57575c7f5635..bf180a803446 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
>  	/* spin if it is temporarily locked at the reclaim path */
>  	if (unlikely(o == locked)) {
>  #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> -		do
> +		do {
>  			cpu_relax();
> -		while (atomic_read(&grp->refcount) == locked);
> +		} while (atomic_read(&grp->refcount) == locked);
>  #endif
>  		goto repeat;
>  	}

Could you please check the latest source code? I cannot find the above code...

So I think this patch cannot be applied directly :(

https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/erofs/internal.h?h=staging-next#n260


... and why you send this patch without modification...

https://lists.ozlabs.org/pipermail/linux-erofs/2018-December/001099.html

https://lists.ozlabs.org/pipermail/linux-erofs/2018-December/001100.html


Thanks,

Gao Xiang

[PATCH] staging: erofs: Add braces to do-while statements

[Gao Xiang]

[oops... I found Thomas sent the exactly the same email again.]

On 2018/12/9 23:59, Thomas Jespersen wrote:
> This fixes warning reported by sparse (with -Wsparse-all).
>
> Signed-off-by: Thomas Jespersen <laumann.thomas at gmail.com>
> ---
>  drivers/staging/erofs/internal.h | 4 ++--
>  drivers/staging/erofs/utils.c    | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
> index 57575c7f5635..bf180a803446 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
>  	/* spin if it is temporarily locked at the reclaim path */
>  	if (unlikely(o == locked)) {
>  #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> -		do
> +		do {
>  			cpu_relax();
> -		while (atomic_read(&grp->refcount) == locked);
> +		} while (atomic_read(&grp->refcount) == locked);
>  #endif
>  		goto repeat;
>  	}

Could you please check the latest source code? I cannot find the above code...

So I think this patch cannot be applied directly :(

https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/erofs/internal.h?h=staging-next#n260


... and why you send this patch without modification...

https://lists.ozlabs.org/pipermail/linux-erofs/2018-December/001099.html

https://lists.ozlabs.org/pipermail/linux-erofs/2018-December/001100.html


Thanks,

Gao Xiang

Re: [PATCH] staging: erofs: Add braces to do-while statements

[Luc Van Oostenryck]

On Sun, Dec 09, 2018 at 08:40:29PM +0100, Greg KH wrote:
> On Sun, Dec 09, 2018 at 09:27:01AM -0800, Joe Perches wrote:
> > On Sun, 2018-12-09 at 17:20 +0100, Greg KH wrote:
> > > On Sun, Dec 09, 2018 at 04:59:00PM +0100, Thomas Jespersen wrote:
> > > > This fixes warning reported by sparse (with -Wsparse-all).
> > > 
> > > Why is sparse warning about this?
> > 
> > Probably because it's the kernel preferred style
> > to use single statement
> > 
> > 	do {
> > 		<foo>;
> > 	} while (<bar>);
> > 
> > over
> > 
> > by about a 20:1 ratio.
> 
> Sparse is spitting out coding style complaints now?

To my own surprise, it's effectively sparse that complained:
	warning: do-while statement is not a compound statement

with the flag -Wsparse-all (or -Wdo-while).
But -Wsparse-all is not a flag normally used for the kernel
(or normaly used on anything at all).

-- Luc

[PATCH] staging: erofs: Add braces to do-while statements

[Luc Van Oostenryck]

On Sun, Dec 09, 2018@08:40:29PM +0100, Greg KH wrote:
> On Sun, Dec 09, 2018@09:27:01AM -0800, Joe Perches wrote:
> > On Sun, 2018-12-09@17:20 +0100, Greg KH wrote:
> > > On Sun, Dec 09, 2018@04:59:00PM +0100, Thomas Jespersen wrote:
> > > > This fixes warning reported by sparse (with -Wsparse-all).
> > > 
> > > Why is sparse warning about this?
> > 
> > Probably because it's the kernel preferred style
> > to use single statement
> > 
> > 	do {
> > 		<foo>;
> > 	} while (<bar>);
> > 
> > over
> > 
> > by about a 20:1 ratio.
> 
> Sparse is spitting out coding style complaints now?

To my own surprise, it's effectively sparse that complained:
	warning: do-while statement is not a compound statement

with the flag -Wsparse-all (or -Wdo-while).
But -Wsparse-all is not a flag normally used for the kernel
(or normaly used on anything at all).

-- Luc

[PATCH] staging: erofs: Add braces to do-while statements

[高翔]

Hi Thomas,

On 2018/12/9 6:07, Thomas Jespersen wrote:
> This fixes warning reported by sparse (with -Wsparse-all).
> 
> Signed-off-by: Thomas Jespersen <laumann.thomas at gmail.com>
> ---
>  drivers/staging/erofs/internal.h | 4 ++--
>  drivers/staging/erofs/utils.c    | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
> index 57575c7f56355424a..bf180a803446655d4 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
>  	/* spin if it is temporarily locked at the reclaim path */
>  	if (unlikely(o == locked)) {
>  #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> -		do
> +		do {
>  			cpu_relax();
> -		while (atomic_read(&grp->refcount) == locked);
> +		} while (atomic_read(&grp->refcount) == locked);
>  #endif
>  		goto repeat;
>  	}


Could you check the latest source code at
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/erofs?h=staging-next
the above you sent doesn't seem the latest source code...

and send your patch to Greg and the staging mailing list as well? if you
want to upstream immediately.

To:
  Greg Kroah-Hartman         <gregkh at linuxfoundation.org>
  linux-staging mailing list <devel at driverdev.osuosl.org>

  linux-erofs mailing list   <linux-erofs at lists.ozlabs.org>
  Gao Xiang                  <gaoxiang25 at huawei.com>
  Chao Yu                    <yuchao0 at huawei.com>

Thanks,
Gao Xiang

[PATCH] staging: erofs: Add braces to do-while statements

[Thomas Jespersen]

This fixes warning reported by sparse (with -Wsparse-all).

Signed-off-by: Thomas Jespersen <laumann.thomas at gmail.com>
---
 drivers/staging/erofs/internal.h | 4 ++--
 drivers/staging/erofs/utils.c    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 57575c7f56355424a..bf180a803446655d4 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
 	/* spin if it is temporarily locked at the reclaim path */
 	if (unlikely(o == locked)) {
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-		do
+		do {
 			cpu_relax();
-		while (atomic_read(&grp->refcount) == locked);
+		} while (atomic_read(&grp->refcount) == locked);
 #endif
 		goto repeat;
 	}
diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
index ea8a962e5c9504a2c..cd76df4a48c734eb4 100644
--- a/drivers/staging/erofs/utils.c
+++ b/drivers/staging/erofs/utils.c
@@ -217,9 +217,9 @@ unsigned long erofs_shrink_scan(struct shrinker *shrink,
 	unsigned long freed = 0;
 
 	spin_lock(&erofs_sb_list_lock);
-	do
+	do {
 		run_no = ++shrinker_run_no;
-	while (run_no == 0);
+	} while (run_no == 0);
 
 	/* Iterate over all mounted superblocks and try to shrink them */
 	p = erofs_sb_list.next;
-- 
2.19.2