On 3/7/2014 10:13 AM, Bart Van Assche wrote:
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,
Hey Bart,
A bit late but I hope not too late...
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.
Correct, taking the spinlock around ib_post_send is allowed and I don't
think any HCA drivers
will ever sleep there.
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.
I just noted that I think it should be avoided due to performance reasons.
We are going towards scsi-mq and I suspect fast-path spinlocks will
become much
more expensive in this era, and we should try to avoid them rather than
take them.
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 ?
I don't think we need to worry about that.
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.
Yes, I agree this would help.
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().
I see... will assigning scmnd = NULL only if srp_post_send will fail,
and restore the assignment as before help?
Sagi.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
