2014-02-10 18:44:20

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: [GIT PULL] (xen) stable/for-jens-3.14

Hey Jens,

Please git pull the following branch:

git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git stable/for-jens-3.14

which is based off v3.13-rc6. If you would like me to rebase it on
a different branch/tag I would be more than happy to do so.

The patches are all bug-fixes and hopefully can go in 3.14.

They deal with xen-blkback shutdown and cause memory leaks
as well as shutdown races. They should go to stable tree and if you
are OK with I will ask them to backport those fixes.

There is also a fix to xen-blkfront to deal with unexpected state
transition. And lastly a fix to the header where it was using the
__aligned__ unnecessarily.

Please pull!


drivers/block/xen-blkback/blkback.c | 63 ++++++++++++++++++++++++-------------
drivers/block/xen-blkback/common.h | 4 ++-
drivers/block/xen-blkback/xenbus.c | 13 ++++++++
drivers/block/xen-blkfront.c | 11 ++++---
include/xen/interface/io/blkif.h | 34 +++++++++-----------
5 files changed, 79 insertions(+), 46 deletions(-)

David Vrabel (1):
xen-blkfront: handle backend CLOSED without CLOSING

Matt Rushton (1):
xen-blkback: fix memory leak when persistent grants are used

Roger Pau Monne (3):
xen-blkback: fix memory leaks
xen-blkback: fix shutdown race
xen-blkif: drop struct blkif_request_segment_aligned


Attachments:
(No filename) (1.30 kB)
(No filename) (473.00 B)
Download all attachments

2014-02-10 19:54:15

by Jens Axboe

[permalink] [raw]
Subject: Re: [GIT PULL] (xen) stable/for-jens-3.14

On Mon, Feb 10 2014, Konrad Rzeszutek Wilk wrote:
> Hey Jens,
>
> Please git pull the following branch:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git stable/for-jens-3.14
>
> which is based off v3.13-rc6. If you would like me to rebase it on
> a different branch/tag I would be more than happy to do so.

Older is fine, it's only an issue if you are ahead of the branch you
want to go into.

dd>
> The patches are all bug-fixes and hopefully can go in 3.14.
>
> They deal with xen-blkback shutdown and cause memory leaks
> as well as shutdown races. They should go to stable tree and if you
> are OK with I will ask them to backport those fixes.
>
> There is also a fix to xen-blkfront to deal with unexpected state
> transition. And lastly a fix to the header where it was using the
> __aligned__ unnecessarily.

Pulled!

--
Jens Axboe

2014-02-11 15:52:22

by Sander Eikelenboom

[permalink] [raw]
Subject: Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-3.14 : NFO: trying to register non-static key. the code is fine but needs lockdep annotation.

Hi Konrad,

Today decided to tryout another kernel RC and your pull request to Jens on top of it .. and I encoutered this one:


[ 438.029756] INFO: trying to register non-static key.
[ 438.029759] the code is fine but needs lockdep annotation.
[ 438.029760] turning off the locking correctness validator.
[ 438.029770] CPU: 3 PID: 9593 Comm: blkback.2.xvda Tainted: G W 3.14.0-rc2-20140211-pcireset-net-btrevert-xenblock+ #1
[ 438.029773] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS V1.8B1 09/13/2010
[ 438.029784] ffff88005224c4f0 ffff88004e5d9b68 ffffffff81b808c4 ffff88004ba2b510
[ 438.029791] 0000000000000002 ffff88004e5d9c38 ffffffff81116eab ffff88004e5d9bf8
[ 438.029798] ffffffff81117b35 0000000000000000 0000000000000000 ffffffff82cee570
[ 438.029799] Call Trace:
[ 438.029815] [<ffffffff81b808c4>] dump_stack+0x46/0x58
[ 438.029826] [<ffffffff81116eab>] __lock_acquire+0x1c2b/0x2220
[ 438.029833] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
[ 438.029841] [<ffffffff81117b0d>] lock_acquire+0xbd/0x150
[ 438.029847] [<ffffffff810e5965>] ? flush_work+0x5/0x290
[ 438.029852] [<ffffffff810e599d>] flush_work+0x3d/0x290
[ 438.029856] [<ffffffff810e5965>] ? flush_work+0x5/0x290
[ 438.029863] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
[ 438.029872] [<ffffffff816fef01>] ? xen_blkif_schedule+0x1a1/0x8d0
[ 438.029881] [<ffffffff81b8ae0d>] ? _raw_spin_unlock_irqrestore+0x6d/0x90
[ 438.029888] [<ffffffff8111392b>] ? trace_hardirqs_on_caller+0xfb/0x240
[ 438.029894] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
[ 438.029901] [<ffffffff816fefe9>] xen_blkif_schedule+0x289/0x8d0
[ 438.029907] [<ffffffff8110d510>] ? __init_waitqueue_head+0x60/0x60
[ 438.029913] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
[ 438.029919] [<ffffffff81b8ae21>] ? _raw_spin_unlock_irqrestore+0x81/0x90
[ 438.029925] [<ffffffff816fed60>] ? xen_blkif_be_int+0x40/0x40
[ 438.029932] [<ffffffff810ee374>] kthread+0xe4/0x100
[ 438.029938] [<ffffffff81b8afe0>] ? _raw_spin_unlock_irq+0x30/0x50
[ 438.029946] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70
[ 438.029951] [<ffffffff81b8c1fc>] ret_from_fork+0x7c/0xb0
[ 438.029958] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70

