On 09/22/2015 09:49 PM, Ewan Milne wrote:
> On Thu, 2015-08-27 at 14:41 +0200, Hannes Reinecke wrote:
>> The current ALUA device_handler has two drawbacks:
>> - We're sending a 'SET TARGET PORT GROUP' command to every LUN,
>> disregarding the fact that several LUNs might be in a port group
>> and will be automatically switched whenever _any_ LUN within
>> that port group receives the command.
>> - Whenever a LUN is in 'transitioning' mode we cannot block I/O
>> to that LUN, instead the controller has to abort the command.
>> This leads to increased traffic across the wire and heavy load
>> on the controller during switchover.
>
> I'm not sure I understand what this means - why couldn't we block I/O?
> and what does 'heavy load' mean? Aborting commands is 'heavy load'?
>
If we're getting a sense code indicating that the LUN is in
transitioning _and_ we're blocking I/O we never ever send down I/Os to
that driver anymore, so we cannot receive any sense codes indicating the
transitioning is done.
At the same time, every I/O we're sending down will be returned by the
storage I/O with a sense code, requiring us to retry the command.
Hence we're constantly retrying I/O.
[ .. ]
>> @@ -811,10 +1088,17 @@ failed:
>> static void alua_bus_detach(struct scsi_device *sdev)
>> {
>> struct alua_dh_data *h = sdev->handler_data;
>> + struct alua_port_group *pg;
>>
>> - if (h->pg) {
>> - kref_put(&h->pg->kref, release_port_group);
>> - h->pg = NULL;
>> + spin_lock(&h->pg_lock);
>> + pg = h->pg;
>> + rcu_assign_pointer(h->pg, NULL);
>> + spin_unlock(&h->pg_lock);
>> + synchronize_rcu();
>> + if (pg) {
>> + if (pg->rtpg_sdev)
>> + flush_workqueue(pg->work_q);
>> + kref_put(&pg->kref, release_port_group);
>> }
>> sdev->handler_data = NULL;
>> kfree(h);
>
> So, you've already had a bit of discussion with Christoph about this,
> the main portion of your ALUA rewrite, and I won't go over all of that,
> except to say that I'd have to agree that having separate work queues
> for the different RTPG/STPG functions and having them manipulate each
> other's flags seems like we'd be better off having just one work
> function that did everything. Less messy and easier to maintain.
>
> Also, it seems like wrong ordering of kref_get() vs. scsi_device_get(),
> in alua_rtpg_queue() since they are released as kref_put() then
> scsi_device_put()?
>
Yeah, I've reworked the reference counting.
And reverted the workqueue handling to use the original model.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
[email protected] +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
