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

2015-09-25 Thread Konrad Rzeszutek Wilk 
On Tue, Sep 22, 2015 at 12:10:18AM +0100, Julien Grall wrote:
> Hi Konrad,
> 
> On 09/09/2015 16:02, Konrad Rzeszutek Wilk wrote:
> >>Konrad, would you like me to resend the patch with the modified commit
> >>message, or do you plan to amend it yourself while committing?
> >
> >I will amend it. Thanks!
> 
> What the status for this patch?

Applied and sent the git pull.
> 
> Regards,
> 
> -- 
> Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

2015-09-21 Thread Julien Grall 

Hi Konrad,

On 09/09/2015 16:02, Konrad Rzeszutek Wilk wrote:

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


I will amend it. Thanks!


What the status for this patch?

Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

2015-09-09 Thread Konrad Rzeszutek Wilk 
On Tue, Sep 08, 2015 at 12:43:41PM +0200, Roger Pau Monné wrote:
> 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 
> > Signed-off-by: Roger Pau Monné 
> > Cc: Julien Grall 
> > Cc: Konrad Rzeszutek Wilk 
> > Cc: Boris Ostrovsky 
> > Cc: David Vrabel 
> > Cc: xen-de...@lists.xenproject.org
> 
>  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: GW   4.2.0 #85
> >> Hardware name: APM X-Gene Mustang board (DT)
> >> Workqueue: events xen_blkif_deferred_free
> >> Call trace:
> >> [] dump_backtrace+0x0/0x124
> >> [] show_stack+0x10/0x1c
> >> [] dump_stack+0x78/0x98
> >> [] warn_slowpath_common+0x9c/0xd4
> >> [] warn_slowpath_null+0x14/0x20
> >> [] xen_blkif_deferred_free+0x1f0/0x1f8
> >> [] process_one_work+0x160/0x3b4
> >> [] worker_thread+0x140/0x494
> >> [] 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?

I will amend it. Thanks!
> 
> Roger.
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

2015-09-08 Thread Roger Pau Monné 
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 
> Signed-off-by: Roger Pau Monné 
> Cc: Julien Grall 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Boris Ostrovsky 
> Cc: David Vrabel 
> Cc: xen-de...@lists.xenproject.org

 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: GW   4.2.0 #85
>> Hardware name: APM X-Gene Mustang board (DT)
>> Workqueue: events xen_blkif_deferred_free
>> Call trace:
>> [] dump_backtrace+0x0/0x124
>> [] show_stack+0x10/0x1c
>> [] dump_stack+0x78/0x98
>> [] warn_slowpath_common+0x9c/0xd4
>> [] warn_slowpath_null+0x14/0x20
>> [] xen_blkif_deferred_free+0x1f0/0x1f8
>> [] process_one_work+0x160/0x3b4
>> [] worker_thread+0x140/0x494
>> [] 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.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

2015-09-07 Thread Bob Liu 
Hi Julien,

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 
>> Signed-off-by: Roger Pau Monné 
>> Cc: Julien Grall 
>> Cc: Konrad Rzeszutek Wilk 
>> Cc: Boris Ostrovsky 
>> Cc: David Vrabel 
>> Cc: xen-de...@lists.xenproject.org
> 
> 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.

Thanks,
-Bob

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

2015-09-07 Thread Julien Grall 
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 
>>> Signed-off-by: Roger Pau Monné 
>>> Cc: Julien Grall 
>>> Cc: Konrad Rzeszutek Wilk 
>>> Cc: Boris Ostrovsky 
>>> Cc: David Vrabel 
>>> Cc: xen-de...@lists.xenproject.org
>>
>> 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: GW   4.2.0 #85
Hardware name: APM X-Gene Mustang board (DT)
Workqueue: events xen_blkif_deferred_free
Call trace:
[] dump_backtrace+0x0/0x124
[] show_stack+0x10/0x1c
[] dump_stack+0x78/0x98
[] warn_slowpath_common+0x9c/0xd4
[] warn_slowpath_null+0x14/0x20
[] xen_blkif_deferred_free+0x1f0/0x1f8
[] process_one_work+0x160/0x3b4
[] worker_thread+0x140/0x494
[] 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.

So I think this should be backported in Linux 4.2 where the patch has
been introduced.

Regards,

-- 
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