Doesn't seem to serious .. but never the less :-)

--

Sander


Monday, February 10, 2014, 8:54:02 PM, you wrote:

> On Mon, Feb 10 2014, Konrad Rzeszutek Wilk wrote:
>> Hey Jens,
>>
>> Please git pull the following branch:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git stable/for-jens-3.14
>>
>> which is based off v3.13-rc6. If you would like me to rebase it on
>> a different branch/tag I would be more than happy to do so.

> Older is fine, it's only an issue if you are ahead of the branch you
> want to go into.

dd>>
>> The patches are all bug-fixes and hopefully can go in 3.14.
>>
>> They deal with xen-blkback shutdown and cause memory leaks
>> as well as shutdown races. They should go to stable tree and if you
>> are OK with I will ask them to backport those fixes.
>>
>> There is also a fix to xen-blkfront to deal with unexpected state
>> transition. And lastly a fix to the header where it was using the
>> __aligned__ unnecessarily.

> Pulled!

2014-02-11 15:57:11

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-3.14 : NFO: trying to register non-static key. the code is fine but needs lockdep annotation.

On Tue, Feb 11, 2014 at 04:52:15PM +0100, Sander Eikelenboom wrote:
> Hi Konrad,
>
> Today decided to tryout another kernel RC and your pull request to Jens on top of it .. and I encoutered this one:

Thank you for testing!

Could you provide the .config file please?

Did you see this _before_ the pull request with Jens? I presume
not, but just double checking?

And lastly - what were you doing when you triggered this? Just launching
a guest?

CC-ing Roger and other folks who were on the patches.

>
>
> [ 438.029756] INFO: trying to register non-static key.
> [ 438.029759] the code is fine but needs lockdep annotation.
> [ 438.029760] turning off the locking correctness validator.
> [ 438.029770] CPU: 3 PID: 9593 Comm: blkback.2.xvda Tainted: G W 3.14.0-rc2-20140211-pcireset-net-btrevert-xenblock+ #1
> [ 438.029773] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS V1.8B1 09/13/2010
> [ 438.029784] ffff88005224c4f0 ffff88004e5d9b68 ffffffff81b808c4 ffff88004ba2b510
> [ 438.029791] 0000000000000002 ffff88004e5d9c38 ffffffff81116eab ffff88004e5d9bf8
> [ 438.029798] ffffffff81117b35 0000000000000000 0000000000000000 ffffffff82cee570
> [ 438.029799] Call Trace:
> [ 438.029815] [<ffffffff81b808c4>] dump_stack+0x46/0x58
> [ 438.029826] [<ffffffff81116eab>] __lock_acquire+0x1c2b/0x2220
> [ 438.029833] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
> [ 438.029841] [<ffffffff81117b0d>] lock_acquire+0xbd/0x150
> [ 438.029847] [<ffffffff810e5965>] ? flush_work+0x5/0x290
> [ 438.029852] [<ffffffff810e599d>] flush_work+0x3d/0x290
> [ 438.029856] [<ffffffff810e5965>] ? flush_work+0x5/0x290
> [ 438.029863] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
> [ 438.029872] [<ffffffff816fef01>] ? xen_blkif_schedule+0x1a1/0x8d0
> [ 438.029881] [<ffffffff81b8ae0d>] ? _raw_spin_unlock_irqrestore+0x6d/0x90
> [ 438.029888] [<ffffffff8111392b>] ? trace_hardirqs_on_caller+0xfb/0x240
> [ 438.029894] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
> [ 438.029901] [<ffffffff816fefe9>] xen_blkif_schedule+0x289/0x8d0
> [ 438.029907] [<ffffffff8110d510>] ? __init_waitqueue_head+0x60/0x60
> [ 438.029913] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
> [ 438.029919] [<ffffffff81b8ae21>] ? _raw_spin_unlock_irqrestore+0x81/0x90
> [ 438.029925] [<ffffffff816fed60>] ? xen_blkif_be_int+0x40/0x40
> [ 438.029932] [<ffffffff810ee374>] kthread+0xe4/0x100
> [ 438.029938] [<ffffffff81b8afe0>] ? _raw_spin_unlock_irq+0x30/0x50
> [ 438.029946] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70
> [ 438.029951] [<ffffffff81b8c1fc>] ret_from_fork+0x7c/0xb0
> [ 438.029958] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70
>
> Doesn't seem to serious .. but never the less :-)
>
> --
>
> Sander
>
>
> Monday, February 10, 2014, 8:54:02 PM, you wrote:
>
> > On Mon, Feb 10 2014, Konrad Rzeszutek Wilk wrote:
> >> Hey Jens,
> >>
> >> Please git pull the following branch:
> >>
> >> git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git stable/for-jens-3.14
> >>
> >> which is based off v3.13-rc6. If you would like me to rebase it on
> >> a different branch/tag I would be more than happy to do so.
>
> > Older is fine, it's only an issue if you are ahead of the branch you
> > want to go into.
>
> dd>>
> >> The patches are all bug-fixes and hopefully can go in 3.14.
> >>
> >> They deal with xen-blkback shutdown and cause memory leaks
> >> as well as shutdown races. They should go to stable tree and if you
> >> are OK with I will ask them to backport those fixes.
> >>
> >> There is also a fix to xen-blkfront to deal with unexpected state
> >> transition. And lastly a fix to the header where it was using the
> >> __aligned__ unnecessarily.
>
> > Pulled!
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2014-02-11 16:07:58

