Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1718448 > unrolled thread

[PATCH 1/2] nfit: Use init_completion() in acpi_nfit_flush_probe()

Started byBoqun Feng <boqun.feng@gmail.com>
First post2017-08-23 17:30 +0200
Last post2017-08-25 02:40 +0200
Articles 8 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/2] nfit: Use init_completion() in acpi_nfit_flush_probe() Boqun Feng <boqun.feng@gmail.com> - 2017-08-23 17:30 +0200
    Re: [PATCH 1/2] nfit: Use init_completion() in acpi_nfit_flush_probe() Dan Williams <dan.j.williams@intel.com> - 2017-08-24 00:10 +0200
    Re: [PATCH 1/2] nfit: Use init_completion() in  acpi_nfit_flush_probe() Thomas Gleixner <tglx@linutronix.de> - 2017-08-24 15:10 +0200
      Re: [PATCH 1/2] nfit: Use init_completion() in  acpi_nfit_flush_probe() Boqun Feng <boqun.feng@gmail.com> - 2017-08-24 15:30 +0200
        Re: [PATCH 1/2] nfit: Use init_completion() in acpi_nfit_flush_probe() Arnd Bergmann <arnd@arndb.de> - 2017-08-24 15:50 +0200
    [PATCH v2 1/2] nfit: Fix the abuse of COMPLETION_INITIALIZER_ONSTACK() Boqun Feng <boqun.feng@gmail.com> - 2017-08-24 16:30 +0200
      Re: [PATCH v2 1/2] nfit: Fix the abuse of  COMPLETION_INITIALIZER_ONSTACK() Boqun Feng <boqun.feng@gmail.com> - 2017-08-25 02:20 +0200
      Re: [PATCH v2 1/2] nfit: Fix the abuse of COMPLETION_INITIALIZER_ONSTACK() Dan Williams <dan.j.williams@intel.com> - 2017-08-25 02:40 +0200

#1718448 — [PATCH 1/2] nfit: Use init_completion() in acpi_nfit_flush_probe()

FromBoqun Feng <boqun.feng@gmail.com>
Date2017-08-23 17:30 +0200
Subject[PATCH 1/2] nfit: Use init_completion() in acpi_nfit_flush_probe()
Message-ID<uhFLs-2fj-9@gated-at.bofh.it>
There is no need to use COMPLETION_INITIALIZER_ONSTACK() in
acpi_nfit_flush_probe(), replace it with init_completion().

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 drivers/acpi/nfit/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 19182d091587..1893e416e7c0 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2884,7 +2884,7 @@ static int acpi_nfit_flush_probe(struct nvdimm_bus_descriptor *nd_desc)
 	 * need to be interruptible while waiting.
 	 */
 	INIT_WORK_ONSTACK(&flush.work, flush_probe);
-	COMPLETION_INITIALIZER_ONSTACK(flush.cmp);
+	init_completion(&flush.cmp);
 	queue_work(nfit_wq, &flush.work);
 	mutex_unlock(&acpi_desc->init_mutex);
 
-- 
2.14.1

[toc] | [next] | [standalone]

#1718669

FromDan Williams <dan.j.williams@intel.com>
Date2017-08-24 00:10 +0200
Message-ID<uhM0y-6hF-11@gated-at.bofh.it>
In reply to#1718448
On Wed, Aug 23, 2017 at 8:25 AM, Boqun Feng <boqun.feng@gmail.com> wrote:
> There is no need to use COMPLETION_INITIALIZER_ONSTACK() in
> acpi_nfit_flush_probe(), replace it with init_completion().

Now that I see the better version of this patch with the improved
changelog in the -mm tree...

Acked-by: Dan Williams <dan.j.williams@intel.com>

[toc] | [prev] | [next] | [standalone]

#1719223 — Re: [PATCH 1/2] nfit: Use init_completion() in acpi_nfit_flush_probe()

FromThomas Gleixner <tglx@linutronix.de>
Date2017-08-24 15:10 +0200
SubjectRe: [PATCH 1/2] nfit: Use init_completion() in acpi_nfit_flush_probe()
Message-ID<ui03x-70q-25@gated-at.bofh.it>
In reply to#1718448
On Wed, 23 Aug 2017, Boqun Feng wrote:

> There is no need to use COMPLETION_INITIALIZER_ONSTACK() in
> acpi_nfit_flush_probe(), replace it with init_completion().

You completely fail to explain WHY.

Thanks,

	tglx

 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  drivers/acpi/nfit/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 19182d091587..1893e416e7c0 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -2884,7 +2884,7 @@ static int acpi_nfit_flush_probe(struct nvdimm_bus_descriptor *nd_desc)
>  	 * need to be interruptible while waiting.
>  	 */
>  	INIT_WORK_ONSTACK(&flush.work, flush_probe);
> -	COMPLETION_INITIALIZER_ONSTACK(flush.cmp);
> +	init_completion(&flush.cmp);
>  	queue_work(nfit_wq, &flush.work);
>  	mutex_unlock(&acpi_desc->init_mutex);
>  
> -- 
> 2.14.1
> 
>

[toc] | [prev] | [next] | [standalone]

