Re: Orangefs ABI documentation

[Al Viro <viro@ZenIV.linux.org.uk>]

On Sat, Jan 23, 2016 at 04:35:32PM -0500, Mike Marshall wrote:
> I compiled and tested the new patches,
> they seem to work more than great, unless
> it is just my imagination that the kernel
> module is much faster now. I'll measure it
> with more than seat-of-the-pants to see
> for sure. The patches are pushed to
> the for-next branch.

As long as the speedup is not due to not actually doing the IO... ;-)

> My "gnarly test" can get the code to flow
> into wait_for_cancellation_downcall, but
> never would flow past the
> "if (signal_pending(current)) {" block,
> though that doesn't prove anything...

AFAICS, it is possible to get there without a signal - you need the first
attempt of file_read / file_write to fail on disconnect, be retried and
that retry to fail on timeout.  _Then_ you get to sending a cancel without
any signals ever being sent to you.  

Said that, I'm not sure that this "we don't wait at all if the signals
are pending" is right - consider e.g. a read getting killed after it had
been picked by daemon and before the daemon gets a chance to reply.
We submit a cancel, but unless daemon picks it immediately, we don't
even bother waiting - we see that cancel hadn't been serviced yet, that
a signal is pending and we remove the cancel from request list.  In that
scenario daemon doesn't get a chance to see the cancel request at all...

In which situations do we want cancel to be given to daemon and do we
need to wait for its response?

> I had to look at the wiki page for "cargo culting" <g>...
> When Becky was working on the cancellation
> problem I alluded to earlier, we talked about and
> suspected the spin_lock_irqsaves in
> service_operation were not appropriate...

It isn't - first of all, prepare_to_wait() does spin_lock_irqsave() itself
(on the queue lock), so interrupt-disabling bits are there anyway.  And
the exclusion on op->lock is completely pointless, obviously - you are waiting
for bufmap initialization, which has nothing to do with specific op.
Note that by that point op is not on any lists, so there's nobody who could
see it, let alone grab the same spinlock...