by Sander Eikelenboom

[permalink] [raw]
Subject: Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-3.14 : NFO: trying to register non-static key. the code is fine but needs lockdep annotation.


Tuesday, February 11, 2014, 4:56:50 PM, you wrote:

> On Tue, Feb 11, 2014 at 04:52:15PM +0100, Sander Eikelenboom wrote:
>> Hi Konrad,
>>
>> Today decided to tryout another kernel RC and your pull request to Jens on top of it .. and I encoutered this one:

> Thank you for testing!

> Could you provide the .config file please?

Attached

> Did you see this _before_ the pull request with Jens? I presume
> not, but just double checking?

Nope not too my knowledge (though it's a bit messy with things broken on 3.14 at the moment)

> And lastly - what were you doing when you triggered this? Just launching
> a guest?

Nope it triggers on guest shutdown ..


> CC-ing Roger and other folks who were on the patches.

>>
>>
>> [ 438.029756] INFO: trying to register non-static key.
>> [ 438.029759] the code is fine but needs lockdep annotation.
>> [ 438.029760] turning off the locking correctness validator.
>> [ 438.029770] CPU: 3 PID: 9593 Comm: blkback.2.xvda Tainted: G W 3.14.0-rc2-20140211-pcireset-net-btrevert-xenblock+ #1
>> [ 438.029773] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS V1.8B1 09/13/2010
>> [ 438.029784] ffff88005224c4f0 ffff88004e5d9b68 ffffffff81b808c4 ffff88004ba2b510
>> [ 438.029791] 0000000000000002 ffff88004e5d9c38 ffffffff81116eab ffff88004e5d9bf8
>> [ 438.029798] ffffffff81117b35 0000000000000000 0000000000000000 ffffffff82cee570
>> [ 438.029799] Call Trace:
>> [ 438.029815] [<ffffffff81b808c4>] dump_stack+0x46/0x58
>> [ 438.029826] [<ffffffff81116eab>] __lock_acquire+0x1c2b/0x2220
>> [ 438.029833] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
>> [ 438.029841] [<ffffffff81117b0d>] lock_acquire+0xbd/0x150
>> [ 438.029847] [<ffffffff810e5965>] ? flush_work+0x5/0x290
>> [ 438.029852] [<ffffffff810e599d>] flush_work+0x3d/0x290
>> [ 438.029856] [<ffffffff810e5965>] ? flush_work+0x5/0x290
>> [ 438.029863] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
>> [ 438.029872] [<ffffffff816fef01>] ? xen_blkif_schedule+0x1a1/0x8d0
>> [ 438.029881] [<ffffffff81b8ae0d>] ? _raw_spin_unlock_irqrestore+0x6d/0x90
>> [ 438.029888] [<ffffffff8111392b>] ? trace_hardirqs_on_caller+0xfb/0x240
>> [ 438.029894] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
>> [ 438.029901] [<ffffffff816fefe9>] xen_blkif_schedule+0x289/0x8d0
>> [ 438.029907] [<ffffffff8110d510>] ? __init_waitqueue_head+0x60/0x60
>> [ 438.029913] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
>> [ 438.029919] [<ffffffff81b8ae21>] ? _raw_spin_unlock_irqrestore+0x81/0x90
>> [ 438.029925] [<ffffffff816fed60>] ? xen_blkif_be_int+0x40/0x40
>> [ 438.029932] [<ffffffff810ee374>] kthread+0xe4/0x100
>> [ 438.029938] [<ffffffff81b8afe0>] ? _raw_spin_unlock_irq+0x30/0x50
>> [ 438.029946] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70
>> [ 438.029951] [<ffffffff81b8c1fc>] ret_from_fork+0x7c/0xb0
>> [ 438.029958] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70
>>
>> Doesn't seem to serious .. but never the less :-)
>>
>> --
>>
>> Sander
>>
>>
>> Monday, February 10, 2014, 8:54:02 PM, you wrote:
>>
>> > On Mon, Feb 10 2014, Konrad Rzeszutek Wilk wrote:
>> >> Hey Jens,
>> >>
>> >> Please git pull the following branch:
>> >>
>> >> git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git stable/for-jens-3.14
>> >>
>> >> which is based off v3.13-rc6. If you would like me to rebase it on
>> >> a different branch/tag I would be more than happy to do so.
>>
>> > Older is fine, it's only an issue if you are ahead of the branch you
>> > want to go into.
>>
>> dd>>
>> >> The patches are all bug-fixes and hopefully can go in 3.14.
>> >>
>> >> They deal with xen-blkback shutdown and cause memory leaks
>> >> as well as shutdown races. They should go to stable tree and if you
>> >> are OK with I will ask them to backport those fixes.
>> >>
>> >> There is also a fix to xen-blkfront to deal with unexpected state
>> >> transition. And lastly a fix to the header where it was using the
>> >> __aligned__ unnecessarily.
>>
>> > Pulled!
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/


Attachments:
.config (92.63 kB)

2014-02-11 17:40:08

by Roger Pau Monne

[permalink] [raw]
Subject: Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-3.14 : NFO: trying to register non-static key. the code is fine but needs lockdep annotation.

On 11/02/14 17:07, Sander Eikelenboom wrote:
>
> Tuesday, February 11, 2014, 4:56:50 PM, you wrote:
>
>> On Tue, Feb 11, 2014 at 04:52:15PM +0100, Sander Eikelenboom wrote:
>>> Hi Konrad,
>>>
>>> Today decided to tryout another kernel RC and your pull request to Jens on top of it .. and I encoutered this one:
>
>> Thank you for testing!
>
>> Could you provide the .config file please?
>
> Attached
>
>> Did you see this _before_ the pull request with Jens? I presume
>> not, but just double checking?
>
> Nope not too my knowledge (though it's a bit messy with things broken on 3.14 at the moment)
>
>> And lastly - what were you doing when you triggered this? Just launching
>> a guest?
>
> Nope it triggers on guest shutdown ..
>
>
>> CC-ing Roger and other folks who were on the patches.
>
>>>
>>>
>>> [ 438.029756] INFO: trying to register non-static key.
>>> [ 438.029759] the code is fine but needs lockdep annotation.
>>> [ 438.029760] turning off the locking correctness validator.
>>> [ 438.029770] CPU: 3 PID: 9593 Comm: blkback.2.xvda Tainted: G W 3.14.0-rc2-20140211-pcireset-net-btrevert-xenblock+ #1
>>> [ 438.029773] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS V1.8B1 09/13/2010
>>> [ 438.029784] ffff88005224c4f0 ffff88004e5d9b68 ffffffff81b808c4 ffff88004ba2b510
>>> [ 438.029791] 0000000000000002 ffff88004e5d9c38 ffffffff81116eab ffff88004e5d9bf8
>>> [ 438.029798] ffffffff81117b35 0000000000000000 0000000000000000 ffffffff82cee570
>>> [ 438.029799] Call Trace:
>>> [ 438.029815] [<ffffffff81b808c4>] dump_stack+0x46/0x58
>>> [ 438.029826] [<ffffffff81116eab>] __lock_acquire+0x1c2b/0x2220
>>> [ 438.029833] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
>>> [ 438.029841] [<ffffffff81117b0d>] lock_acquire+0xbd/0x150
>>> [ 438.029847] [<ffffffff810e5965>] ? flush_work+0x5/0x290
>>> [ 438.029852] [<ffffffff810e599d>] flush_work+0x3d/0x290
>>> [ 438.029856] [<ffffffff810e5965>] ? flush_work+0x5/0x290
>>> [ 438.029863] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
>>> [ 438.029872] [<ffffffff816fef01>] ? xen_blkif_schedule+0x1a1/0x8d0
>>> [ 438.029881] [<ffffffff81b8ae0d>] ? _raw_spin_unlock_irqrestore+0x6d/0x90
>>> [ 438.029888] [<ffffffff8111392b>] ? trace_hardirqs_on_caller+0xfb/0x240
>>> [ 438.029894] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
>>> [ 438.029901] [<ffffffff816fefe9>] xen_blkif_schedule+0x289/0x8d0
>>> [ 438.029907] [<ffffffff8110d510>] ? __init_waitqueue_head+0x60/0x60
>>> [ 438.029913] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
>>> [ 438.029919] [<ffffffff81b8ae21>] ? _raw_spin_unlock_irqrestore+0x81/0x90
>>> [ 438.029925] [<ffffffff816fed60>] ? xen_blkif_be_int+0x40/0x40
>>> [ 438.029932] [<ffffffff810ee374>] kthread+0xe4/0x100
>>> [ 438.029938] [<ffffffff81b8afe0>] ? _raw_spin_unlock_irq+0x30/0x50
>>> [ 438.029946] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70
>>> [ 438.029951] [<ffffffff81b8c1fc>] ret_from_fork+0x7c/0xb0
>>> [ 438.029958] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70
>>>
>>> Doesn't seem to serious .. but never the less :-)

Thanks for the report!

Does the following patch solve the problem?

---
commit c1460953d081c8a18ac9e84fe90f696cdceae105
Author: Roger Pau Monne <[email protected]>
Date: Tue Feb 11 17:21:19 2014 +0100

xen-blkback: init persistent_purge_work work_struct

Do a dummy initialization of the persistent_purge_work
work_struct on xen_blkif_alloc, so that when flush_work is called on
shutdown the struct is initialized even if it hasn't been used.

Signed-off-by: Roger Pau Monn? <[email protected]>

diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 84973c6..3df7575 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -129,6 +129,17 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
blkif->free_pages_num = 0;
atomic_set(&blkif->persistent_gnt_in_use, 0);
atomic_set(&blkif->inflight, 0);
+ /*
+ * Init the work struct with a NULL function, this is done
+ * so that flush_work doesn't complain when shutting down if
+ * persistent_purge_work has not been used during the lifetime
+ * of this blkback instance.
+ *
+ * NB: In purge_persistent_gnt we make sure that
+ * persistent_purge_work is always correctly setup with a valid
+ * function pointer before being scheduled.
+ */
+ INIT_WORK(&blkif->persistent_purge_work, NULL);

INIT_LIST_HEAD(&blkif->pending_free);


2014-02-11 17:52:27

by David Vrabel

[permalink] [raw]
Subject: Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-3.14 : NFO: trying to register non-static key. the code is fine but needs lockdep annotation.

On 11/02/14 17:40, Roger Pau Monn? wrote:
> On 11/02/14 17:07, Sander Eikelenboom wrote:
>>
>> Tuesday, February 11, 2014, 4:56:50 PM, you wrote:
>>
>>> On Tue, Feb 11, 2014 at 04:52:15PM +0100, Sander Eikelenboom wrote:
>>>> Hi Konrad,
>>>>
>>>> Today decided to tryout another kernel RC and your pull request to Jens on top of it .. and I encoutered this one:
>>
>>> Thank you for testing!
>>
>>> Could you provide the .config file please?
>>
>> Attached
>>
>>> Did you see this _before_ the pull request with Jens? I presume
>>> not, but just double checking?
>>
>> Nope not too my knowledge (though it's a bit messy with things broken on 3.14 at the moment)
>>
>>> And lastly - what were you doing when you triggered this? Just launching
>>> a guest?
>>
>> Nope it triggers on guest shutdown ..
>>
>>
>>> CC-ing Roger and other folks who were on the patches.
>>
>>>>
>>>>
>>>> [ 438.029756] INFO: trying to register non-static key.
>>>> [ 438.029759] the code is fine but needs lockdep annotation.
>>>> [ 438.029760] turning off the locking correctness validator.
>>>> [ 438.029770] CPU: 3 PID: 9593 Comm: blkback.2.xvda Tainted: G W 3.14.0-rc2-20140211-pcireset-net-btrevert-xenblock+ #1
>>>> [ 438.029773] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS V1.8B1 09/13/2010
>>>> [ 438.029784] ffff88005224c4f0 ffff88004e5d9b68 ffffffff81b808c4 ffff88004ba2b510
>>>> [ 438.029791] 0000000000000002 ffff88004e5d9c38 ffffffff81116eab ffff88004e5d9bf8
>>>> [ 438.029798] ffffffff81117b35 0000000000000000 0000000000000000 ffffffff82cee570
>>>> [ 438.029799] Call Trace:
>>>> [ 438.029815] [<ffffffff81b808c4>] dump_stack+0x46/0x58
>>>> [ 438.029826] [<ffffffff81116eab>] __lock_acquire+0x1c2b/0x2220
>>>> [ 438.029833] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
>>>> [ 438.029841] [<ffffffff81117b0d>] lock_acquire+0xbd/0x150
>>>> [ 438.029847] [<ffffffff810e5965>] ? flush_work+0x5/0x290
>>>> [ 438.029852] [<ffffffff810e599d>] flush_work+0x3d/0x290
>>>> [ 438.029856] [<ffffffff810e5965>] ? flush_work+0x5/0x290
>>>> [ 438.029863] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
>>>> [ 438.029872] [<ffffffff816fef01>] ? xen_blkif_schedule+0x1a1/0x8d0
>>>> [ 438.029881] [<ffffffff81b8ae0d>] ? _raw_spin_unlock_irqrestore+0x6d/0x90
>>>> [ 438.029888] [<ffffffff8111392b>] ? trace_hardirqs_on_caller+0xfb/0x240
>>>> [ 438.029894] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
>>>> [ 438.029901] [<ffffffff816fefe9>] xen_blkif_schedule+0x289/0x8d0
>>>> [ 438.029907] [<ffffffff8110d510>] ? __init_waitqueue_head+0x60/0x60
>>>> [ 438.029913] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
>>>> [ 438.029919] [<ffffffff81b8ae21>] ? _raw_spin_unlock_irqrestore+0x81/0x90
>>>> [ 438.029925] [<ffffffff816fed60>] ? xen_blkif_be_int+0x40/0x40
>>>> [ 438.029932] [<ffffffff810ee374>] kthread+0xe4/0x100
>>>> [ 438.029938] [<ffffffff81b8afe0>] ? _raw_spin_unlock_irq+0x30/0x50
>>>> [ 438.029946] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70
>>>> [ 438.029951] [<ffffffff81b8c1fc>] ret_from_fork+0x7c/0xb0
>>>> [ 438.029958] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70
>>>>
>>>> Doesn't seem to serious .. but never the less :-)
>
> Thanks for the report!
>
> Does the following patch solve the problem?
>
> ---
> commit c1460953d081c8a18ac9e84fe90f696cdceae105
> Author: Roger Pau Monne <[email protected]>
> Date: Tue Feb 11 17:21:19 2014 +0100
>
> xen-blkback: init persistent_purge_work work_struct
>
> Do a dummy initialization of the persistent_purge_work
> work_struct on xen_blkif_alloc, so that when flush_work is called on
> shutdown the struct is initialized even if it hasn't been used.
>
> Signed-off-by: Roger Pau Monn? <[email protected]>
>
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index 84973c6..3df7575 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -129,6 +129,17 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
> blkif->free_pages_num = 0;
> atomic_set(&blkif->persistent_gnt_in_use, 0);
> atomic_set(&blkif->inflight, 0);
> + /*
> + * Init the work struct with a NULL function, this is done
> + * so that flush_work doesn't complain when shutting down if
> + * persistent_purge_work has not been used during the lifetime
> + * of this blkback instance.
> + *
> + * NB: In purge_persistent_gnt we make sure that
> + * persistent_purge_work is always correctly setup with a valid
> + * function pointer before being scheduled.
> + */
> + INIT_WORK(&blkif->persistent_purge_work, NULL);

I think you should init this fully here and remove the other call to
INIT_WORK.

David

2014-02-11 18:15:18

by Roger Pau Monne

[permalink] [raw]
Subject: Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-3.14 : NFO: trying to register non-static key. the code is fine but needs lockdep annotation.

On 11/02/14 18:52, David Vrabel wrote:
> On 11/02/14 17:40, Roger Pau Monn? wrote:
>> On 11/02/14 17:07, Sander Eikelenboom wrote:
>>>
>>> Tuesday, February 11, 2014, 4:56:50 PM, you wrote:
>>>
>>>> On Tue, Feb 11, 2014 at 04:52:15PM +0100, Sander Eikelenboom wrote:
>>>>> Hi Konrad,
>>>>>
>>>>> Today decided to tryout another kernel RC and your pull request to Jens on top of it .. and I encoutered this one:
>>>
>>>> Thank you for testing!
>>>
>>>> Could you provide the .config file please?
>>>
>>> Attached
>>>
>>>> Did you see this _before_ the pull request with Jens? I presume
>>>> not, but just double checking?
>>>
>>> Nope not too my knowledge (though it's a bit messy with things broken on 3.14 at the moment)
>>>
>>>> And lastly - what were you doing when you triggered this? Just launching
>>>> a guest?
>>>
>>> Nope it triggers on guest shutdown ..
>>>
>>>
>>>> CC-ing Roger and other folks who were on the patches.
>>>
>>>>>
>>>>>
>>>>> [ 438.029756] INFO: trying to register non-static key.
>>>>> [ 438.029759] the code is fine but needs lockdep annotation.
>>>>> [ 438.029760] turning off the locking correctness validator.
>>>>> [ 438.029770] CPU: 3 PID: 9593 Comm: blkback.2.xvda Tainted: G W 3.14.0-rc2-20140211-pcireset-net-btrevert-xenblock+ #1
>>>>> [ 438.029773] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS V1.8B1 09/13/2010
>>>>> [ 438.029784] ffff88005224c4f0 ffff88004e5d9b68 ffffffff81b808c4 ffff88004ba2b510
>>>>> [ 438.029791] 0000000000000002 ffff88004e5d9c38 ffffffff81116eab ffff88004e5d9bf8
>>>>> [ 438.029798] ffffffff81117b35 0000000000000000 0000000000000000 ffffffff82cee570
>>>>> [ 438.029799] Call Trace:
>>>>> [ 438.029815] [<ffffffff81b808c4>] dump_stack+0x46/0x58
>>>>> [ 438.029826] [<ffffffff81116eab>] __lock_acquire+0x1c2b/0x2220
>>>>> [ 438.029833] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
>>>>> [ 438.029841] [<ffffffff81117b0d>] lock_acquire+0xbd/0x150
>>>>> [ 438.029847] [<ffffffff810e5965>] ? flush_work+0x5/0x290
>>>>> [ 438.029852] [<ffffffff810e599d>] flush_work+0x3d/0x290
>>>>> [ 438.029856] [<ffffffff810e5965>] ? flush_work+0x5/0x290
>>>>> [ 438.029863] [<ffffffff81117b35>] ? lock_acquire+0xe5/0x150
>>>>> [ 438.029872] [<ffffffff816fef01>] ? xen_blkif_schedule+0x1a1/0x8d0
>>>>> [ 438.029881] [<ffffffff81b8ae0d>] ? _raw_spin_unlock_irqrestore+0x6d/0x90
>>>>> [ 438.029888] [<ffffffff8111392b>] ? trace_hardirqs_on_caller+0xfb/0x240
>>>>> [ 438.029894] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
>>>>> [ 438.029901] [<ffffffff816fefe9>] xen_blkif_schedule+0x289/0x8d0
>>>>> [ 438.029907] [<ffffffff8110d510>] ? __init_waitqueue_head+0x60/0x60
>>>>> [ 438.029913] [<ffffffff81113a7d>] ? trace_hardirqs_on+0xd/0x10
>>>>> [ 438.029919] [<ffffffff81b8ae21>] ? _raw_spin_unlock_irqrestore+0x81/0x90
>>>>> [ 438.029925] [<ffffffff816fed60>] ? xen_blkif_be_int+0x40/0x40
>>>>> [ 438.029932] [<ffffffff810ee374>] kthread+0xe4/0x100
>>>>> [ 438.029938] [<ffffffff81b8afe0>] ? _raw_spin_unlock_irq+0x30/0x50
>>>>> [ 438.029946] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70
>>>>> [ 438.029951] [<ffffffff81b8c1fc>] ret_from_fork+0x7c/0xb0
>>>>> [ 438.029958] [<ffffffff810ee290>] ? __init_kthread_worker+0x70/0x70
>>>>>
>>>>> Doesn't seem to serious .. but never the less :-)
>>
>> Thanks for the report!
>>
>> Does the following patch solve the problem?
>>
>> ---
>> commit c1460953d081c8a18ac9e84fe90f696cdceae105
>> Author: Roger Pau Monne <[email protected]>
>> Date: Tue Feb 11 17:21:19 2014 +0100
>>
>> xen-blkback: init persistent_purge_work work_struct
>>
>> Do a dummy initialization of the persistent_purge_work
>> work_struct on xen_blkif_alloc, so that when flush_work is called on
>> shutdown the struct is initialized even if it hasn't been used.
>>
>> Signed-off-by: Roger Pau Monn? <[email protected]>
>>
>> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
>> index 84973c6..3df7575 100644
>> --- a/drivers/block/xen-blkback/xenbus.c
>> +++ b/drivers/block/xen-blkback/xenbus.c
>> @@ -129,6 +129,17 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
>> blkif->free_pages_num = 0;
>> atomic_set(&blkif->persistent_gnt_in_use, 0);
>> atomic_set(&blkif->inflight, 0);
>> + /*
>> + * Init the work struct with a NULL function, this is done
>> + * so that flush_work doesn't complain when shutting down if
>> + * persistent_purge_work has not been used during the lifetime
>> + * of this blkback instance.
>> + *
>> + * NB: In purge_persistent_gnt we make sure that
>> + * persistent_purge_work is always correctly setup with a valid
>> + * function pointer before being scheduled.
>> + */
>> + INIT_WORK(&blkif->persistent_purge_work, NULL);
>
> I think you should init this fully here and remove the other call to
> INIT_WORK.

That would mean that unmap_purged_grants would no longer be static and
I should also add a prototype for it in blkback/common.h, which is kind
of ugly IMHO.

---
commit 980e72e45454b64ccb7f23b6794a769384e51038
Author: Roger Pau Monne <[email protected]>
Date: Tue Feb 11 19:04:06 2014 +0100

xen-blkback: init persistent_purge_work work_struct

Initialize persistent_purge_work work_struct on xen_blkif_alloc (and
remove the previous initialization done in purge_persistent_gnt). This
prevents flush_work from complaining even if purge_persistent_gnt has
not been used.

Signed-off-by: Roger Pau Monn? <[email protected]>

diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index e612627..10cd50b 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -299,7 +299,7 @@ static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
BUG_ON(num != 0);
}

-static void unmap_purged_grants(struct work_struct *work)
+void xen_blkbk_unmap_purged_grants(struct work_struct *work)
{
struct gnttab_unmap_grant_ref unmap[BLKIF_MAX_SEGMENTS_PER_REQUEST];
struct page *pages[BLKIF_MAX_SEGMENTS_PER_REQUEST];
@@ -420,7 +420,6 @@ finished:
blkif->vbd.overflow_max_grants = 0;

/* We can defer this work */
- INIT_WORK(&blkif->persistent_purge_work, unmap_purged_grants);
schedule_work(&blkif->persistent_purge_work);
pr_debug(DRV_PFX "Purged %u/%u\n", (total - num_clean), total);
return;
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index 9eb34e2..be05277 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -385,6 +385,7 @@ int xen_blkbk_flush_diskcache(struct xenbus_transaction xbt,
int xen_blkbk_barrier(struct xenbus_transaction xbt,
struct backend_info *be, int state);
struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be);
+void xen_blkbk_unmap_purged_grants(struct work_struct *work);

static inline void blkif_get_x86_32_req(struct blkif_request *dst,
struct blkif_x86_32_request *src)
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 84973c6..9a547e6 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -129,6 +129,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
blkif->free_pages_num = 0;
atomic_set(&blkif->persistent_gnt_in_use, 0);
atomic_set(&blkif->inflight, 0);
+ INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);

INIT_LIST_HEAD(&blkif->pending_free);


2014-02-11 18:22:00

by David Vrabel

[permalink] [raw]
Subject: Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-3.14 : NFO: trying to register non-static key. the code is fine but needs lockdep annotation.

On 11/02/14 18:15, Roger Pau Monn? wrote:
> On 11/02/14 18:52, David Vrabel wrote:
>>
> That would mean that unmap_purged_grants would no longer be static and
> I should also add a prototype for it in blkback/common.h, which is kind
> of ugly IMHO.

But less ugly than initializing work with a NULL function, IMO.

> commit 980e72e45454b64ccb7f23b6794a769384e51038
> Author: Roger Pau Monne <[email protected]>
> Date: Tue Feb 11 19:04:06 2014 +0100
>
> xen-blkback: init persistent_purge_work work_struct
>
> Initialize persistent_purge_work work_struct on xen_blkif_alloc (and
> remove the previous initialization done in purge_persistent_gnt). This
> prevents flush_work from complaining even if purge_persistent_gnt has
> not been used.
>
> Signed-off-by: Roger Pau Monn? <[email protected]>

Reviewed-by: David Vrabel <[email protected]>

Thanks.

David

2014-02-11 20:22:05

by Sander Eikelenboom

[permalink] [raw]
Subject: Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-3.14 : NFO: trying to register non-static key. the code is fine but needs lockdep annotation.



Tuesday, February 11, 2014, 7:21:56 PM, you wrote:

> On 11/02/14 18:15, Roger Pau Monn? wrote:
>> On 11/02/14 18:52, David Vrabel wrote:
>>>
>> That would mean that unmap_purged_grants would no longer be static and
>> I should also add a prototype for it in blkback/common.h, which is kind
>> of ugly IMHO.

> But less ugly than initializing work with a NULL function, IMO.

>> commit 980e72e45454b64ccb7f23b6794a769384e51038
>> Author: Roger Pau Monne <[email protected]>
>> Date: Tue Feb 11 19:04:06 2014 +0100
>>
>> xen-blkback: init persistent_purge_work work_struct
>>
>> Initialize persistent_purge_work work_struct on xen_blkif_alloc (and
>> remove the previous initialization done in purge_persistent_gnt). This
>> prevents flush_work from complaining even if purge_persistent_gnt has
>> not been used.
>>
>> Signed-off-by: Roger Pau Monn? <[email protected]>

> Reviewed-by: David Vrabel <[email protected]>

And a Tested-by: Sander Eikelenboom <[email protected]>

Thanks !

> Thanks.

> David

2014-02-11 21:44:46

by Jens Axboe

[permalink] [raw]
Subject: Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-3.14 : NFO: trying to register non-static key. the code is fine but needs lockdep annotation.

On 2014-02-11 13:21, Sander Eikelenboom wrote:
>
> Tuesday, February 11, 2014, 7:21:56 PM, you wrote:
>
>> On 11/02/14 18:15, Roger Pau Monn? wrote:
>>> On 11/02/14 18:52, David Vrabel wrote:
>>>>
>>> That would mean that unmap_purged_grants would no longer be static and
>>> I should also add a prototype for it in blkback/common.h, which is kind
>>> of ugly IMHO.
>
>> But less ugly than initializing work with a NULL function, IMO.
>
>>> commit 980e72e45454b64ccb7f23b6794a769384e51038
>>> Author: Roger Pau Monne <[email protected]>
>>> Date: Tue Feb 11 19:04:06 2014 +0100
>>>
>>> xen-blkback: init persistent_purge_work work_struct
>>>
>>> Initialize persistent_purge_work work_struct on xen_blkif_alloc (and
>>> remove the previous initialization done in purge_persistent_gnt). This
>>> prevents flush_work from complaining even if purge_persistent_gnt has
>>> not been used.
>>>
>>> Signed-off-by: Roger Pau Monn? <[email protected]>
>
>> Reviewed-by: David Vrabel <[email protected]>
>
> And a Tested-by: Sander Eikelenboom <[email protected]>

Konrad, I was going to ship my current tree today, but looks like we
need this too.

--
Jens Axboe

2014-02-12 03:15:52

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-3.14 : NFO: trying to register non-static key. the code is fine but needs lockdep annotation.

On February 11, 2014 4:44:42 PM EST, Jens Axboe <[email protected]> wrote:
>On 2014-02-11 13:21, Sander Eikelenboom wrote:
>>
>> Tuesday, February 11, 2014, 7:21:56 PM, you wrote:
>>
>>> On 11/02/14 18:15, Roger Pau Monné wrote:
>>>> On 11/02/14 18:52, David Vrabel wrote:
>>>>>
>>>> That would mean that unmap_purged_grants would no longer be static
>and
>>>> I should also add a prototype for it in blkback/common.h, which is
>kind
>>>> of ugly IMHO.
>>
>>> But less ugly than initializing work with a NULL function, IMO.
>>
>>>> commit 980e72e45454b64ccb7f23b6794a769384e51038
>>>> Author: Roger Pau Monne <[email protected]>
>>>> Date: Tue Feb 11 19:04:06 2014 +0100
>>>>
>>>> xen-blkback: init persistent_purge_work work_struct
>>>>
>>>> Initialize persistent_purge_work work_struct on
>xen_blkif_alloc (and
>>>> remove the previous initialization done in
>purge_persistent_gnt). This
>>>> prevents flush_work from complaining even if
>purge_persistent_gnt has
>>>> not been used.
>>>>
>>>> Signed-off-by: Roger Pau MonnĂ© <[email protected]>
>>
>>> Reviewed-by: David Vrabel <[email protected]>
>>
>> And a Tested-by: Sander Eikelenboom <[email protected]>
>
>Konrad, I was going to ship my current tree today, but looks like we
>need this too.

Yes. Are you OK taking it from this thread or would you like me to prep a GIT pull with it?

Thanks!

2014-02-12 03:34:43

by Jens Axboe

[permalink] [raw]
Subject: Re: [Xen-devel] [GIT PULL] (xen) stable/for-jens-3.14 : NFO: trying to register non-static key. the code is fine but needs lockdep annotation.

On Tue, Feb 11 2014, Konrad Rzeszutek Wilk wrote:
> On February 11, 2014 4:44:42 PM EST, Jens Axboe <[email protected]> wrote:
> >On 2014-02-11 13:21, Sander Eikelenboom wrote:
> >>
> >> Tuesday, February 11, 2014, 7:21:56 PM, you wrote:
> >>
> >>> On 11/02/14 18:15, Roger Pau Monn? wrote:
> >>>> On 11/02/14 18:52, David Vrabel wrote:
> >>>>>
> >>>> That would mean that unmap_purged_grants would no longer be static
> >and
> >>>> I should also add a prototype for it in blkback/common.h, which is
> >kind
> >>>> of ugly IMHO.
> >>
> >>> But less ugly than initializing work with a NULL function, IMO.
> >>
> >>>> commit 980e72e45454b64ccb7f23b6794a769384e51038
> >>>> Author: Roger Pau Monne <[email protected]>
> >>>> Date: Tue Feb 11 19:04:06 2014 +0100
> >>>>
> >>>> xen-blkback: init persistent_purge_work work_struct
> >>>>
> >>>> Initialize persistent_purge_work work_struct on
> >xen_blkif_alloc (and
> >>>> remove the previous initialization done in
> >purge_persistent_gnt). This
> >>>> prevents flush_work from complaining even if
> >purge_persistent_gnt has
> >>>> not been used.
> >>>>
> >>>> Signed-off-by: Roger Pau Monn? <[email protected]>
> >>
> >>> Reviewed-by: David Vrabel <[email protected]>
> >>
> >> And a Tested-by: Sander Eikelenboom <[email protected]>
> >
> >Konrad, I was going to ship my current tree today, but looks like we
> >need this too.
>
> Yes. Are you OK taking it from this thread or would you like me to prep a GIT pull with it?

I'll just grab it from here, no problem. Done.

--
Jens Axboe