linux-kernel - Re: [PATCH v3 3/9] xen/blkfront: separate per ring information out of device info

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 10 Oct 2015 16:30:44 +0800
From:	Bob Liu <bob.liu@...cle.com>
To:	Roger Pau Monné <roger.pau@...rix.com>
CC:	xen-devel@...ts.xen.org, david.vrabel@...rix.com,
	linux-kernel@...r.kernel.org, konrad.wilk@...cle.com,
	felipe.franciosi@...rix.com, axboe@...com, hch@...radead.org,
	avanzini.arianna@...il.com, rafal.mielniczuk@...rix.com,
	boris.ostrovsky@...cle.com, jonathan.davies@...rix.com
Subject: Re: [PATCH v3 3/9] xen/blkfront: separate per ring information out
 of device info


On 10/03/2015 01:02 AM, Roger Pau Monné wrote:
> El 05/09/15 a les 14.39, Bob Liu ha escrit:
>> Split per ring information to an new structure:blkfront_ring_info, also rename
>> per blkfront_info to blkfront_dev_info.
>   ^ removed.
>>
>> A ring is the representation of a hardware queue, every vbd device can associate
>> with one or more blkfront_ring_info depending on how many hardware
>> queues/rings to be used.
>>
>> This patch is a preparation for supporting real multi hardware queues/rings.
>>
>> Signed-off-by: Arianna Avanzini <avanzini.arianna@...il.com>
>> Signed-off-by: Bob Liu <bob.liu@...cle.com>
>> ---
>>  drivers/block/xen-blkfront.c |  854 ++++++++++++++++++++++--------------------
>>  1 file changed, 445 insertions(+), 409 deletions(-)
>>
>> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
>> index 5dd591d..bf416d5 100644
>> --- a/drivers/block/xen-blkfront.c
>> +++ b/drivers/block/xen-blkfront.c
>> @@ -107,7 +107,7 @@ static unsigned int xen_blkif_max_ring_order;
>>  module_param_named(max_ring_page_order, xen_blkif_max_ring_order, int, S_IRUGO);
>>  MODULE_PARM_DESC(max_ring_page_order, "Maximum order of pages to be used for the shared ring");
>>  
>> -#define BLK_RING_SIZE(info) __CONST_RING_SIZE(blkif, PAGE_SIZE * (info)->nr_ring_pages)
>> +#define BLK_RING_SIZE(dinfo) __CONST_RING_SIZE(blkif, PAGE_SIZE * (dinfo)->nr_ring_pages)
> 
> This change looks pointless, any reason to use dinfo instead of info?
> 
>>  #define BLK_MAX_RING_SIZE __CONST_RING_SIZE(blkif, PAGE_SIZE * XENBUS_MAX_RING_PAGES)
>>  /*
>>   * ring-ref%i i=(-1UL) would take 11 characters + 'ring-ref' is 8, so 19
>> @@ -116,12 +116,31 @@ MODULE_PARM_DESC(max_ring_page_order, "Maximum order of pages to be used for the
>>  #define RINGREF_NAME_LEN (20)
>>  
>>  /*
>> + *  Per-ring info.
>> + *  Every blkfront device can associate with one or more blkfront_ring_info,
>> + *  depending on how many hardware queues to be used.
>> + */
>> +struct blkfront_ring_info
>> +{
>> +	struct blkif_front_ring ring;
>> +	unsigned int ring_ref[XENBUS_MAX_RING_PAGES];
>> +	unsigned int evtchn, irq;
>> +	struct work_struct work;
>> +	struct gnttab_free_callback callback;
>> +	struct blk_shadow shadow[BLK_MAX_RING_SIZE];
>> +	struct list_head grants;
>> +	struct list_head indirect_pages;
>> +	unsigned int persistent_gnts_c;
> 
> persistent grants should be per-device, not per-queue IMHO. Is it really
> hard to make this global instead of per-queue?
> 

I didn't see the benefit of making it per-device, but disadvantages instead:
If persistent grants are per-device, then we have to introduce an extra lock to protect this list.
Which will complicate the code and may slow down the performance when the queue number is large e.g 16 queues.

Regards,
Bob Liu

>> +	unsigned long shadow_free;
>> +	struct blkfront_dev_info *dinfo;
>> +};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/