#1719234 — Re: [PATCH 1/2] nfit: Use init_completion() in acpi_nfit_flush_probe()

FromBoqun Feng <boqun.feng@gmail.com>
Date2017-08-24 15:30 +0200
SubjectRe: [PATCH 1/2] nfit: Use init_completion() in acpi_nfit_flush_probe()
Message-ID<ui0mS-78Q-15@gated-at.bofh.it>
In reply to#1719223

[Multipart message — attachments visible in raw view] — view raw

On Thu, Aug 24, 2017 at 03:07:42PM +0200, Thomas Gleixner wrote:
> On Wed, 23 Aug 2017, Boqun Feng wrote:
> 
> > There is no need to use COMPLETION_INITIALIZER_ONSTACK() in
> > acpi_nfit_flush_probe(), replace it with init_completion().
> 
> You completely fail to explain WHY.
> 

I thought COMPLETION_INITIALIZER_ONSTACK() should only use in assigment
or compound literals, so the usage here is obviously wrong, but seems
I was wrong?

Ingo,

Is the usage of COMPLETION_INITIALIZER_ONSTACK() correct? If not,
I could rephrase my commit log saying this is a fix for wrong usage of
COMPLETION_INITIALIZER_ONSTACK(), otherwise, I will rewrite the commit
indicating this patch is a necessary dependency for patch #2. Thanks!

Regards,
Boqun

> Thanks,
> 
> 	tglx
> 
>  
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> >  drivers/acpi/nfit/core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> > index 19182d091587..1893e416e7c0 100644
> > --- a/drivers/acpi/nfit/core.c
> > +++ b/drivers/acpi/nfit/core.c
> > @@ -2884,7 +2884,7 @@ static int acpi_nfit_flush_probe(struct nvdimm_bus_descriptor *nd_desc)
> >  	 * need to be interruptible while waiting.
> >  	 */
> >  	INIT_WORK_ONSTACK(&flush.work, flush_probe);
> > -	COMPLETION_INITIALIZER_ONSTACK(flush.cmp);
> > +	init_completion(&flush.cmp);
> >  	queue_work(nfit_wq, &flush.work);
> >  	mutex_unlock(&acpi_desc->init_mutex);
> >  
> > -- 
> > 2.14.1
> > 
> >

[toc] | [prev] | [next] | [standalone]

#1719252

FromArnd Bergmann <arnd@arndb.de>
Date2017-08-24 15:50 +0200
Message-ID<ui0Ge-7fv-23@gated-at.bofh.it>
In reply to#1719234
On Thu, Aug 24, 2017 at 3:28 PM, Boqun Feng <boqun.feng@gmail.com> wrote:
> On Thu, Aug 24, 2017 at 03:07:42PM +0200, Thomas Gleixner wrote:
>> On Wed, 23 Aug 2017, Boqun Feng wrote:
>>
>> > There is no need to use COMPLETION_INITIALIZER_ONSTACK() in
>> > acpi_nfit_flush_probe(), replace it with init_completion().
>>
>> You completely fail to explain WHY.
>>
>
> I thought COMPLETION_INITIALIZER_ONSTACK() should only use in assigment
> or compound literals, so the usage here is obviously wrong, but seems
> I was wrong?
>
> Ingo,
>
> Is the usage of COMPLETION_INITIALIZER_ONSTACK() correct? If not,
> I could rephrase my commit log saying this is a fix for wrong usage of
> COMPLETION_INITIALIZER_ONSTACK(), otherwise, I will rewrite the commit
> indicating this patch is a necessary dependency for patch #2. Thanks!

I think your patch is correct, but your changelog text is useless, as
Thomas mentioned: you should instead explain that it breaks with the
other fix in the series, and what the difference between init_completion()
and COMPLETION_INITIALIZER_ONSTACK() is.

      Arnd

[toc] | [prev] | [next] | [standalone]

#1719313 — [PATCH v2 1/2] nfit: Fix the abuse of COMPLETION_INITIALIZER_ONSTACK()

FromBoqun Feng <boqun.feng@gmail.com>
Date2017-08-24 16:30 +0200
Subject[PATCH v2 1/2] nfit: Fix the abuse of COMPLETION_INITIALIZER_ONSTACK()
Message-ID<ui1iZ-7Ir-67@gated-at.bofh.it>
In reply to#1718448
COMPLETION_INITIALIZER_ONSTACK() is supposed to used as an initializer,
in other words, it should only be used in assignment expressions or
compound literals. So the usage in drivers/acpi/nfit/core.c:

	COMPLETION_INITIALIZER_ONSTACK(flush.cmp);

, is inappropriate.

Besides, this usage could also break compilations for another fix to
reduce stack sizes caused by COMPLETION_INITIALIZER_ONSTACK(), because
that fix changes COMPLETION_INITIALIZER_ONSTACK() from rvalue to lvalue,
and usage as above will report error:

	drivers/acpi/nfit/core.c: In function 'acpi_nfit_flush_probe':
	include/linux/completion.h:77:3: error: value computed is not used [-Werror=unused-value]
	  (*({ init_completion(&work); &work; }))

