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
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
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!
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/
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/
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);
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
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);
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
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
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
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!
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