Re: [Xen-devel] [PATCH v4 09/10] xen/blkfront: make persistent grants per-queue

[Konrad Rzeszutek Wilk]

On Mon, Nov 02, 2015 at 12:21:45PM +0800, Bob Liu wrote:
> Make persistent grants per-queue/ring instead of per-device, so that we can
> drop the 'dev_lock' and get better scalability.

And what is the performance value for this? How much better
scalability do you get with this?
.. snip..
> @@ -1010,6 +1002,23 @@ static void blkif_free_ring(struct blkfront_ring_info 
> *rinfo)
>   }
>   }
>  
> + /* Remove all persistent grants */

Missing full stop.
> + if (!list_empty(&rinfo->grants)) {
> + list_for_each_entry_safe(persistent_gnt, n,
> +  &rinfo->grants, node) {
> + list_del(&persistent_gnt->node);
> + if (persistent_gnt->gref != GRANT_INVALID_REF) {
> + gnttab_end_foreign_access(persistent_gnt->gref,
> +   0, 0UL);
> + rinfo->persistent_gnts_c--;
> + }
> + if (info->feature_persistent)
> + __free_page(pfn_to_page(persistent_gnt->pfn));
> + kfree(persistent_gnt);
> + }
> + }
> + BUG_ON(rinfo->persistent_gnts_c != 0);
> +

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

[Xen-devel] [PATCH v4 09/10] xen/blkfront: make persistent grants per-queue

[Bob Liu]

Make persistent grants per-queue/ring instead of per-device, so that we can
drop the 'dev_lock' and get better scalability.

Signed-off-by: Bob Liu 
---
 drivers/block/xen-blkfront.c | 89 +---
 1 file changed, 34 insertions(+), 55 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 23096d7..eb19f08 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -133,6 +133,8 @@ struct blkfront_ring_info {
struct gnttab_free_callback callback;
struct blk_shadow shadow[BLK_MAX_RING_SIZE];
struct list_head indirect_pages;
+   struct list_head grants;
+   unsigned int persistent_gnts_c;
unsigned long shadow_free;
struct blkfront_info *dev_info;
 };
@@ -144,8 +146,6 @@ struct blkfront_ring_info {
  */
 struct blkfront_info
 {
-   /* Lock to proect info->grants list shared by multi rings */
-   spinlock_t dev_lock;
struct mutex mutex;
struct xenbus_device *xbdev;
struct gendisk *gd;
@@ -155,8 +155,6 @@ struct blkfront_info
/* Number of pages per ring buffer */
unsigned int nr_ring_pages;
struct request_queue *rq;
-   struct list_head grants;
-   unsigned int persistent_gnts_c;
unsigned int feature_flush;
unsigned int feature_discard:1;
unsigned int feature_secdiscard:1;
@@ -231,7 +229,6 @@ static int fill_grant_buffer(struct blkfront_ring_info 
*rinfo, int num)
struct grant *gnt_list_entry, *n;
int i = 0;
 
-   spin_lock_irq(&info->dev_lock);
while(i < num) {
gnt_list_entry = kzalloc(sizeof(struct grant), GFP_NOIO);
if (!gnt_list_entry)
@@ -247,35 +244,32 @@ static int fill_grant_buffer(struct blkfront_ring_info 
*rinfo, int num)
}
 
gnt_list_entry->gref = GRANT_INVALID_REF;
-   list_add(&gnt_list_entry->node, &info->grants);
+   list_add(&gnt_list_entry->node, &rinfo->grants);
i++;
}
-   spin_unlock_irq(&info->dev_lock);
 
return 0;
 
 out_of_memory:
list_for_each_entry_safe(gnt_list_entry, n,
-&info->grants, node) {
+&rinfo->grants, node) {
list_del(&gnt_list_entry->node);
if (info->feature_persistent)
__free_page(pfn_to_page(gnt_list_entry->pfn));
kfree(gnt_list_entry);
i--;
}
-   spin_unlock_irq(&info->dev_lock);
BUG_ON(i != 0);
return -ENOMEM;
 }
 
 static struct grant *get_grant(grant_ref_t *gref_head,
unsigned long pfn,
-   struct blkfront_info *info)
+  struct blkfront_ring_info *info)
 {
struct grant *gnt_list_entry;
unsigned long buffer_gfn;
 
-   spin_lock(&info->dev_lock);
BUG_ON(list_empty(&info->grants));
gnt_list_entry = list_first_entry(&info->grants, struct grant,
  node);
@@ -283,21 +277,19 @@ static struct grant *get_grant(grant_ref_t *gref_head,
 
if (gnt_list_entry->gref != GRANT_INVALID_REF) {
info->persistent_gnts_c--;
-   spin_unlock(&info->dev_lock);
return gnt_list_entry;
}
-   spin_unlock(&info->dev_lock);
 
/* Assign a gref to this page */
gnt_list_entry->gref = gnttab_claim_grant_reference(gref_head);
BUG_ON(gnt_list_entry->gref == -ENOSPC);
-   if (!info->feature_persistent) {
+   if (!info->dev_info->feature_persistent) {
BUG_ON(!pfn);
gnt_list_entry->pfn = pfn;
}
buffer_gfn = pfn_to_gfn(gnt_list_entry->pfn);
gnttab_grant_foreign_access_ref(gnt_list_entry->gref,
-   info->xbdev->otherend_id,
+   info->dev_info->xbdev->otherend_id,
buffer_gfn, 0);
return gnt_list_entry;
 }
@@ -559,13 +551,13 @@ static int blkif_queue_request(struct request *req, 
struct blkfront_ring_info *r
list_del(&indirect_page->lru);
pfn = page_to_pfn(indirect_page);
}
-   gnt_list_entry = get_grant(&gref_head, pfn, 
info);
+   gnt_list_entry = get_grant(&gref_head, pfn, 
rinfo);
rinfo->shadow[id].indirect_grants[n] = 
gnt_list_entry;
segments = 
kmap_atomic(pfn_to_page(gnt_list_entry->pfn));
ring_req->u.indirect.indirect_grefs[n] = 
gnt_list_entry->gref;
}
 
-   gnt_list_entry = get_grant(&gref_head, 
page_to_pfn(sg_pa