* Re: [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-26 12:16 ` Bob Liu
@ 2015-03-26 15:32 ` Roger Pau Monné
2015-03-28 0:44 ` Konrad Rzeszutek Wilk
2015-03-28 0:44 ` Konrad Rzeszutek Wilk
2015-03-26 15:32 ` Roger Pau Monné
` (4 subsequent siblings)
5 siblings, 2 replies; 24+ messages in thread
From: Roger Pau Monné @ 2015-03-26 15:32 UTC (permalink / raw)
To: Bob Liu, xen-devel
Cc: boris.ostrovsky, jennifer.herbert, david.vrabel, linux-kernel,
konrad.wilk
El 26/03/15 a les 13.16, Bob Liu ha escrit:
> There are several place using gnttab async unmap and wait for
> completion, so move the common code to a function
> gnttab_unmap_refs_async_wait_completion().
>
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
For the blkback parts:
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Although, as David already said, I think we should do better than BUG_ON
on error.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-26 15:32 ` Roger Pau Monné
@ 2015-03-28 0:44 ` Konrad Rzeszutek Wilk
2015-03-28 0:44 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 24+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-28 0:44 UTC (permalink / raw)
To: Roger Pau Monné
Cc: jennifer.herbert, linux-kernel, david.vrabel, xen-devel, boris.ostrovsky
On Thu, Mar 26, 2015 at 04:32:45PM +0100, Roger Pau Monné wrote:
> El 26/03/15 a les 13.16, Bob Liu ha escrit:
> > There are several place using gnttab async unmap and wait for
> > completion, so move the common code to a function
> > gnttab_unmap_refs_async_wait_completion().
> >
> > Signed-off-by: Bob Liu <bob.liu@oracle.com>
>
> For the blkback parts:
>
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
>
> Although, as David already said, I think we should do better than BUG_ON
> on error.
Have an 'stuck deferred pages' list and an timer to occasionally
flush them out? Something similar to 569ca5b3f94cd0b3295ec5943aa457cf4a4f6a3a
"xen/gnttab: add deferred freeing logic" ?
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-26 15:32 ` Roger Pau Monné
2015-03-28 0:44 ` Konrad Rzeszutek Wilk
@ 2015-03-28 0:44 ` Konrad Rzeszutek Wilk
2015-03-30 0:03 ` Bob Liu
2015-03-30 0:03 ` Bob Liu
1 sibling, 2 replies; 24+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-28 0:44 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Bob Liu, xen-devel, boris.ostrovsky, jennifer.herbert,
david.vrabel, linux-kernel
On Thu, Mar 26, 2015 at 04:32:45PM +0100, Roger Pau Monné wrote:
> El 26/03/15 a les 13.16, Bob Liu ha escrit:
> > There are several place using gnttab async unmap and wait for
> > completion, so move the common code to a function
> > gnttab_unmap_refs_async_wait_completion().
> >
> > Signed-off-by: Bob Liu <bob.liu@oracle.com>
>
> For the blkback parts:
>
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
>
> Although, as David already said, I think we should do better than BUG_ON
> on error.
Have an 'stuck deferred pages' list and an timer to occasionally
flush them out? Something similar to 569ca5b3f94cd0b3295ec5943aa457cf4a4f6a3a
"xen/gnttab: add deferred freeing logic" ?
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-28 0:44 ` Konrad Rzeszutek Wilk
@ 2015-03-30 0:03 ` Bob Liu
2015-03-30 0:03 ` Bob Liu
1 sibling, 0 replies; 24+ messages in thread
From: Bob Liu @ 2015-03-30 0:03 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: jennifer.herbert, linux-kernel, david.vrabel, xen-devel,
boris.ostrovsky, Roger Pau Monné
On 03/28/2015 08:44 AM, Konrad Rzeszutek Wilk wrote:
> On Thu, Mar 26, 2015 at 04:32:45PM +0100, Roger Pau Monné wrote:
>> El 26/03/15 a les 13.16, Bob Liu ha escrit:
>>> There are several place using gnttab async unmap and wait for
>>> completion, so move the common code to a function
>>> gnttab_unmap_refs_async_wait_completion().
>>>
>>> Signed-off-by: Bob Liu <bob.liu@oracle.com>
>>
>> For the blkback parts:
>>
>> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
>>
>> Although, as David already said, I think we should do better than BUG_ON
>> on error.
>
This patch only keeps the existing behaviour the same as before. I
prefer to make new patches(based on this one) if want to change the
behaviour.
> Have an 'stuck deferred pages' list and an timer to occasionally
> flush them out? Something similar to 569ca5b3f94cd0b3295ec5943aa457cf4a4f6a3a
> "xen/gnttab: add deferred freeing logic" ?
Sounds like a good idea.
--
Regards,
-Bob
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-28 0:44 ` Konrad Rzeszutek Wilk
2015-03-30 0:03 ` Bob Liu
@ 2015-03-30 0:03 ` Bob Liu
2015-04-01 10:05 ` David Vrabel
2015-04-01 10:05 ` [Xen-devel] " David Vrabel
1 sibling, 2 replies; 24+ messages in thread
From: Bob Liu @ 2015-03-30 0:03 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Roger Pau Monné,
xen-devel, boris.ostrovsky, jennifer.herbert, david.vrabel,
linux-kernel
On 03/28/2015 08:44 AM, Konrad Rzeszutek Wilk wrote:
> On Thu, Mar 26, 2015 at 04:32:45PM +0100, Roger Pau Monné wrote:
>> El 26/03/15 a les 13.16, Bob Liu ha escrit:
>>> There are several place using gnttab async unmap and wait for
>>> completion, so move the common code to a function
>>> gnttab_unmap_refs_async_wait_completion().
>>>
>>> Signed-off-by: Bob Liu <bob.liu@oracle.com>
>>
>> For the blkback parts:
>>
>> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
>>
>> Although, as David already said, I think we should do better than BUG_ON
>> on error.
>
This patch only keeps the existing behaviour the same as before. I
prefer to make new patches(based on this one) if want to change the
behaviour.
> Have an 'stuck deferred pages' list and an timer to occasionally
> flush them out? Something similar to 569ca5b3f94cd0b3295ec5943aa457cf4a4f6a3a
> "xen/gnttab: add deferred freeing logic" ?
Sounds like a good idea.
--
Regards,
-Bob
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-30 0:03 ` Bob Liu
@ 2015-04-01 10:05 ` David Vrabel
2015-04-01 10:05 ` [Xen-devel] " David Vrabel
1 sibling, 0 replies; 24+ messages in thread
From: David Vrabel @ 2015-04-01 10:05 UTC (permalink / raw)
To: Bob Liu, Konrad Rzeszutek Wilk
Cc: jennifer.herbert, linux-kernel, david.vrabel, xen-devel,
boris.ostrovsky, Roger Pau Monné
On 30/03/15 01:03, Bob Liu wrote:
>
> On 03/28/2015 08:44 AM, Konrad Rzeszutek Wilk wrote:
>> On Thu, Mar 26, 2015 at 04:32:45PM +0100, Roger Pau Monné wrote:
>>> El 26/03/15 a les 13.16, Bob Liu ha escrit:
>>>> There are several place using gnttab async unmap and wait for
>>>> completion, so move the common code to a function
>>>> gnttab_unmap_refs_async_wait_completion().
>>>>
>>>> Signed-off-by: Bob Liu <bob.liu@oracle.com>
>>>
>>> For the blkback parts:
>>>
>>> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
>>>
>>> Although, as David already said, I think we should do better than BUG_ON
>>> on error.
>>
>
> This patch only keeps the existing behaviour the same as before. I
> prefer to make new patches(based on this one) if want to change the
> behaviour.
>
>> Have an 'stuck deferred pages' list and an timer to occasionally
>> flush them out? Something similar to 569ca5b3f94cd0b3295ec5943aa457cf4a4f6a3a
>> "xen/gnttab: add deferred freeing logic" ?
>
> Sounds like a good idea.
If it failed the first time I don't see how it would succeed later. No
one has actually hit this particular BUG_ON() so I would leave it as is.
David
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-30 0:03 ` Bob Liu
2015-04-01 10:05 ` David Vrabel
@ 2015-04-01 10:05 ` David Vrabel
1 sibling, 0 replies; 24+ messages in thread
From: David Vrabel @ 2015-04-01 10:05 UTC (permalink / raw)
To: Bob Liu, Konrad Rzeszutek Wilk
Cc: jennifer.herbert, linux-kernel, david.vrabel, xen-devel,
boris.ostrovsky, Roger Pau Monné
On 30/03/15 01:03, Bob Liu wrote:
>
> On 03/28/2015 08:44 AM, Konrad Rzeszutek Wilk wrote:
>> On Thu, Mar 26, 2015 at 04:32:45PM +0100, Roger Pau Monné wrote:
>>> El 26/03/15 a les 13.16, Bob Liu ha escrit:
>>>> There are several place using gnttab async unmap and wait for
>>>> completion, so move the common code to a function
>>>> gnttab_unmap_refs_async_wait_completion().
>>>>
>>>> Signed-off-by: Bob Liu <bob.liu@oracle.com>
>>>
>>> For the blkback parts:
>>>
>>> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
>>>
>>> Although, as David already said, I think we should do better than BUG_ON
>>> on error.
>>
>
> This patch only keeps the existing behaviour the same as before. I
> prefer to make new patches(based on this one) if want to change the
> behaviour.
>
>> Have an 'stuck deferred pages' list and an timer to occasionally
>> flush them out? Something similar to 569ca5b3f94cd0b3295ec5943aa457cf4a4f6a3a
>> "xen/gnttab: add deferred freeing logic" ?
>
> Sounds like a good idea.
If it failed the first time I don't see how it would succeed later. No
one has actually hit this particular BUG_ON() so I would leave it as is.
David
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-26 12:16 ` Bob Liu
2015-03-26 15:32 ` Roger Pau Monné
@ 2015-03-26 15:32 ` Roger Pau Monné
2015-03-26 19:01 ` Konrad Rzeszutek Wilk
` (3 subsequent siblings)
5 siblings, 0 replies; 24+ messages in thread
From: Roger Pau Monné @ 2015-03-26 15:32 UTC (permalink / raw)
To: Bob Liu, xen-devel
Cc: boris.ostrovsky, jennifer.herbert, david.vrabel, linux-kernel
El 26/03/15 a les 13.16, Bob Liu ha escrit:
> There are several place using gnttab async unmap and wait for
> completion, so move the common code to a function
> gnttab_unmap_refs_async_wait_completion().
>
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
For the blkback parts:
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Although, as David already said, I think we should do better than BUG_ON
on error.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-26 12:16 ` Bob Liu
2015-03-26 15:32 ` Roger Pau Monné
2015-03-26 15:32 ` Roger Pau Monné
@ 2015-03-26 19:01 ` Konrad Rzeszutek Wilk
2015-03-26 19:01 ` Konrad Rzeszutek Wilk
` (2 subsequent siblings)
5 siblings, 0 replies; 24+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-26 19:01 UTC (permalink / raw)
To: Bob Liu, axboe
Cc: xen-devel, boris.ostrovsky, jennifer.herbert, david.vrabel,
roger.pau, linux-kernel
On Thu, Mar 26, 2015 at 08:16:01PM +0800, Bob Liu wrote:
> There are several place using gnttab async unmap and wait for
> completion, so move the common code to a function
> gnttab_unmap_refs_async_wait_completion().
>
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
Jens, and would you be OK if this patch went through the Xen tree?
Thanks.
> ---
> drivers/block/xen-blkback/blkback.c | 31 +++----------------------------
> drivers/xen/gntdev.c | 28 +++-------------------------
> drivers/xen/grant-table.c | 29 +++++++++++++++++++++++++++++
> include/xen/grant_table.h | 1 +
> 4 files changed, 36 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> index f59d7c3..0c8da82 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -263,17 +263,6 @@ static void put_persistent_gnt(struct xen_blkif *blkif,
> atomic_dec(&blkif->persistent_gnt_in_use);
> }
>
> -static void free_persistent_gnts_unmap_callback(int result,
> - struct gntab_unmap_queue_data *data)
> -{
> - struct completion *c = data->data;
> -
> - /* BUG_ON used to reproduce existing behaviour,
> - but is this the best way to deal with this? */
> - BUG_ON(result);
> - complete(c);
> -}
> -
> static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
> unsigned int num)
> {
> @@ -283,12 +272,7 @@ static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
> struct rb_node *n;
> int segs_to_unmap = 0;
> struct gntab_unmap_queue_data unmap_data;
> - struct completion unmap_completion;
> -
> - init_completion(&unmap_completion);
>
> - unmap_data.data = &unmap_completion;
> - unmap_data.done = &free_persistent_gnts_unmap_callback;
> unmap_data.pages = pages;
> unmap_data.unmap_ops = unmap;
> unmap_data.kunmap_ops = NULL;
> @@ -308,8 +292,7 @@ static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
> !rb_next(&persistent_gnt->node)) {
>
> unmap_data.count = segs_to_unmap;
> - gnttab_unmap_refs_async(&unmap_data);
> - wait_for_completion(&unmap_completion);
> + BUG_ON(gnttab_unmap_refs_async_wait_completion(&unmap_data));
>
> put_free_pages(blkif, pages, segs_to_unmap);
> segs_to_unmap = 0;
> @@ -330,12 +313,7 @@ void xen_blkbk_unmap_purged_grants(struct work_struct *work)
> int segs_to_unmap = 0;
> struct xen_blkif *blkif = container_of(work, typeof(*blkif), persistent_purge_work);
> struct gntab_unmap_queue_data unmap_data;
> - struct completion unmap_completion;
>
> - init_completion(&unmap_completion);
> -
> - unmap_data.data = &unmap_completion;
> - unmap_data.done = &free_persistent_gnts_unmap_callback;
> unmap_data.pages = pages;
> unmap_data.unmap_ops = unmap;
> unmap_data.kunmap_ops = NULL;
> @@ -355,9 +333,7 @@ void xen_blkbk_unmap_purged_grants(struct work_struct *work)
>
> if (++segs_to_unmap == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
> unmap_data.count = segs_to_unmap;
> - gnttab_unmap_refs_async(&unmap_data);
> - wait_for_completion(&unmap_completion);
> -
> + BUG_ON(gnttab_unmap_refs_async_wait_completion(&unmap_data));
> put_free_pages(blkif, pages, segs_to_unmap);
> segs_to_unmap = 0;
> }
> @@ -365,8 +341,7 @@ void xen_blkbk_unmap_purged_grants(struct work_struct *work)
> }
> if (segs_to_unmap > 0) {
> unmap_data.count = segs_to_unmap;
> - gnttab_unmap_refs_async(&unmap_data);
> - wait_for_completion(&unmap_completion);
> + BUG_ON(gnttab_unmap_refs_async_wait_completion(&unmap_data));
> put_free_pages(blkif, pages, segs_to_unmap);
> }
> }
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index d5bb1a3..7a88524 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -327,30 +327,10 @@ static int map_grant_pages(struct grant_map *map)
> return err;
> }
>
> -struct unmap_grant_pages_callback_data
> -{
> - struct completion completion;
> - int result;
> -};
> -
> -static void unmap_grant_callback(int result,
> - struct gntab_unmap_queue_data *data)
> -{
> - struct unmap_grant_pages_callback_data* d = data->data;
> -
> - d->result = result;
> - complete(&d->completion);
> -}
> -
> static int __unmap_grant_pages(struct grant_map *map, int offset, int pages)
> {
> int i, err = 0;
> struct gntab_unmap_queue_data unmap_data;
> - struct unmap_grant_pages_callback_data data;
> -
> - init_completion(&data.completion);
> - unmap_data.data = &data;
> - unmap_data.done= &unmap_grant_callback;
>
> if (map->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) {
> int pgno = (map->notify.addr >> PAGE_SHIFT);
> @@ -367,11 +347,9 @@ static int __unmap_grant_pages(struct grant_map *map, int offset, int pages)
> unmap_data.pages = map->pages + offset;
> unmap_data.count = pages;
>
> - gnttab_unmap_refs_async(&unmap_data);
> -
> - wait_for_completion(&data.completion);
> - if (data.result)
> - return data.result;
> + err = gnttab_unmap_refs_async_wait_completion(&unmap_data);
> + if (err)
> + return err;
>
> for (i = 0; i < pages; i++) {
> if (map->unmap_ops[offset+i].status)
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 17972fb..1dce9bc 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -123,6 +123,12 @@ struct gnttab_ops {
> int (*query_foreign_access)(grant_ref_t ref);
> };
>
> +struct unmap_refs_async_callback_data
> +{
> + struct completion completion;
> + int result;
> +};
> +
> static struct gnttab_ops *gnttab_interface;
>
> static int grant_table_version;
> @@ -863,6 +869,29 @@ void gnttab_unmap_refs_async(struct gntab_unmap_queue_data* item)
> }
> EXPORT_SYMBOL_GPL(gnttab_unmap_refs_async);
>
> +static void unmap_refs_async_callback(int result,
> + struct gntab_unmap_queue_data *data)
> +{
> + struct unmap_refs_async_callback_data* d = data->data;
> +
> + d->result = result;
> + complete(&d->completion);
> +}
> +
> +int gnttab_unmap_refs_async_wait_completion(struct gntab_unmap_queue_data* item)
> +{
> + struct unmap_refs_async_callback_data data;
> +
> + init_completion(&data.completion);
> + item->data = &data;
> + item->done= &unmap_refs_async_callback;
> + gnttab_unmap_refs_async(item);
> + wait_for_completion(&data.completion);
> +
> + return data.result;
> +}
> +EXPORT_SYMBOL_GPL(gnttab_unmap_refs_async_wait_completion);
> +
> static int gnttab_map_frames_v1(xen_pfn_t *frames, unsigned int nr_gframes)
> {
> int rc;
> diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
> index 143ca5f..46bad05 100644
> --- a/include/xen/grant_table.h
> +++ b/include/xen/grant_table.h
> @@ -191,6 +191,7 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
> struct gnttab_unmap_grant_ref *kunmap_ops,
> struct page **pages, unsigned int count);
> void gnttab_unmap_refs_async(struct gntab_unmap_queue_data* item);
> +int gnttab_unmap_refs_async_wait_completion(struct gntab_unmap_queue_data* item);
>
>
> /* Perform a batch of grant map/copy operations. Retry every batch slot
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-26 12:16 ` Bob Liu
` (2 preceding siblings ...)
2015-03-26 19:01 ` Konrad Rzeszutek Wilk
@ 2015-03-26 19:01 ` Konrad Rzeszutek Wilk
2015-04-01 10:18 ` David Vrabel
2015-04-01 10:18 ` [Xen-devel] " David Vrabel
5 siblings, 0 replies; 24+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-26 19:01 UTC (permalink / raw)
To: Bob Liu, axboe
Cc: jennifer.herbert, linux-kernel, david.vrabel, xen-devel,
boris.ostrovsky, roger.pau
On Thu, Mar 26, 2015 at 08:16:01PM +0800, Bob Liu wrote:
> There are several place using gnttab async unmap and wait for
> completion, so move the common code to a function
> gnttab_unmap_refs_async_wait_completion().
>
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
Jens, and would you be OK if this patch went through the Xen tree?
Thanks.
> ---
> drivers/block/xen-blkback/blkback.c | 31 +++----------------------------
> drivers/xen/gntdev.c | 28 +++-------------------------
> drivers/xen/grant-table.c | 29 +++++++++++++++++++++++++++++
> include/xen/grant_table.h | 1 +
> 4 files changed, 36 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> index f59d7c3..0c8da82 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -263,17 +263,6 @@ static void put_persistent_gnt(struct xen_blkif *blkif,
> atomic_dec(&blkif->persistent_gnt_in_use);
> }
>
> -static void free_persistent_gnts_unmap_callback(int result,
> - struct gntab_unmap_queue_data *data)
> -{
> - struct completion *c = data->data;
> -
> - /* BUG_ON used to reproduce existing behaviour,
> - but is this the best way to deal with this? */
> - BUG_ON(result);
> - complete(c);
> -}
> -
> static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
> unsigned int num)
> {
> @@ -283,12 +272,7 @@ static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
> struct rb_node *n;
> int segs_to_unmap = 0;
> struct gntab_unmap_queue_data unmap_data;
> - struct completion unmap_completion;
> -
> - init_completion(&unmap_completion);
>
> - unmap_data.data = &unmap_completion;
> - unmap_data.done = &free_persistent_gnts_unmap_callback;
> unmap_data.pages = pages;
> unmap_data.unmap_ops = unmap;
> unmap_data.kunmap_ops = NULL;
> @@ -308,8 +292,7 @@ static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
> !rb_next(&persistent_gnt->node)) {
>
> unmap_data.count = segs_to_unmap;
> - gnttab_unmap_refs_async(&unmap_data);
> - wait_for_completion(&unmap_completion);
> + BUG_ON(gnttab_unmap_refs_async_wait_completion(&unmap_data));
>
> put_free_pages(blkif, pages, segs_to_unmap);
> segs_to_unmap = 0;
> @@ -330,12 +313,7 @@ void xen_blkbk_unmap_purged_grants(struct work_struct *work)
> int segs_to_unmap = 0;
> struct xen_blkif *blkif = container_of(work, typeof(*blkif), persistent_purge_work);
> struct gntab_unmap_queue_data unmap_data;
> - struct completion unmap_completion;
>
> - init_completion(&unmap_completion);
> -
> - unmap_data.data = &unmap_completion;
> - unmap_data.done = &free_persistent_gnts_unmap_callback;
> unmap_data.pages = pages;
> unmap_data.unmap_ops = unmap;
> unmap_data.kunmap_ops = NULL;
> @@ -355,9 +333,7 @@ void xen_blkbk_unmap_purged_grants(struct work_struct *work)
>
> if (++segs_to_unmap == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
> unmap_data.count = segs_to_unmap;
> - gnttab_unmap_refs_async(&unmap_data);
> - wait_for_completion(&unmap_completion);
> -
> + BUG_ON(gnttab_unmap_refs_async_wait_completion(&unmap_data));
> put_free_pages(blkif, pages, segs_to_unmap);
> segs_to_unmap = 0;
> }
> @@ -365,8 +341,7 @@ void xen_blkbk_unmap_purged_grants(struct work_struct *work)
> }
> if (segs_to_unmap > 0) {
> unmap_data.count = segs_to_unmap;
> - gnttab_unmap_refs_async(&unmap_data);
> - wait_for_completion(&unmap_completion);
> + BUG_ON(gnttab_unmap_refs_async_wait_completion(&unmap_data));
> put_free_pages(blkif, pages, segs_to_unmap);
> }
> }
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index d5bb1a3..7a88524 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -327,30 +327,10 @@ static int map_grant_pages(struct grant_map *map)
> return err;
> }
>
> -struct unmap_grant_pages_callback_data
> -{
> - struct completion completion;
> - int result;
> -};
> -
> -static void unmap_grant_callback(int result,
> - struct gntab_unmap_queue_data *data)
> -{
> - struct unmap_grant_pages_callback_data* d = data->data;
> -
> - d->result = result;
> - complete(&d->completion);
> -}
> -
> static int __unmap_grant_pages(struct grant_map *map, int offset, int pages)
> {
> int i, err = 0;
> struct gntab_unmap_queue_data unmap_data;
> - struct unmap_grant_pages_callback_data data;
> -
> - init_completion(&data.completion);
> - unmap_data.data = &data;
> - unmap_data.done= &unmap_grant_callback;
>
> if (map->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) {
> int pgno = (map->notify.addr >> PAGE_SHIFT);
> @@ -367,11 +347,9 @@ static int __unmap_grant_pages(struct grant_map *map, int offset, int pages)
> unmap_data.pages = map->pages + offset;
> unmap_data.count = pages;
>
> - gnttab_unmap_refs_async(&unmap_data);
> -
> - wait_for_completion(&data.completion);
> - if (data.result)
> - return data.result;
> + err = gnttab_unmap_refs_async_wait_completion(&unmap_data);
> + if (err)
> + return err;
>
> for (i = 0; i < pages; i++) {
> if (map->unmap_ops[offset+i].status)
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 17972fb..1dce9bc 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -123,6 +123,12 @@ struct gnttab_ops {
> int (*query_foreign_access)(grant_ref_t ref);
> };
>
> +struct unmap_refs_async_callback_data
> +{
> + struct completion completion;
> + int result;
> +};
> +
> static struct gnttab_ops *gnttab_interface;
>
> static int grant_table_version;
> @@ -863,6 +869,29 @@ void gnttab_unmap_refs_async(struct gntab_unmap_queue_data* item)
> }
> EXPORT_SYMBOL_GPL(gnttab_unmap_refs_async);
>
> +static void unmap_refs_async_callback(int result,
> + struct gntab_unmap_queue_data *data)
> +{
> + struct unmap_refs_async_callback_data* d = data->data;
> +
> + d->result = result;
> + complete(&d->completion);
> +}
> +
> +int gnttab_unmap_refs_async_wait_completion(struct gntab_unmap_queue_data* item)
> +{
> + struct unmap_refs_async_callback_data data;
> +
> + init_completion(&data.completion);
> + item->data = &data;
> + item->done= &unmap_refs_async_callback;
> + gnttab_unmap_refs_async(item);
> + wait_for_completion(&data.completion);
> +
> + return data.result;
> +}
> +EXPORT_SYMBOL_GPL(gnttab_unmap_refs_async_wait_completion);
> +
> static int gnttab_map_frames_v1(xen_pfn_t *frames, unsigned int nr_gframes)
> {
> int rc;
> diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
> index 143ca5f..46bad05 100644
> --- a/include/xen/grant_table.h
> +++ b/include/xen/grant_table.h
> @@ -191,6 +191,7 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
> struct gnttab_unmap_grant_ref *kunmap_ops,
> struct page **pages, unsigned int count);
> void gnttab_unmap_refs_async(struct gntab_unmap_queue_data* item);
> +int gnttab_unmap_refs_async_wait_completion(struct gntab_unmap_queue_data* item);
>
>
> /* Perform a batch of grant map/copy operations. Retry every batch slot
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-26 12:16 ` Bob Liu
` (3 preceding siblings ...)
2015-03-26 19:01 ` Konrad Rzeszutek Wilk
@ 2015-04-01 10:18 ` David Vrabel
2015-04-01 10:18 ` [Xen-devel] " David Vrabel
5 siblings, 0 replies; 24+ messages in thread
From: David Vrabel @ 2015-04-01 10:18 UTC (permalink / raw)
To: Bob Liu, xen-devel
Cc: boris.ostrovsky, roger.pau, jennifer.herbert, linux-kernel, david.vrabel
On 26/03/15 12:16, Bob Liu wrote:
> There are several place using gnttab async unmap and wait for
> completion, so move the common code to a function
> gnttab_unmap_refs_async_wait_completion().
>
[...]
> +
> +int gnttab_unmap_refs_async_wait_completion(struct gntab_unmap_queue_data* item)
This name is a bit wordy. Can you rename it to:
gnttab_unmap_refs_sync() gnttab_unmap_refs_wait(), or
gnttab_unmap_refs_safe().
(I can't decide which is the best name.).
David
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-03-26 12:16 ` Bob Liu
` (4 preceding siblings ...)
2015-04-01 10:18 ` David Vrabel
@ 2015-04-01 10:18 ` David Vrabel
2015-04-01 11:55 ` Bob Liu
2015-04-01 11:55 ` [Xen-devel] " Bob Liu
5 siblings, 2 replies; 24+ messages in thread
From: David Vrabel @ 2015-04-01 10:18 UTC (permalink / raw)
To: Bob Liu, xen-devel
Cc: jennifer.herbert, linux-kernel, david.vrabel, boris.ostrovsky, roger.pau
On 26/03/15 12:16, Bob Liu wrote:
> There are several place using gnttab async unmap and wait for
> completion, so move the common code to a function
> gnttab_unmap_refs_async_wait_completion().
>
[...]
> +
> +int gnttab_unmap_refs_async_wait_completion(struct gntab_unmap_queue_data* item)
This name is a bit wordy. Can you rename it to:
gnttab_unmap_refs_sync() gnttab_unmap_refs_wait(), or
gnttab_unmap_refs_safe().
(I can't decide which is the best name.).
David
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-04-01 10:18 ` [Xen-devel] " David Vrabel
@ 2015-04-01 11:55 ` Bob Liu
2015-04-01 11:55 ` [Xen-devel] " Bob Liu
1 sibling, 0 replies; 24+ messages in thread
From: Bob Liu @ 2015-04-01 11:55 UTC (permalink / raw)
To: David Vrabel
Cc: jennifer.herbert, LKML, xen-devel, boris.ostrovsky, Roger Pau Monné
On 04/01/2015 06:18 PM, David Vrabel wrote:
> On 26/03/15 12:16, Bob Liu wrote:
>> There are several place using gnttab async unmap and wait for
>> completion, so move the common code to a function
>> gnttab_unmap_refs_async_wait_completion().
>>
> [...]
>> +
>> +int gnttab_unmap_refs_async_wait_completion(struct gntab_unmap_queue_data* item)
>
> This name is a bit wordy. Can you rename it to:
> gnttab_unmap_refs_sync() gnttab_unmap_refs_wait(), or
Sure, I think gnttab_unmap_refs_sync() is fine.
Will be updated.
> gnttab_unmap_refs_safe().
>
> (I can't decide which is the best name.).
>
> David
>
--
Regards,
-Bob
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH 2/2] xen/grant: introduce func gnttab_unmap_refs_async_wait_completion()
2015-04-01 10:18 ` [Xen-devel] " David Vrabel
2015-04-01 11:55 ` Bob Liu
@ 2015-04-01 11:55 ` Bob Liu
1 sibling, 0 replies; 24+ messages in thread
From: Bob Liu @ 2015-04-01 11:55 UTC (permalink / raw)
To: David Vrabel
Cc: xen-devel, jennifer.herbert, LKML, boris.ostrovsky, konrad.wilk,
Roger Pau Monné
On 04/01/2015 06:18 PM, David Vrabel wrote:
> On 26/03/15 12:16, Bob Liu wrote:
>> There are several place using gnttab async unmap and wait for
>> completion, so move the common code to a function
>> gnttab_unmap_refs_async_wait_completion().
>>
> [...]
>> +
>> +int gnttab_unmap_refs_async_wait_completion(struct gntab_unmap_queue_data* item)
>
> This name is a bit wordy. Can you rename it to:
> gnttab_unmap_refs_sync() gnttab_unmap_refs_wait(), or
Sure, I think gnttab_unmap_refs_sync() is fine.
Will be updated.
> gnttab_unmap_refs_safe().
>
> (I can't decide which is the best name.).
>
> David
>
--
Regards,
-Bob
^ permalink raw reply [flat|nested] 24+ messages in thread