From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bart Van Assche Subject: Re: [PATCH 1/3] IB/srp: Remove target from list before freeing Scsi_Host structure Date: Thu, 10 Oct 2013 14:48:48 +0200 Message-ID: <5256A230.9020007@acm.org> References: <5256941A.3040506@acm.org> <52569485.8050708@acm.org> <5256A183.3030404@profitbricks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5256A183.3030404-EIkl63zCoXaH+58JC4qpiA@public.gmane.org> Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Jack Wang Cc: David Dillow , Vu Pham , Sagi Grimberg , Sebastian Riemer , linux-rdma , Roland Dreier List-Id: linux-rdma@vger.kernel.org On 10/10/13 14:45, Jack Wang wrote: > On 10/10/2013 01:50 PM, Bart Van Assche wrote: >> From: Vu Pham >> >> Remove an SRP target from the SRP target list before invoking the >> last scsi_host_put() call. This change is necessary because that >> last put frees the memory that holds the srp_target_port structure. >> >> This patch prevents that the following kernel oops can be triggered: >> >> RIP: 0010:[] __lock_acquire+0x500/0x1570 >> Call Trace: >> [] lock_acquire+0xa4/0x120 >> [] _spin_lock+0x36/0x70 >> [] srp_remove_work+0xef/0x180 [ib_srp] >> [] worker_thread+0x21c/0x3d0 >> [] kthread+0x96/0xa0 >> [] child_rip+0xa/0x20 >> >> Signed-off-by: Vu Pham >> [bvanassche: Modified path description and CC'ed stable] >> Signed-off-by: Bart Van Assche >> Cc: David Dillow >> Cc: Roland Dreier >> Cc: Sebastian Riemer >> Cc: Jack Wang >> Cc: >> --- >> drivers/infiniband/ulp/srp/ib_srp.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c >> index f93baf8..f1318a8 100644 >> --- a/drivers/infiniband/ulp/srp/ib_srp.c >> +++ b/drivers/infiniband/ulp/srp/ib_srp.c >> @@ -534,6 +534,11 @@ static void srp_remove_target(struct srp_target_port *target) >> ib_destroy_cm_id(target->cm_id); >> srp_free_target_ib(target); >> srp_free_req_data(target); >> + >> + spin_lock(&target->srp_host->target_lock); >> + list_del(&target->list); >> + spin_unlock(&target->srp_host->target_lock); >> + >> scsi_host_put(target->scsi_host); >> } >> >> @@ -545,10 +550,6 @@ static void srp_remove_work(struct work_struct *work) >> WARN_ON_ONCE(target->state != SRP_TARGET_REMOVED); >> >> srp_remove_target(target); >> - >> - spin_lock(&target->srp_host->target_lock); >> - list_del(&target->list); >> - spin_unlock(&target->srp_host->target_lock); >> } > > Or move the 3 line code on top of srp_remove_target(target);? That would break srp_conn_unique() - removal from the target list must only happen after a DREQ has been sent by the initiator to the target. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html