Re: Re: [PATCH] Add ALUA hardware handler

[Mike Christie <michaelc@cs.wisc.edu>]

Hannes Reinecke wrote:
> Mike Christie wrote:
>> Hannes Reinecke wrote:
>>> Hi Alasdair,
>>>
>>> this is a patch to add a SPC-3 hardware handler. SPC-3 ALUA has
>>> provisioning for 'explicit' port group state change via the
>>> SET TARGET GROUP STATES command, and some newer storage
>>> arrays do benefit from this.
>>> Eg HP EVAs and newer EMC Clariions already support explicit ALUA.
>>>
>>> Please apply.
>>>
>>> Cheers,
>>>
>>> Hannes
>>>
>> Does this also work for adaptec or snap iscsi targets or whatever they
>> are called targets?
>>
> Don't know, I don't have one. Care to try?

I do not have it. I was asking because some people were asking about 
alua and I told them to talk to you.

> 
>>
>> Just some quick higher level comments
>>
>> +static int submit_std_inquiry(struct alua_handler *h)
>> +{
>> +    struct request *rq;
>> +    unsigned err = (DRIVER_ERROR << 24);
>> +
>> +    rq = prepare_req(h, h->inq, TPGS_INQUIRY_SIZE, READ);
>>
>>
>> I do not think you want to use GFP_KERNEL allocations in this path, so
>> all the prepare_req allocs should be changed. GFP_NOIO is probably best.
>>
> Yes, probably.
> 
>> +    if (!rq)
>> +        return err;
>> +
>> +    /* Prepare the command. */
>> +    rq->cmd[0] = INQUIRY;
>> +    rq->cmd[1] = 0;
>> +    rq->cmd[2] = 0;
>> +    rq->cmd[4] = TPGS_INQUIRY_SIZE;
>> +    rq->cmd_len = COMMAND_SIZE(INQUIRY);
>> +
>> +    blk_execute_rq(rq->q, NULL, rq, 1);
>>
>> There is only one workqueue for all the dm devices, so you do not want
>> to do one command (or how many processors there are) at a time and wait
>> for each one to complete with blk_execute_rq. You should use the async
>> one, blk_execute_rq_nowait, like rdac.
>>
> This is actually by design. Problem here is the port group as returned by
> REPORT TARGET PORT GROUPS does not have any association to the controller
> which handles these ports.
> So if we were to send all REPORT TARGET PORT GROUPS commands (roughly)
> simultaneously we're pretty much guaranteed to hit the same controller
> several times; and if we have to do a SET TARGET PORT GROUPS in addition
> we'll be having to do loads of retries as the controller might be busy
> (if he's transitioning) or reporting an UNIT ATTENTION if the port
> group states have been updated. So I'd rather keep it that way so as
> to not flood the controller. And then we're only having to send two
> commands, so this should okay even sequentially.

What if I have emc box and another non alua box? Your handler blocks 
activity for all other devices.

> 
> And incidentally, rdac implements it's own workqueue per controller

rdac does one single threaded workqueue for the entire module, and it 
does not send IO synchronously.

> as it's only capable to handle one MODE SELECT command at time.
> 

Yeah, and chandra handled it in a way that does not affect all other 
handlers.