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

[Roger Pau Monnà <roger.pau@xxxxxxxxxx>]

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@xxxxxxxxx>
>>> Signed-off-by: Bob Liu <bob.liu@xxxxxxxxxx>
>>> ---
>>>  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.


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