[dm-devel] Re: [PATCH] Add ALUA hardware handler

[Hannes Reinecke]

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?

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

And incidentally, rdac implements it's own workqueue per controller
as it's only capable to handle one MODE SELECT command at time.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare at suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)