This fixes warning reported by sparse (with -Wsparse-all).
Signed-off-by: Thomas Jespersen <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>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
(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.
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?
[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 <[email protected]>
> ---
> 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
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