>  static void release_port_group(struct kref *kref)
>  {
>       struct alua_port_group *pg;
>  
> +     synchronize_rcu();
>       pg = container_of(kref, struct alua_port_group, kref);
> +     if (pg->rtpg_sdev)
> +             flush_delayed_work(&pg->rtpg_work);
>       spin_lock(&port_group_lock);
>       list_del(&pg->node);
>       spin_unlock(&port_group_lock);

I don't think this is correct - we need a grace period after the
list_del to prevent new lookups.  I suspect the right thing to do here
is to simply use kfree_rcu for the pg.  This also avoids waiting for
whole grace periods for each deleted port_group, which might be rather
expensive.

> +     /* Check for existing port group references */
> +     spin_lock(&h->pg_lock);
> +     if (h->pg) {
> +             old_pg = pg;
> +             if (h->pg != pg) {
> +                     /* port group has changed. Update to new port group */
> +                     old_pg = h->pg;
> +                     rcu_assign_pointer(h->pg, pg);
> +             }
> +     } else {
> +             rcu_assign_pointer(h->pg, pg);
> +     }

This looks confusing - pg is the structure we just allocated, so why
do we assign it to old_pg?  It seems like the above could be written
similar and more clear as:


        olg_pg = h->pg;
        if (pg != old_pg)
                rcu_assign_pointer(h->pg, pg);

> +     if (!sdev) {
> +             WARN_ON(pg->flags & ALUA_PG_RUN_RTPG ||
> +                     pg->flags & ALUA_PG_RUN_STPG);

Please use two separate WARN_ONs here to know which one triggered from
the line in the kernel log.
--
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

Reply via email to