From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sagi Grimberg Subject: Re: [PATCH v3 11/11] IB/srp: Fix a race condition triggered by destroying a queue pair Date: Thu, 30 Oct 2014 17:10:10 +0200 Message-ID: <545254D2.3080707@dev.mellanox.co.il> References: <545240AE.6060009@acm.org> <5452420B.2070206@acm.org> <54524A7B.3060708@dev.mellanox.co.il> <545250FA.1090908@acm.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <545250FA.1090908@acm.org> Sender: linux-scsi-owner@vger.kernel.org To: Bart Van Assche , Christoph Hellwig Cc: Jens Axboe , Sagi Grimberg , Sebastian Parschauer , "Martin K. Petersen" , Robert Elliott , Ming Lei , "linux-scsi@vger.kernel.org" , linux-rdma List-Id: linux-rdma@vger.kernel.org On 10/30/2014 4:53 PM, Bart Van Assche wrote: > On 10/30/14 15:26, Sagi Grimberg wrote: >> On 10/30/2014 3:50 PM, Bart Van Assche wrote: >>> + /* Destroying a QP and reusing ch->done is only safe if not >>> connected */ >>> + WARN_ON_ONCE(target->connected); >> >> I thought we agreed that cannot happen. I guess I don't mind keeping >> it... BTW, were you able to reproduce this race as well? > > Hello Sagi, > > Agreed that it is possible to conclude from the ib_srp source code that > the above "WARN_ON_ONCE(target->connected)" statement will never be > triggered. The reason I added that statement is to have an explicit > check of that condition. Such statements can be helpful in case anyone > would like to modify the SRP initiator driver. > Yea, OK. > I have not yet been able to reproduce the crash addressed by this patch. > But the code added via this patch has been triggered in my tests. Since > this code is e.g. called from srp_free_ch_ib(), unloading the ib_srp > kernel module after login succeeded is sufficient to trigger this code. Yes of course, was just curious to know if you got this crash as well... Sagi.