Re: [PATCH v1 1/3] IB/srp: Fix crash when unmapping data loop

[Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>]

On 03/06/14 17:10, Sagi Grimberg wrote:
> So I took Roland latest 3.14-rc1 and tried to reproduce this issue using
> HCA with no FMRs support and was *NOT* able to reproduce this issue. This
> issue reproduced for me on RH6 backported srp and I can't tell where is
> the delta at the moment. Perhaps Sebastian can share his scenario that
> reproduces this issue and was solved by the patch I proposed.
> 
> I suggest to try and reproduce it with your ib_srp-backport code over
> RH6, worth a look right?

Hello Sagi,

It would help if you could explain why you are concerned about holding a
spin lock during the srp_post_send() call. According to
Documentation/infiniband/core_locking.txt ib_post_send() is not allowed
to sleep. And before the SCSI host lock push-down the SCSI host lock was
held around the srp_queuecommand() invocation. In other words, holding a
spin lock around the srp_post_send() call in srp_queuecommand() is
neither new nor disallowed by the RDMA API documentation. And if there
would be any (legacy?) RDMA HCA drivers that are not upstream and for
which it is not safe to invoke ib_post_send() with a spin lock held, how
about invoking srp_post_send() in distro's that support these drivers
without holding target->lock ? The patch I posted fixes a race between
srp_finish_req() and the error path in srp_queuecommand by setting
req->scmnd after the srp_post_send() call instead of before. Holding
target->lock around the srp_post_send() call and the req->scmnd
assignment is only needed to avoid the (theoretical) race where
processing of an SRP reply would already have been started before
req->scmnd has been assigned by srp_queuecommand().

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