Re: [PATCHv8 20/23] scsi: Add 'access_state' attribute

[Hannes Reinecke <hare@suse.de>]

On 02/22/2016 05:45 AM, Bart Van Assche wrote:
> On 02/21/16 00:27, Hannes Reinecke wrote:
>> On 02/20/2016 04:16 PM, Bart Van Assche wrote:
>>> On 02/20/16 00:03, Hannes Reinecke wrote:
>>>> Also when using your suggestion the 'access_state' attribute will only
>>>> be created _after_ the 'ADD' uevent, making it impossible to use it
>>>> from
>>>> udev events.
>>>
>>> Can you give an example in which it would be useful to read the ALUA
>>> state from a udev handler ? I'm not sure such an example exists.
>>>
>> When evaluating the 'access_state' from an uevent we can avoid sending
>> I/O if the path is unavailable; eg if the path is in 'transitioning' I/O
>> will be queued until that path becomes available again.
>> Which means that the uevent will be delayed during udev processing, so
>> that the event will never be read by multipathing (as it's being invoked
>> only after udev event processing has finished).
>> And in extreme cases (like OnTap takeover/giveback) it will even drop
>> the event completely due to a timeout.
> 
> Hello Hannes,
> 
> Thanks for the feedback.
> 
> Please split the access_state attribute in two sysfs attributes - one
> for the ALUA state and one for the "preferred" state. This will make it
> easier for shell scripts to process these sysfs attributes.
> 
Okay, agreed.

> I agree with using the 'is_visible' callback. But since using that
> callback will cause the ALUA sysfs attributes to become only visible
> after the ALUA handler has been attached I don't see why not to move the
> implementation of the ALUA sysfs attributes into the ALUA device handler
> source file.
> 
Thing is, there are others (like the vpg_pg attributes) which will
benefit from using the 'is_visible' callback. And move the
'access_state' attribute into the generic functions makes the footprint
smaller, as the required infrastructure is already present.
Plus I would really like to have the 'access_state' variable part of the
scsi_device, to avoid the intrinsics of having to look it up from the
device_handler data.

> BTW, since the ALUA state can change at any time after the access_state
> sysfs attribute has been read and before I/O is submitted my preference
> is to avoid triggering any SCSI commands from a udev rule that depend on
> the ALUA state.
> 
Yes, I thought about it some more, too.
The main reason why I need the 'access_state' attribute is to decouple
the multipath daemon; at the moment the multipath daemon has to issue
REPORT TARGET PORT GROUPS frequently to figure out the status, which is
causing quite some load on the target. When using the 'access_state'
attribute we would avoid doing I/O for that and have a consistent view,
both on the kernel and the multipath daemon side.

But it's actually a good thing to have the 'access_state' patch in a
different series; I've got some more patches converting the remaining
device_handler to also supply the 'access_state' values.
Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html