On Thu, Feb 02, 2012 at 04:59:54PM -0800, Bhanu Prakash Gollapudi wrote:
> On 2/2/2012 1:03 PM, Neil Horman wrote:
> >Fix it by adding an ema_lock to list accesses. Make it an rcu lock so we
> >don't
> >get Warnings when we try to tx a frame with a spinlock held
> >
>
>
> > /**
> >@@ -2166,6 +2182,7 @@ struct fc_exch_mgr_anchor *fc_exch_mgr_add(struct
> >fc_lport *lport,
> > bool (*match)(struct fc_frame *))
> > {
> > struct fc_exch_mgr_anchor *ema;
> >+ unsigned long flags;
> >
> > ema = kmalloc(sizeof(*ema), GFP_ATOMIC);
> > if (!ema)
> >@@ -2174,8 +2191,10 @@ struct fc_exch_mgr_anchor *fc_exch_mgr_add(struct
> >fc_lport *lport,
> > ema->mp = mp;
> > ema->match = match;
> > /* add EM anchor to EM anchors list */
> >- list_add_tail(&ema->ema_list,&lport->ema_list);
> >+ spin_lock_irqsave(&lport->ema_lock, flags);
>
> Neil, why do we need spin_lock_irqsave() for ema_lock?
> spin_lock_bh() is good enough I think. Probably you're getting the
> warning because you are using irqsave.
>
spin_lock_bh might be sufficient, I figured we could start with irqsave and
convert individual paths to bh as they were cleared. Theres really very little
performance difference, and both are only held for very short periods while the
list is added to/removed from. The longer traversals are rcu_read_lock
protected.
As for the warning, there is none, you're confusing this patch with my earlier
error in which I held and exchanges ex_lock with irqs disabled. This lock
protects the ema_list from these potentially concurrent paths:
1) traversals of the list in fc_find_ema from softirq context in the recieve
path for fcoe (called from fcoe_rcv->fcoe_recv_frame->fc_exch_recv->fc_find_ema)
2) traversals of the list that occur during parallel bottom halves in
fc_exch_alloc, fc_seq_assign, and fc_exch_lookup that may occur in the receive
path operating in softirq on other cpus in parallel
3) list_add and list_del operations that can be intiated via userspace calls
through sysfs.
Theres no warning because the lock is only held during the short period when we
add or remove an entry. The read side is protected via rcu_read_lock.
Neil
_______________________________________________
devel mailing list
devel@open-fcoe.org
https://lists.open-fcoe.org/mailman/listinfo/devel
