Benjamin LaHaise <bcrl@xxxxxxxxx> writes:
>> At the very least, you need to also include man page updates for this.
>> Honestly, though, we should not have let this patch set go in as is. If
>
> Yes, the man page needs to be updated, but this change in the API is
> backwards compatible. The existing man page says that it returns 0 on
> sucess and returns an io_event -- anything other than a 0 return implies
> that a completion event will be returned later. Cancellation has never
> provided a 100% guarantee that it will succeed without providing a
> completion event.
The man page states:
If the AIO context is found, the event will be canceled and then
copied into the memory pointed to by result without being placed
into the completion queue.
That sounds pretty definitive to me.
>> any software actually wrote code to handle cancellation, this change
>> would surely cause confusion. That said, I think you got away with this
>> for two reasons:
>> 1) no drivers implements a cancellation routine (except the crazy usb
>> gadgetfs thing)
>> 2) userspace code may not even call io_cancel (who knows?), and if they
>> do, well, see point 1.
>
> I implemented some test code to verify that the new approach to cancel
> works.
I have no doubt it works, Ben. :) My main concern is a change to the
kernel<->user ABI.
>> My preference would be for this behavior change to be reverted. You
>> could make a case for keeping the change and updating the
>> documentation. I'm not convinced 100% one way or the other. So what do
>> you (and others) think?
>
> I'm not convinced it needs to be reverted. Anything that's not a file or
> block device requires cancellation support, and the in-kernel api changes
> were meant to better handle various race conditions.
I don't understand why you're pitting this change against working
cancelation support. They seem to be separate issues to me.
>> p.s. I did do a quick google search, and the only caller of io_cancel I
>> could find was a test harness. But, that's certainly not proof that
>> nobody does it!
>
> You're ignoring the fact that cancellation is used internally to the kernel
> on process exit or io context destruction. Cancel methods have to work, as
> otherwise a process will hang at exit.
I was not ignoring that at all. We don't provide backwards
compatibility for kernel modules, and the modules in-tree are changed
when interfaces change. I'm really only concerned about whether we
think it matters that the kernel<->user ABI was changed.
I think at some level we're talking past each other. Hopefully I've
clarified my questions.
Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
