iwlwifi: mvm: BACKPORT_WANT_DEV_COREDUMP?

iwlwifi: mvm: BACKPORT_WANT_DEV_COREDUMP?

[Paul Bolle]

Your commit aadede6e9f4c ("iwlwifi: mvm: port to devcoredump framework")
landed in today's linux-next (next-20141031). It adds a select statement
for BACKPORT_WANT_DEV_COREDUMP. There's no Kconfig symbol 
BACKPORT_WANT_DEV_COREDUMP so this select is currently a nop. (In
https://lkml.org/lkml/2014/9/30/578 I proposed a patch that emits a
warning in cases like this.)

Did you perhaps meant to select WANT_DEV_COREDUMP? Or is the Kconfig
symbol BACKPORT_WANT_DEV_COREDUMP queued somewhere?


Paul Bolle

Re: iwlwifi: mvm: BACKPORT_WANT_DEV_COREDUMP?

[Johannes Berg]

On Fri, 2014-10-31 at 09:40 +0100, Paul Bolle wrote:
> Your commit aadede6e9f4c ("iwlwifi: mvm: port to devcoredump framework")
> landed in today's linux-next (next-20141031). It adds a select statement
> for BACKPORT_WANT_DEV_COREDUMP. There's no Kconfig symbol 
> BACKPORT_WANT_DEV_COREDUMP so this select is currently a nop. (In
> https://lkml.org/lkml/2014/9/30/578 I proposed a patch that emits a
> warning in cases like this.)
> 
> Did you perhaps meant to select WANT_DEV_COREDUMP?

Yes. We'll fix it up in the iwlwifi tree.

Thanks for the report!

johannes

Re: iwlwifi: mvm: BACKPORT_WANT_DEV_COREDUMP?

[Paul Bolle]

On Fri, 2014-10-31 at 09:45 +0100, Johannes Berg wrote:
> On Fri, 2014-10-31 at 09:40 +0100, Paul Bolle wrote:
> > Your commit aadede6e9f4c ("iwlwifi: mvm: port to devcoredump framework")
> > landed in today's linux-next (next-20141031). It adds a select statement
> > for BACKPORT_WANT_DEV_COREDUMP. There's no Kconfig symbol 
> > BACKPORT_WANT_DEV_COREDUMP so this select is currently a nop. (In
> > https://lkml.org/lkml/2014/9/30/578 I proposed a patch that emits a
> > warning in cases like this.)
> > 
> > Did you perhaps meant to select WANT_DEV_COREDUMP?
> 
> Yes. We'll fix it up in the iwlwifi tree.
> 
> Thanks for the report!

Perhaps you could also look into somehow guarding the call of
dev_coredumpm(), that this commit added, with checks for
CONFIG_DEV_COREDUMP. See, I had a quick look at all this and selecting
WANT_DEV_COREDUMP might not be enough, because DISABLE_DEV_COREDUMP can
still, well, disable DEV_COREDUMP. Or am I misreading the Kconfig
symbols that regulate DEV_COREDUMP?

Thanks,


Paul Bolle

Re: iwlwifi: mvm: BACKPORT_WANT_DEV_COREDUMP?

[Johannes Berg]

On Fri, 2014-10-31 at 10:06 +0100, Paul Bolle wrote:
> On Fri, 2014-10-31 at 09:45 +0100, Johannes Berg wrote:
> > On Fri, 2014-10-31 at 09:40 +0100, Paul Bolle wrote:
> > > Your commit aadede6e9f4c ("iwlwifi: mvm: port to devcoredump framework")
> > > landed in today's linux-next (next-20141031). It adds a select statement
> > > for BACKPORT_WANT_DEV_COREDUMP. There's no Kconfig symbol 
> > > BACKPORT_WANT_DEV_COREDUMP so this select is currently a nop. (In
> > > https://lkml.org/lkml/2014/9/30/578 I proposed a patch that emits a
> > > warning in cases like this.)
> > > 
> > > Did you perhaps meant to select WANT_DEV_COREDUMP?
> > 
> > Yes. We'll fix it up in the iwlwifi tree.
> > 
> > Thanks for the report!
> 
> Perhaps you could also look into somehow guarding the call of
> dev_coredumpm(), that this commit added, with checks for
> CONFIG_DEV_COREDUMP. See, I had a quick look at all this and selecting
> WANT_DEV_COREDUMP might not be enough, because DISABLE_DEV_COREDUMP can
> still, well, disable DEV_COREDUMP. Or am I misreading the Kconfig
> symbols that regulate DEV_COREDUMP?

No, you're correctly reading that. However, the devcoredump header file
provides simple functions in this case. That means there's some extra
work (allocating and filling the buffer just to free it immediately) but
it simplifies the code.

johannes

Re: iwlwifi: mvm: BACKPORT_WANT_DEV_COREDUMP?

[Paul Bolle]

On Fri, 2014-10-31 at 10:08 +0100, Johannes Berg wrote:
> On Fri, 2014-10-31 at 10:06 +0100, Paul Bolle wrote:
> > Perhaps you could also look into somehow guarding the call of
> > dev_coredumpm(), that this commit added, with checks for
> > CONFIG_DEV_COREDUMP. See, I had a quick look at all this and selecting
> > WANT_DEV_COREDUMP might not be enough, because DISABLE_DEV_COREDUMP can
> > still, well, disable DEV_COREDUMP. Or am I misreading the Kconfig
> > symbols that regulate DEV_COREDUMP?
> 
> No, you're correctly reading that. However, the devcoredump header file
> provides simple functions in this case. That means there's some extra
> work (allocating and filling the buffer just to free it immediately) but
> it simplifies the code.

I see. More than a quick look was required here. Thanks for explaining
this!


Paul Bolle