2015-09-07 Thread Bob Liu 

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 
 Signed-off-by: Roger Pau Monné 
 Cc: Julien Grall 
 Cc: Konrad Rzeszutek Wilk 
 Cc: Boris Ostrovsky 
 Cc: David Vrabel 
 Cc: xen-de...@lists.xenproject.org
>>>
>>> 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: GW   4.2.0 #85
> Hardware name: APM X-Gene Mustang board (DT)
> Workqueue: events xen_blkif_deferred_free
> Call trace:
> [] dump_backtrace+0x0/0x124
> [] show_stack+0x10/0x1c
> [] dump_stack+0x78/0x98
> [] warn_slowpath_common+0x9c/0xd4
> [] warn_slowpath_null+0x14/0x20
> [] xen_blkif_deferred_free+0x1f0/0x1f8
> [] process_one_work+0x160/0x3b4
> [] worker_thread+0x140/0x494
> [] 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.

-- 
Regards,
-Bob

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

2015-09-04 Thread Roger Pau Monne 
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 
Signed-off-by: Roger Pau Monné 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Boris Ostrovsky 
Cc: David Vrabel 
Cc: xen-de...@lists.xenproject.org
---
This patch should be backported to 4.2.
---
 drivers/block/xen-blkback/xenbus.c | 38 --
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c 
b/drivers/block/xen-blkback/xenbus.c
index deb3f00..7676575 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -212,6 +212,9 @@ static int xen_blkif_map(struct xen_blkif *blkif, 
grant_ref_t *gref,
 
 static int xen_blkif_disconnect(struct xen_blkif *blkif)
 {
+   struct pending_req *req, *n;
+   int i = 0, j;
+
if (blkif->xenblkd) {
kthread_stop(blkif->xenblkd);
wake_up(>shutdown_wq);
@@ -238,13 +241,28 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
/* Remove all persistent grants and the cache of ballooned pages. */
xen_blkbk_free_caches(blkif);
 
+   /* Check that there is no request in use */
+   list_for_each_entry_safe(req, n, >pending_free, free_list) {
+   list_del(>free_list);
+
+   for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++)
+   kfree(req->segments[j]);
+
+   for (j = 0; j < MAX_INDIRECT_PAGES; j++)
+   kfree(req->indirect_pages[j]);
+
+   kfree(req);
+   i++;
+   }
+
+   WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
+   blkif->nr_ring_pages = 0;
+
return 0;
 }
 
 static void xen_blkif_free(struct xen_blkif *blkif)
 {
-   struct pending_req *req, *n;
-   int i = 0, j;
 
xen_blkif_disconnect(blkif);
xen_vbd_free(>vbd);
@@ -257,22 +275,6 @@ static void xen_blkif_free(struct xen_blkif *blkif)
BUG_ON(!list_empty(>free_pages));
BUG_ON(!RB_EMPTY_ROOT(>persistent_gnts));
 
-   /* Check that there is no request in use */
-   list_for_each_entry_safe(req, n, >pending_free, free_list) {
-   list_del(>free_list);
-
-   for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++)
-   kfree(req->segments[j]);
-
-   for (j = 0; j < MAX_INDIRECT_PAGES; j++)
-   kfree(req->indirect_pages[j]);
-
-   kfree(req);
-   i++;
-   }
-
-   WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
-
kmem_cache_free(xen_blkif_cachep, blkif);
 }
 
-- 
1.9.5 (Apple Git-50.3)


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

2015-09-04 Thread Julien Grall 
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 
> Signed-off-by: Roger Pau Monné 
> Cc: Julien Grall 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Boris Ostrovsky 
> Cc: David Vrabel 
> Cc: xen-de...@lists.xenproject.org

The patch is fixing my problem when using UEFI in the guest. Thank you!

Tested-by: Julien Grall 

Regards,

-- 
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

2015-09-04 Thread Konrad Rzeszutek Wilk 
On Fri, Sep 04, 2015 at 02:51:10PM +0100, 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 
> > Signed-off-by: Roger Pau Monné 
> > Cc: Julien Grall 
> > Cc: Konrad Rzeszutek Wilk 
> > Cc: Boris Ostrovsky 
> > Cc: David Vrabel 
> > Cc: xen-de...@lists.xenproject.org
> 
> The patch is fixing my problem when using UEFI in the guest. Thank you!
> 
> Tested-by: Julien Grall 

Testing it now for regressions and should be sending an git pull next week
for it.

Thanks!
> 
> Regards,
> 
> -- 
> Julien Grall
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel