Re: [Xen-devel] [PATCH] xen-blkback: free requests on disconnection

From: Roger Pau MonnÃ
Date: Tue Sep 08 2015 - 06:43:51 EST


El 07/09/15 a les 13.35, Bob Liu ha escrit:
>
> On 09/07/2015 07:10 PM, Julien Grall wrote:
>> On 07/09/15 07:07, Bob Liu wrote:
>>> Hi Julien,
>>
>> Hi Bob,
>>
>>> On 09/04/2015 09:51 PM, Julien Grall wrote:
>>>> Hi Roger,
>>>>
>>>> On 04/09/15 11:08, Roger Pau Monne wrote:
>>>>> Request allocation has been moved to connect_ring, which is called every
>>>>> time blkback connects to the frontend (this can happen multiple times during
>>>>> a blkback instance life cycle). On the other hand, request freeing has not
>>>>> been moved, so it's only called when destroying the backend instance. Due to
>>>>> this mismatch, blkback can allocate the request pool multiple times, without
>>>>> freeing it.
>>>>>
>>>>> In order to fix it, move the freeing of requests to xen_blkif_disconnect to
>>>>> restore the symmetry between request allocation and freeing.
>>>>>
>>>>> Reported-by: Julien Grall <julien.grall@xxxxxxxxxx>
>>>>> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
>>>>> Cc: Julien Grall <julien.grall@xxxxxxxxxx>
>>>>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>>>>> Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>>>>> Cc: David Vrabel <david.vrabel@xxxxxxxxxx>
>>>>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
>>>>
>>>> The patch is fixing my problem when using UEFI in the guest. Thank you!
>>>>
>>>
>>> Could you please explain the problem you met a bit more?
>>> So that I can know back port this patch if met similar issue.
>>
>> This is related to commit 86839c56dee28c315a4c19b7bfee450ccd84cd25
>> "xen/block: add multi-page ring support" (Roger, it may be worth to
>> indicate the offending commit in you commit message).
>>
>> When starting a guest using UEFI. After the domain is destroyed I get
>> the following warning from blkback:
>>
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 2 PID: 95 at
>> /home/julien/works/linux/drivers/block/xen-blkback/xenbus.c:274
>> xen_blkif_deferred_free+0x1f4/0x1f8()
>> Modules linked in:
>> CPU: 2 PID: 95 Comm: kworker/2:1 Tainted: G W 4.2.0 #85
>> Hardware name: APM X-Gene Mustang board (DT)
>> Workqueue: events xen_blkif_deferred_free
>> Call trace:
>> [<ffff8000000890a8>] dump_backtrace+0x0/0x124
>> [<ffff8000000891dc>] show_stack+0x10/0x1c
>> [<ffff8000007653bc>] dump_stack+0x78/0x98
>> [<ffff800000097e88>] warn_slowpath_common+0x9c/0xd4
>> [<ffff800000097f80>] warn_slowpath_null+0x14/0x20
>> [<ffff800000557a0c>] xen_blkif_deferred_free+0x1f0/0x1f8
>> [<ffff8000000ad020>] process_one_work+0x160/0x3b4
>> [<ffff8000000ad3b4>] worker_thread+0x140/0x494
>> [<ffff8000000b2e34>] kthread+0xd8/0xf0
>> ---[ end trace 6f859b7883c88cdd ]---
>>
>> This is because the allocation of the requests are done during the
>> connection but the free is done when the domain is destroyed. Therefore
>> if the domain is re-initializing the connection (because UEFI or PV Grub
>> is used), the request won't be free and kept until the end.
>>
>
> Thank you!
> Roger, I think it's better to have this information in your commit message too.

Konrad, would you like me to resend the patch with the modified commit
message, or do you plan to amend it yourself while committing?

Roger.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/