From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bart Van Assche Subject: Re: [PATCH 04/12] IB/srp: Fix connection state tracking Date: Tue, 5 May 2015 11:59:15 +0200 Message-ID: <55489473.6030504@sandisk.com> References: <5541EE21.3050809@sandisk.com> <5541EE9F.8090605@sandisk.com> <5541FB3F.70902@dev.mellanox.co.il> <5542111E.1080305@sandisk.com> <5542439D.1000107@dev.mellanox.co.il> <55488E06.8040308@sandisk.com> <5548911F.8060505@dev.mellanox.co.il> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5548911F.8060505-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Sagi Grimberg , Doug Ledford Cc: James Bottomley , Sagi Grimberg , Sebastian Parschauer , linux-rdma , "linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" List-Id: linux-rdma@vger.kernel.org On 05/05/15 11:45, Sagi Grimberg wrote: > On 5/5/2015 12:31 PM, Bart Van Assche wrote: >> On 04/30/15 17:00, Sagi Grimberg wrote: >>> On 4/30/2015 2:25 PM, Bart Van Assche wrote: >>>> On 04/30/15 11:51, Sagi Grimberg wrote: >>>>> On 4/30/2015 11:58 AM, Bart Van Assche wrote: >>>>>> @@ -170,6 +170,7 @@ struct srp_rdma_ch { >>>>>> >>>>>> struct completion tsk_mgmt_done; >>>>>> u8 tsk_mgmt_status; >>>>>> + bool connected; >>>>> >>>>> I generally agree with this flag, but I'm a bit confused about the >>>>> semantics of two connected flags? Also, we check the target connected >>>>> flag on TMFs, although we are executing it on a channel (should we >>>>> check both?) >>>>> >>>>> I'd say keep only the channel connected flag, the target logic needs to >>>>> be mandated by the state. >>>> >>>> I think we need both flags. The ch->connected flag is used only in >>>> srp_destroy_qp() to verify whether a channel has been disconnected >>>> before it is destroyed. The target->connected flag provides an easy way >>>> to test the connected state in e.g. srp_disconnect_target(). >>> >>> We can just as easily check rport state. rport state is modified before >>> target-connected. I still think one is redundant. >> >> Sorry but I disagree. If e.g. both the fast_io_fail and dev_loss_tmo >> timers have been disabled then the rport state is SRP_RPORT_RUNNING all >> the time. At least in that case it is not possible to derive the >> target->connected state from the rport state. > > Can't you rely on ch->connected for that? > > We can keep both, but I just think that the meaning of > target->connected is confusing now. Hello Sagi, I will see whether I can remove the target->connected state variable. 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