linux-next: build failure after merge of the pm tree (and warning)

linux-next: build failure after merge of the pm tree (and warning)

[Stephen Rothwell]

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

Hi Rafael,

After merging the pm tree, today's linux-next build (x86_64 allmodconfig)
failed like this:

drivers/dma/dmatest.c: In function 'dmatest_func':
drivers/dma/dmatest.c:255:2: error: implicit declaration of function 'set_freezable_with_signal' [-Werror=implicit-function-declaration]

Caused by commit cd3bc8fbc2d5 ("freezer: kill unused
set_freezable_with_signal()") interacting with commit 981ed70d8e4f
("dmatest: make dmatest threads freezable") from Linus' tree (merged into
v3.2-rc1).

I reverted the pm tree commit for today.

I also noticed this warning:

drivers/dma/dmatest.c: In function 'dmatest_add_channel':
drivers/dma/dmatest.c:594:28: warning: the omitted middle operand in ?: will always be 'true', suggest explicit middle operand [-Wparentheses]

Which has been there since 2009 ... I think points out an error in the
code (looking at the lines above there).  The two occurrences above were
fixed in commit f1aef8b6e6ab ("dmaengine: dmatest: correct thread_count
while using multiple thread per channel") but the third came in via a
different branch (commit 58691d64c44a "dmatest: add pq support") and was
not corrected.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: build failure after merge of the pm tree (and warning)

[Rafael J. Wysocki]

On Monday, November 14, 2011, Stephen Rothwell wrote:
> Hi Rafael,
> 
> After merging the pm tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
> 
> drivers/dma/dmatest.c: In function 'dmatest_func':
> drivers/dma/dmatest.c:255:2: error: implicit declaration of function 'set_freezable_with_signal' [-Werror=implicit-function-declaration]
> 
> Caused by commit cd3bc8fbc2d5 ("freezer: kill unused
> set_freezable_with_signal()") interacting with commit 981ed70d8e4f
> ("dmatest: make dmatest threads freezable") from Linus' tree (merged into
> v3.2-rc1).
> 
> I reverted the pm tree commit for today.

Thanks, I've added the following patch to linux-pm/linux-next, which should
fix this problem (Tejun, if that's not the right thing to do, please let me
know).

Rafael


---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: Freezer: Make dmatest_func() use set_freezable()

According to the commit cd3bc8fbc2d55ae0918184fb34992054dc4eb710
(freezer: kill unused set_freezable_with_signal()) changelog,
it should be sufficient to use set_freezable() instead of
set_freezable_with_signal(), which has been removed, in
dmatest_func(), so do that and fix a build issue.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/dma/dmatest.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/dma/dmatest.c
===================================================================
--- linux.orig/drivers/dma/dmatest.c
+++ linux/drivers/dma/dmatest.c
@@ -252,7 +252,7 @@ static int dmatest_func(void *data)
 	int			i;
 
 	thread_name = current->comm;
-	set_freezable_with_signal();
+	set_freezable();
 
 	ret = -ENOMEM;
 

Re: linux-next: build failure after merge of the pm tree (and warning)

[Tejun Heo]

Hello,

2011/11/14 Rafael J. Wysocki <rjw@sisk.pl>:
> Thanks, I've added the following patch to linux-pm/linux-next, which should
> fix this problem (Tejun, if that's not the right thing to do, please let me
> know).

Working on a proper fix now.

Thanks.

-- 
tejun

Re: linux-next: build failure after merge of the pm tree (and warning)

[Rafael J. Wysocki]

On Tuesday, November 15, 2011, Tejun Heo wrote:
> Hello,
> 
> 2011/11/14 Rafael J. Wysocki <rjw@sisk.pl>:
> > Thanks, I've added the following patch to linux-pm/linux-next, which should
> > fix this problem (Tejun, if that's not the right thing to do, please let me
> > know).
> 
> Working on a proper fix now.

OK, thanks!

Rafael

Re: linux-next: build failure after merge of the pm tree (and warning)

[Guennadi Liakhovetski]

Hi Rafael

On Tue, 15 Nov 2011, Rafael J. Wysocki wrote:

> On Monday, November 14, 2011, Stephen Rothwell wrote:
> > Hi Rafael,
> > 
> > After merging the pm tree, today's linux-next build (x86_64 allmodconfig)
> > failed like this:
> > 
> > drivers/dma/dmatest.c: In function 'dmatest_func':
> > drivers/dma/dmatest.c:255:2: error: implicit declaration of function 'set_freezable_with_signal' [-Werror=implicit-function-declaration]
> > 
> > Caused by commit cd3bc8fbc2d5 ("freezer: kill unused
> > set_freezable_with_signal()") interacting with commit 981ed70d8e4f
> > ("dmatest: make dmatest threads freezable") from Linus' tree (merged into
> > v3.2-rc1).
> > 
> > I reverted the pm tree commit for today.
> 
> Thanks, I've added the following patch to linux-pm/linux-next, which should
> fix this problem (Tejun, if that's not the right thing to do, please let me
> know).
> 
> Rafael
> 
> 
> ---
> From: Rafael J. Wysocki <rjw@sisk.pl>
> Subject: Freezer: Make dmatest_func() use set_freezable()
> 
> According to the commit cd3bc8fbc2d55ae0918184fb34992054dc4eb710
> (freezer: kill unused set_freezable_with_signal()) changelog,
> it should be sufficient to use set_freezable() instead of
> set_freezable_with_signal(), which has been removed, in
> dmatest_func(), so do that and fix a build issue.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/dma/dmatest.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux/drivers/dma/dmatest.c
> ===================================================================
> --- linux.orig/drivers/dma/dmatest.c
> +++ linux/drivers/dma/dmatest.c
> @@ -252,7 +252,7 @@ static int dmatest_func(void *data)
>  	int			i;
>  
>  	thread_name = current->comm;
> -	set_freezable_with_signal();
> +	set_freezable();

No, this isn't a correct fix.

Thanks
Guennadi

>  
>  	ret = -ENOMEM;
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: linux-next: build failure after merge of the pm tree (and warning)

[Tejun Heo]

Hello, Guennadi.

On Tue, Nov 15, 2011 at 12:33:29AM +0100, Guennadi Liakhovetski wrote:
> No, this isn't a correct fix.

I've been looking at the code and am scratching my head about what
happens after timeout.  If test timed out (tmo == 0), it prints out
error message and continues to the next iteration of the test loop,
which will reinitialize the on-stack completion.  This essentially
makes the previous test run's callback_param pointer dangling.  The
completion needs to be either detached from the callback or waited
upon even if it timed out.  Am I missing something?

Thanks.

-- 
tejun

Re: linux-next: build failure after merge of the pm tree (and warning)

[Williams, Dan J]

[ sorry I've been out of town... ]

On Mon, Nov 14, 2011 at 3:39 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Guennadi.
>
> On Tue, Nov 15, 2011 at 12:33:29AM +0100, Guennadi Liakhovetski wrote:
>> No, this isn't a correct fix.
>
> I've been looking at the code and am scratching my head about what
> happens after timeout.  If test timed out (tmo == 0), it prints out
> error message and continues to the next iteration of the test loop,
> which will reinitialize the on-stack completion.  This essentially
> makes the previous test run's callback_param pointer dangling.  The
> completion needs to be either detached from the callback or waited
> upon even if it timed out.  Am I missing something?

If the completion times out then it is almost certainly a hardware or
driver bug.  There is no facility to get a dma driver to forget a
queued operation.  The test should probably abort at that point, but
it isn't intended to be a recoverable condition.

--
Dan

Re: linux-next: build failure after merge of the pm tree (and warning)

[Rafael J. Wysocki]

On Tuesday, November 15, 2011, Guennadi Liakhovetski wrote:
> Hi Rafael
> 
> On Tue, 15 Nov 2011, Rafael J. Wysocki wrote:
> 
> > On Monday, November 14, 2011, Stephen Rothwell wrote:
> > > Hi Rafael,
> > > 
> > > After merging the pm tree, today's linux-next build (x86_64 allmodconfig)
> > > failed like this:
> > > 
> > > drivers/dma/dmatest.c: In function 'dmatest_func':
> > > drivers/dma/dmatest.c:255:2: error: implicit declaration of function 'set_freezable_with_signal' [-Werror=implicit-function-declaration]
> > > 
> > > Caused by commit cd3bc8fbc2d5 ("freezer: kill unused
> > > set_freezable_with_signal()") interacting with commit 981ed70d8e4f
> > > ("dmatest: make dmatest threads freezable") from Linus' tree (merged into
> > > v3.2-rc1).
> > > 
> > > I reverted the pm tree commit for today.
> > 
> > Thanks, I've added the following patch to linux-pm/linux-next, which should
> > fix this problem (Tejun, if that's not the right thing to do, please let me
> > know).
> > 
> > Rafael
> > 
> > 
> > ---
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > Subject: Freezer: Make dmatest_func() use set_freezable()
> > 
> > According to the commit cd3bc8fbc2d55ae0918184fb34992054dc4eb710
> > (freezer: kill unused set_freezable_with_signal()) changelog,
> > it should be sufficient to use set_freezable() instead of
> > set_freezable_with_signal(), which has been removed, in
> > dmatest_func(), so do that and fix a build issue.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  drivers/dma/dmatest.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Index: linux/drivers/dma/dmatest.c
> > ===================================================================
> > --- linux.orig/drivers/dma/dmatest.c
> > +++ linux/drivers/dma/dmatest.c
> > @@ -252,7 +252,7 @@ static int dmatest_func(void *data)
> >  	int			i;
> >  
> >  	thread_name = current->comm;
> > -	set_freezable_with_signal();
> > +	set_freezable();
> 
> No, this isn't a correct fix.

OK, I'll drop it, then.

Thanks,
Rafael