linux-kernel - Re: [PATCH v3 8/9] xen/blkback: pseudo support for multi hardware queues/rings

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:	Wed, 7 Oct 2015 13:49:25 +0200
From:	Roger Pau Monné <roger.pau@...rix.com>
To:	Bob Liu <bob.liu@...cle.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 8/9] xen/blkback: pseudo support for multi hardware
 queues/rings

El 07/10/15 a les 12.50, Bob Liu ha escrit:
> 
> On 10/05/2015 11:08 PM, Roger Pau Monné wrote:
>> El 05/09/15 a les 14.39, Bob Liu ha escrit:
>>> Prepare patch for multi hardware queues/rings, the ring number was set to 1 by
>>> force.
>>
>> This should be:
>>
>> Preparatory patch for multiple hardware queues (rings). The number of
>> rings is unconditionally set to 1.
>>
>> But frankly this description is not helpful at all, you should describe
>> the preparatory changes and why you need them.
>>
> 
> Will update, the purpose is just to make each patch smaller and more readable.
> 
>>>
>>> Signed-off-by: Arianna Avanzini <avanzini.arianna@...il.com>
>>> Signed-off-by: Bob Liu <bob.liu@...cle.com>
>>> ---
>>>  drivers/block/xen-blkback/common.h |    3 +-
>>>  drivers/block/xen-blkback/xenbus.c |  328 +++++++++++++++++++++++-------------
>>>  2 files changed, 209 insertions(+), 122 deletions(-)
>>>
>>> diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
>>> index cc253d4..ba058a0 100644
>>> --- a/drivers/block/xen-blkback/common.h
>>> +++ b/drivers/block/xen-blkback/common.h
>>> @@ -339,7 +339,8 @@ struct xen_blkif {
>>>  	unsigned long long			st_wr_sect;
>>>  	unsigned int nr_ring_pages;
>>>  	/* All rings for this device */
>>> -	struct xen_blkif_ring ring;
>>> +	struct xen_blkif_ring *rings;
>>> +	unsigned int nr_rings;
>>>  };
>>>  
>>>  struct seg_buf {
>>> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
>>> index 6482ee3..04b8d0d 100644
>>> --- a/drivers/block/xen-blkback/xenbus.c
>>> +++ b/drivers/block/xen-blkback/xenbus.c
>>> @@ -26,6 +26,7 @@
>>>  /* Enlarge the array size in order to fully show blkback name. */
>>>  #define BLKBACK_NAME_LEN (20)
>>>  #define RINGREF_NAME_LEN (20)
>>> +#define RINGREF_NAME_LEN (20)
>>
>> Duplicate define?
>>
> 
> Will update.
> 
>>>  
>>>  struct backend_info {
>>>  	struct xenbus_device	*dev;
>>> @@ -84,11 +85,13 @@ static int blkback_name(struct xen_blkif *blkif, char *buf)
>>>  
>>>  static void xen_update_blkif_status(struct xen_blkif *blkif)
>>>  {
>>> -	int err;
>>> +	int err, i;
>>>  	char name[BLKBACK_NAME_LEN];
>>> +	struct xen_blkif_ring *ring;
>>> +	char per_ring_name[BLKBACK_NAME_LEN + 2];
>>
>> Hm, why don't you just add + 2 to the place where BLKBACK_NAME_LEN is
>> defined and use the same character array ("name")? This is just a waste
>> of stack.
>>
> 
> per_ring_name = name + 'queue number';
> If reuse name[], I'm not sure whether
> snprintf(name, BLKBACK_NAME_LEN + 2, "%s-%d", name, i); can work.

What I would like to avoid is having two character arrays in the stack
when IMHO we can achieve the same with a single array that's long enough.

Roger.

--
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/