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

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

[Boqun Feng]

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

--
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>

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

[Dan Williams]

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>

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

[Thomas Gleixner]

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
> 
> 

--
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>

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

[Boqun Feng]

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

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
> > 
> > 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Arnd Bergmann]

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

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

[Boqun Feng]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 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

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

[Boqun Feng]

[-- Attachment #1: Type: text/plain, Size: 1921 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Dan Williams]

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>

--
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>