RE: srp_transport: Fix atttribute registration race

["Bill Boas" <bboas-klaOcWyJdxkshyMvu7JE4pqQE7yCjDx5@public.gmane.org>]

Bart, James, Dave,

At SC we are running a video streaming demo across a 7000 mile fiber
distance using IB extension boxes from Bay Microsystems at 40 Gbps. The
video streaming app uses SRP to transmit the streams across this long
distance link. Performance is not all what we would have hoped. We think
that because SRP uses RC mode between initiator and target that it is the RC
behavior over this distance that is impacting performance. Are any of you
aware of an SRP implementation that uses UC mode.

We appreciate your respones in advance,

Bill
Bill Boas
Tel: 01-510-375-8840
Email: bboas-klaOcWyJdxkshyMvu7JE4pqQE7yCjDx5@public.gmane.org


 
-----Original Message-----
From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
[mailto:linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Bart Van Assche
Sent: Tuesday, November 01, 2011 1:48 PM
To: James Bottomley
Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Fujita Tomonori;
Brian King; David Dillow; Roland Dreier
Subject: Re: srp_transport: Fix atttribute registration race

On Mon, Oct 31, 2011 at 10:33 AM, James Bottomley
<James.Bottomley-JuX6DAaQMKPCXq6kfMZ53/egYHeGw8Jk@public.gmane.org> wrote:
> On Fri, 2011-10-21 at 18:57 +0200, Bart Van Assche wrote:
> > Register transport attributes after the attribute array has been set 
> > up instead of before. The current code is racy because there is no 
> > guarantee that the CPU examining the attribute container will see 
> > all values written to the container.
>
> I don't agree with this change log.  As far as the kernel is 
> concerned, nothing happens until that function returns because the 
> only way to use anything is to get a match to succeed, and they all 
> check for
> ->transportt which will be NULL.
>
> I can accept that it's best practise to initialise something before 
> registering it, because the reverse excites everyone's bogosity 
> sensors, it's just not a bug in this case.

It's very well possible that I have missed something, but it's not clear to
me what's wrong with the patch description ? At least with ib_srp device
registration happens from another context than module initialization so it
can potentially run on another CPU. I don't see any kind of memory barrier
after the initialization of the SRP transport attribute array. So at least
in theory and on an architecture with a sufficiently weak memory model (e.g.
the POWER
architecture) there is no guarantee that these store operations will be
observed before an SRP device gets registered. Even more, there is no
guarantee that the CPU on which device registration happens will observe
these store operations in the same order as these were performed on the CPU
that ran srp_attach_transport().

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

--
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