On Sun, 2011-11-27 at 10:44 +0100, Bart Van Assche wrote: > On Sun, Nov 27, 2011 at 2:06 AM, Nicholas A. Bellinger > <nab@xxxxxxxxxxxxxxx> wrote: > > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c > > index ebe39d8..2d26c21 100644 > > --- a/drivers/infiniband/ulp/srpt/ib_srpt.c > > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c > > @@ -2340,7 +2340,13 @@ static void srpt_release_channel_work(struct work_struct *w) > > BUG_ON(!sdev); > > > > se_sess = ch->sess; > > - BUG_ON(!se_sess); > > + /* > > + * May be NULL when shutting down during srpt_cm_req_recv() login > > + * exception where the caller is expected to free *ch in the > > + * failure path. > > + */ > > + if (!se_sess) > > + return; > > > > target_splice_sess_cmd_list(se_sess); > > target_wait_for_sess_cmds(se_sess, 0); > > This doesn't look like a proper fix to me. If session allocation or > registration failed, srpt_cm_req_recv() can already have invoked > kfree(ch) before srpt_release_channel_work() gets invoked. > How so..? The SRP login failure path in srpt_cm_req_recv() is already calling srpt_destroy_ch_ib() -> kthread_stop(ch->thread) to trigger the call to srpt_release_channel_work() when shutting down srpt_compl_thread(). This is currently because srpt_create_ch_ib() has already been called to start compl thread while various failures may still prevent a successful SRP login from occuring. So while the patch fixes an OOPs I've been seeing with your original 'Fix Last WQE handling' patch during SRP login failure, I'm wondering if the better changing is to try to move kthread_run() out of srpt_create_ch_ib(), and wait until after ib_send_cm_rep() has completed before starting up srpt_compl_thread(). Any thoughts..? Thanks, --nab -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
