> +struct alua_dh_data {
> + struct alua_port_group *pg;
> + int rel_port;
> + int tpgs;
The tpgs file doesn't make much sense in the new alua_dh_data
structure. Please return it explicitly from alua_check_tpgs and
assign it directly to the member in struct alua_port_group.
> +static void release_port_group(struct kref *kref)
> +{
> + struct alua_port_group *pg;
> +
> + pg = container_of(kref, struct alua_port_group, kref);
> + printk(KERN_WARNING "alua: release port group %d\n", pg->group_id);
Why is this a warning? I'd consider it nothing but debug noise.
> - err = alua_rtpg(sdev, h, 0);
> - if (err != SCSI_DH_OK)
> + if (err != SCSI_DH_OK || !h->pg)
> goto out;
>
> + kref_get(&h->pg->kref);
What prevents this kref_get from racing with the last kref_put?
I think all the kref_get calls in this patch need to be
kref_get_unless_zero with proper error handling.
> + rcu_read_lock();
> + pg = rcu_dereference(h->pg);
> + if (!pg) {
> + rcu_read_unlock();
> + return -ENXIO;
> + }
What is this rcu_read_lock supposed to protect against given
that struct alua_port_group isn't RCU freed? I think this needs
to be another kref_get_unless_zero.
> +
> if (optimize)
> - h->flags |= ALUA_OPTIMIZE_STPG;
> + pg->flags |= ALUA_OPTIMIZE_STPG;
> else
> - h->flags &= ~ALUA_OPTIMIZE_STPG;
> + pg->flags |= ~ALUA_OPTIMIZE_STPG;
> + rcu_read_unlock();
The read-modify-write of pg->flags will need some sort of locking.
Seems like most modifications of it are under pg->rtpg_lock, so
I'd suggest to enforce that as a strict rule.
> + err = alua_rtpg(sdev, h->pg, 1);
> + if (err != SCSI_DH_OK) {
> + kref_put(&h->pg->kref, release_port_group);
> + goto out;
goto out_put_pg;
out_put_pg:
> + kref_put(&h->pg->kref, release_port_group);
> out:
> if (fn)
> fn(data, err);
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
