On Mon, 2011-10-24 at 20:39 -0700, Bhanu Prakash Gollapudi wrote:
> On 10/24/2011 5:22 PM, Vasu Dev wrote:
> > On Mon, 2011-10-24 at 14:36 -0700, Bhanu Prakash Gollapudi wrote:
> >> On 10/24/2011 1:42 PM, Neil Horman wrote:
> >>> On Mon, Oct 24, 2011 at 11:59:52AM -0700, Zou, Yi wrote:
> >>>>
> >>>>>
> >>>>> On 10/21/2011 4:36 PM, Neil Horman wrote:
> >>>>>> This oops was reported to me recently:
> >>>>>> PID: 5176 TASK: ffff880215274100 CPU: 0 COMMAND: "fc_rport_eq"
> >>>>>> 0 [ffff880218c65760] machine_kexec at ffffffff81031d3b
> >>>>>> 1 [ffff880218c657c0] crash_kexec at ffffffff810b8e92
> >>>>>> 2 [ffff880218c65890] oops_end at ffffffff814ef890
> >>>>>> 3 [ffff880218c658c0] no_context at ffffffff8104226b
> >>>>>> 4 [ffff880218c65910] __bad_area_nosemaphore at ffffffff810424f5
> >>>>>> 5 [ffff880218c65960] bad_area_nosemaphore at ffffffff810425c3
> >>>>>> 6 [ffff880218c65970] __do_page_fault at ffffffff81042c9d
> >>>>>> 7 [ffff880218c65a90] do_page_fault at ffffffff814f186e
> >>>>>> 8 [ffff880218c65ac0] page_fault at ffffffff814eec25
> >>>>>> 9 [ffff880218c65bb8] fc_fcp_complete_locked at ffffffffa02ed739 [libfc]
> >>>>>> 10 [ffff880218c65c08] fc_fcp_retry_cmd at ffffffffa02ed86f [libfc]
> >>>>>> 11 [ffff880218c65c28] fc_fcp_recv at ffffffffa02eed3f [libfc]
> >>>>>> 12 [ffff880218c65d28] fc_exch_mgr_reset at ffffffffa02e2373 [libfc]
> >>>>>> 13 [ffff880218c65db8] fc_rport_work at ffffffffa02e9f10 [libfc]
> >>>>>> 14 [ffff880218c65e38] worker_thread at ffffffff8108b250
> >>>>>> 15 [ffff880218c65ee8] kthread at ffffffff81090806
> >>>>>> 16 [ffff880218c65f48] kernel_thread at ffffffff8100c10a
> >>>>>>
> >>>>>> It results from two contexts that try to manipulate the same
> >>>>> fcoe_exch_pool
> >>>>>> without syncronizing themselves:
> >>>>>>
> >>>>>> 1) The fcoe event_work workqueue which calls
> >>>>>> fc_rport_work
> >>>>>> fc_exch_mgr_reset
> >>>>>> fc_exch_pool_reset
> >>>>>>
> >>>>>> 2) The FCOE transport destroy path, which schedules a destroy_work
> >>>>> workqueue,
> >>>>>> calling:
> >>>>>> fcoe_destroy_work
> >>>>>> fcoe_if_destroy
> >>>>>> fc_exch_mgr_free
> >>>>>> fc_exch_mgr_del
> >>>>>> fc_exch_mgr_destroy
> >>>>>>
> >>>>>> The pool_reset path holds the pool look, but no references to the pool
> >>>>> manager
> >>>>>> kobject, while exch_mgr_destroy path drops what is ostensibly the last
> >>>>>> reference to the pool manager kobject (causing its freeing), while not
> >>>>> holding
> >>>>>> the pool lock.
> >>>>>>
> >>>>>> The attached patch has been confirmed to prevent the panic.
> >>>>>>
> >>>>>> Signed-off-by: Neil Horman<nhor...@tuxdriver.com>
> >>>>>> CC: Robert Love<robert.w.l...@intel.com>
> >>>>>> ---
> >>>>>> drivers/scsi/libfc/fc_exch.c | 2 ++
> >>>>>> 1 files changed, 2 insertions(+), 0 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/scsi/libfc/fc_exch.c
> >>>>> b/drivers/scsi/libfc/fc_exch.c
> >>>>>> index 8325489..d7c8100 100644
> >>>>>> --- a/drivers/scsi/libfc/fc_exch.c
> >>>>>> +++ b/drivers/scsi/libfc/fc_exch.c
> >>>>>> @@ -1815,10 +1815,12 @@ void fc_exch_mgr_reset(struct fc_lport *lport,
> >>>>> u32 sid, u32 did)
> >>>>>> unsigned int cpu;
> >>>>>>
> >>>>>> list_for_each_entry(ema,&lport->ema_list, ema_list) {
> >>>>>> + kref_get(&ema->mp->kref)
> >>>>>
> >>>>> If the above contexts are not synchronized, we may be accessing illegal
> >>>>> pointer in ema. Just before calling kref_get(), the ema may have been
> >>>>> free'd in fc_exch_mgr_del(). We do not still have protection for it,
> >>>>> right? am I missing something?
> >>>>>
> >>>> Are we not holding lport lock when doing fc_exch_mgr_free() to guard the
> >>>> ema_list?
> >>>> I thought we are, I might be wrong.
> >>>>
> >>> I thought that lock got grabbed somwhere in fcoe_destroy_work
> >> I do not see lport lock being taken while calling fc_exch_mgr_free().
> >> Are you referring to fcoe_config_mutex? If so, that will serialize exch
> >> mgr add/del operations, not the ema_list traversal in fc_exch_mgr_reset
> >> from fc_rport_work context while the ema delete is going-on in another
> >> context.
> >
> > I also don't see lport lock used here serializing here and may cause
> > circular locking around fc_exch_mgr_reset().
> >
> >>
> >> I think this condition should not have happened in the first place.
> >> fc_fabric_logoff() should have waited for all the rport_works to
> >> complete. So, when fc_exch_mgr_free() is called the rport_work should
> >> have been completed. Is there a different scenario to see the issue?
> >
> > It can happen as Neil described in the patch due to deferred rport
> > worker executing after destroy thread done executing which also frees up
> > all resources including EM/Pools.
>
> Vasu, I don't think that can happen.
>
> > The issues showed up with EM resources
> > but rport worker can try accessing freed lport also, so I think fix
> > should be to flush pending rport worker thread before proceeding with
> > freeing EM or lport resource.
>
> Current code already does this.
>
> rport worker threads are getting flushed in fc_fabric_logoff by calling
> lport->tt.rport_flush_queue(), which flushes rport_event_queue.
Yeap, the rport_event_queue is flushed before freeing EM or lport but
while looking rport workers code I noticed that we have a another race
with rport->retry_work on system wq. I mean what if rport_retry pending
and at that time destroy goes ahead and deletes lport and its rports, EM
etc and then later pending retry_worker executes. Looks like another
race unless I'm missing something.
> This is
> called before freeing up all resources including em/pools
> (fc_exch_mgr_free) in fcoe_if_destroy().
>
The reported crash trace is not with accessing freed EM or its pool
anyway since crash is from fc_fcp_complete_locked() during expected
exches reset from rport worker. So crash is with a FCP layer and don't
know how Neil concluded its issue with accessing freed EM pool with
described two code paths.
Neil what are steps to reproduce this issue quickly ? That may help to
root cause issue by capturing debug info (exch id) along with trace.
Thanks
Vasu
_______________________________________________
devel mailing list
devel@open-fcoe.org
https://lists.open-fcoe.org/mailman/listinfo/devel