This patch fixes this by replacing COMPLETION_INITIALIZER_ONSTACK() with
init_completion() in acpi_nfit_flush_probe(), which does the same
initialization without any other problem.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 drivers/acpi/nfit/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 19182d091587..1893e416e7c0 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2884,7 +2884,7 @@ static int acpi_nfit_flush_probe(struct nvdimm_bus_descriptor *nd_desc)
 	 * need to be interruptible while waiting.
 	 */
 	INIT_WORK_ONSTACK(&flush.work, flush_probe);
-	COMPLETION_INITIALIZER_ONSTACK(flush.cmp);
+	init_completion(&flush.cmp);
 	queue_work(nfit_wq, &flush.work);
 	mutex_unlock(&acpi_desc->init_mutex);
 
-- 
2.14.1

[toc] | [prev] | [next] | [standalone]

#1719666 — Re: [PATCH v2 1/2] nfit: Fix the abuse of COMPLETION_INITIALIZER_ONSTACK()

FromBoqun Feng <boqun.feng@gmail.com>
Date2017-08-25 02:20 +0200
SubjectRe: [PATCH v2 1/2] nfit: Fix the abuse of COMPLETION_INITIALIZER_ONSTACK()
Message-ID<uiavT-5eJ-15@gated-at.bofh.it>
In reply to#1719313

[Multipart message — attachments visible in raw view] — view raw

On Thu, Aug 24, 2017 at 10:22:36PM +0800, Boqun Feng wrote:
> COMPLETION_INITIALIZER_ONSTACK() is supposed to used as an initializer,
> in other words, it should only be used in assignment expressions or
> compound literals. So the usage in drivers/acpi/nfit/core.c:
> 
> 	COMPLETION_INITIALIZER_ONSTACK(flush.cmp);
> 
> , is inappropriate.
> 
> Besides, this usage could also break compilations for another fix to
> reduce stack sizes caused by COMPLETION_INITIALIZER_ONSTACK(), because
> that fix changes COMPLETION_INITIALIZER_ONSTACK() from rvalue to lvalue,
> and usage as above will report error:
> 
> 	drivers/acpi/nfit/core.c: In function 'acpi_nfit_flush_probe':
> 	include/linux/completion.h:77:3: error: value computed is not used [-Werror=unused-value]
> 	  (*({ init_completion(&work); &work; }))
> 
> This patch fixes this by replacing COMPLETION_INITIALIZER_ONSTACK() with
> init_completion() in acpi_nfit_flush_probe(), which does the same
> initialization without any other problem.
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---

Sorry, forget to metion:

v1 --> v2:
	Improve the commit log, based on Dan, Thomas and Arnd's
	comments.

Only V2 of this patch #1 is updated. 

Regards,
Boqun

>  drivers/acpi/nfit/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 19182d091587..1893e416e7c0 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -2884,7 +2884,7 @@ static int acpi_nfit_flush_probe(struct nvdimm_bus_descriptor *nd_desc)
>  	 * need to be interruptible while waiting.
>  	 */
>  	INIT_WORK_ONSTACK(&flush.work, flush_probe);
> -	COMPLETION_INITIALIZER_ONSTACK(flush.cmp);
> +	init_completion(&flush.cmp);
>  	queue_work(nfit_wq, &flush.work);
>  	mutex_unlock(&acpi_desc->init_mutex);
>  
> -- 
> 2.14.1
>

[toc] | [prev] | [next] | [standalone]

#1719670 — Re: [PATCH v2 1/2] nfit: Fix the abuse of COMPLETION_INITIALIZER_ONSTACK()

FromDan Williams <dan.j.williams@intel.com>
Date2017-08-25 02:40 +0200
SubjectRe: [PATCH v2 1/2] nfit: Fix the abuse of COMPLETION_INITIALIZER_ONSTACK()
Message-ID<uiaPf-5kN-3@gated-at.bofh.it>
In reply to#1719313
On Thu, Aug 24, 2017 at 7:22 AM, Boqun Feng <boqun.feng@gmail.com> wrote:
> COMPLETION_INITIALIZER_ONSTACK() is supposed to used as an initializer,
> in other words, it should only be used in assignment expressions or
> compound literals. So the usage in drivers/acpi/nfit/core.c:
>
>         COMPLETION_INITIALIZER_ONSTACK(flush.cmp);
>
> , is inappropriate.
>
> Besides, this usage could also break compilations for another fix to
> reduce stack sizes caused by COMPLETION_INITIALIZER_ONSTACK(), because
> that fix changes COMPLETION_INITIALIZER_ONSTACK() from rvalue to lvalue,
> and usage as above will report error:
>
>         drivers/acpi/nfit/core.c: In function 'acpi_nfit_flush_probe':
>         include/linux/completion.h:77:3: error: value computed is not used [-Werror=unused-value]
>           (*({ init_completion(&work); &work; }))
>
> This patch fixes this by replacing COMPLETION_INITIALIZER_ONSTACK() with
> init_completion() in acpi_nfit_flush_probe(), which does the same
> initialization without any other problem.
>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>

Acked-by: Dan Williams <dan.j.williams@intel.com>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web