All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: Mike Christie <michaelc@cs.wisc.edu>
Cc: Alasdair G Kergon <agk@redhat.com>,
	SCSI Mailing List <linux-scsi@vger.kernel.org>,
	device-mapper development <dm-devel@redhat.com>,
	christophe varoqui <christophe.varoqui@free.fr>
Subject: Re: [PATCH] Add ALUA hardware handler
Date: Thu, 11 Oct 2007 10:31:43 +0200	[thread overview]
Message-ID: <470DDF6F.5090909@suse.de> (raw)
In-Reply-To: <470D1DAE.1000900@cs.wisc.edu>

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@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2007-10-11  8:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-10 12:55 [PATCH] Add ALUA hardware handler Hannes Reinecke
2007-10-10 18:45 ` Mike Christie
2007-10-11  8:31   ` Hannes Reinecke [this message]
2007-10-11 17:19     ` Mike Christie
2007-10-11 17:47       ` Mike Christie
2007-10-11  2:08 ` [dm-devel] " Chandra Seetharaman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=470DDF6F.5090909@suse.de \
    --to=hare@suse.de \
    --cc=agk@redhat.com \
    --cc=christophe.varoqui@free.fr \
    --cc=dm-devel@redhat.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=michaelc@cs.wisc.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.