Re: [PATCH 16/16] scsi_dh_alua: Use workqueue for RTPG
On 04/29/2014 11:47 PM, Stewart, Sean wrote: On Fri, 2014-01-31 at 10:30 +0100, Hannes Reinecke wrote: @@ -797,37 +838,40 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg) off = 8 + (ucp[7] * 4); } - sdev_printk(KERN_INFO, sdev, - "%s: port group %02x state %c %s supports %c%c%c%c%c%c%c\n", - ALUA_DH_NAME, pg->group_id, print_alua_state(pg->state), - pg->pref ? "preferred" : "non-preferred", - valid_states&TPGS_SUPPORT_TRANSITION?'T':'t', - valid_states&TPGS_SUPPORT_OFFLINE?'O':'o', - valid_states&TPGS_SUPPORT_LBA_DEPENDENT?'L':'l', - valid_states&TPGS_SUPPORT_UNAVAILABLE?'U':'u', - valid_states&TPGS_SUPPORT_STANDBY?'S':'s', - valid_states&TPGS_SUPPORT_NONOPTIMIZED?'N':'n', - valid_states&TPGS_SUPPORT_OPTIMIZED?'A':'a'); Hannes, I was wondering why this was changed from an sdev_printk to a printk? I can see the AAS of a TPG on a target, but with it this way I do not know with respect to what logical unit it is. + printk(KERN_INFO "%s: target %s port group %02x state %c %s " + "supports %c%c%c%c%c%c%c\n", ALUA_DH_NAME, pg->target_id_str, + pg->group_id, print_alua_state(pg->state), + pg->pref ? "preferred" : "non-preferred", + valid_states&TPGS_SUPPORT_TRANSITION?'T':'t', + valid_states&TPGS_SUPPORT_OFFLINE?'O':'o', + valid_states&TPGS_SUPPORT_LBA_DEPENDENT?'L':'l', + valid_states&TPGS_SUPPORT_UNAVAILABLE?'U':'u', + valid_states&TPGS_SUPPORT_STANDBY?'S':'s', + valid_states&TPGS_SUPPORT_NONOPTIMIZED?'N':'n', + valid_states&TPGS_SUPPORT_OPTIMIZED?'A':'a'); Reasoning was the the 'target port' is actually independent on the scsi device; there might be several scsi devices pointing to the same target port. So using 'sdev_printk' here wouldn't be entirely correct. But you are right, we're losing the information about the LUN here. So I'll be reverting that bit. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 16/16] scsi_dh_alua: Use workqueue for RTPG
On Fri, 2014-01-31 at 10:30 +0100, Hannes Reinecke wrote: > @@ -797,37 +838,40 @@ static int alua_rtpg(struct scsi_device *sdev, struct > alua_port_group *pg) > off = 8 + (ucp[7] * 4); > } > > - sdev_printk(KERN_INFO, sdev, > - "%s: port group %02x state %c %s supports %c%c%c%c%c%c%c\n", > - ALUA_DH_NAME, pg->group_id, print_alua_state(pg->state), > - pg->pref ? "preferred" : "non-preferred", > - valid_states&TPGS_SUPPORT_TRANSITION?'T':'t', > - valid_states&TPGS_SUPPORT_OFFLINE?'O':'o', > - valid_states&TPGS_SUPPORT_LBA_DEPENDENT?'L':'l', > - valid_states&TPGS_SUPPORT_UNAVAILABLE?'U':'u', > - valid_states&TPGS_SUPPORT_STANDBY?'S':'s', > - valid_states&TPGS_SUPPORT_NONOPTIMIZED?'N':'n', > - valid_states&TPGS_SUPPORT_OPTIMIZED?'A':'a'); Hannes, I was wondering why this was changed from an sdev_printk to a printk? I can see the AAS of a TPG on a target, but with it this way I do not know with respect to what logical unit it is. > + printk(KERN_INFO "%s: target %s port group %02x state %c %s " > +"supports %c%c%c%c%c%c%c\n", ALUA_DH_NAME, pg->target_id_str, > +pg->group_id, print_alua_state(pg->state), > +pg->pref ? "preferred" : "non-preferred", > +valid_states&TPGS_SUPPORT_TRANSITION?'T':'t', > +valid_states&TPGS_SUPPORT_OFFLINE?'O':'o', > +valid_states&TPGS_SUPPORT_LBA_DEPENDENT?'L':'l', > +valid_states&TPGS_SUPPORT_UNAVAILABLE?'U':'u', > +valid_states&TPGS_SUPPORT_STANDBY?'S':'s', > +valid_states&TPGS_SUPPORT_NONOPTIMIZED?'N':'n', > +valid_states&TPGS_SUPPORT_OPTIMIZED?'A':'a'); > > Thanks, Sean -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 16/16] scsi_dh_alua: Use workqueue for RTPG
On 01/31/14 10:30, Hannes Reinecke wrote:
> +static void alua_check(struct scsi_device *sdev)
> +{
> + struct alua_dh_data *h = get_alua_data(sdev);
> + struct alua_port_group *pg;
> +
> + if (!h)
> + return;
> +
> + rcu_read_lock();
> + pg = rcu_dereference(h->pg);
> + if (pg) {
> + kref_get(&pg->kref);
> + rcu_read_unlock();
> + alua_rtpg_queue(pg, sdev, NULL);
> + kref_put(&pg->kref, release_port_group);
> + } else
> + rcu_read_unlock();
> +}
>From the implementation it seems to me like the target port group data
is kept just as long as it is in use ? Has it been considered to keep
that data as long as the sdev exists instead ? I think that would result
in far fewer reference count manipulations and hence in code that is
easier to read and to verify.
Thanks,
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 16/16] scsi_dh_alua: Use workqueue for RTPG
On Fri, Dec 20, 2013 at 8:13 PM, Hannes Reinecke wrote:
>
> +static void alua_rtpg_work(struct work_struct *work)
> +{
> ..
> + if (pg->flags & ALUA_PG_RUN_STPG) {
> + spin_unlock_irqrestore(&pg->rtpg_lock, flags);
> + err = alua_stpg(sdev, pg);
> + spin_lock_irqsave(&pg->rtpg_lock, flags);
> + pg->flags &= ~ALUA_PG_RUN_STPG;
> + pg->flags |= ALUA_PG_STPG_DONE;
> + if (err == SCSI_DH_RETRY) {
> + pg->flags |= ALUA_PG_RUN_RTPG;
> + pg->interval = ALUA_RTPG_DELAY_MSECS * 1000;
>
Is the line above a typo?
pg->interval is measured in second unit. You won't want to set it as
milliseconds*1000. And ALUA_RTPG_DELAY_MSECS is used for queueing delayed rtpg
work, not for delay in retry cases such as state of transitioning. What you
want may be set a new delay time to start, similar to "pg->interval += 2" in
alua_rtpg when state of transitioning is returned.
Thanks,
Vaughan
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
