> Date: Tue, 18 Jan 2022 20:05:24 +0100
> From: "J. Hannken-Illjes" <hann...@mailbox.org>
>
> This doesn't work. Running the attached program on a current kernel:
>
> # ./revoke
> pty /dev/tty00, pid 2107
> revoke /dev/tty00
> read /dev/tty00 -> 0
> done
>
> With your patch:
>
> # ./revoke
> pty /dev/tty00, pid 975
> revoke /dev/tty00
>
> and hangs hard. The revoke() operation enters spec_node_revoke()
> and here it waits forever for sn_iocnt to become zero.
>
> As the read from the other thread set sn_iocnt to one by entering
> through spec_io_enter() the system will hang until this read returns.
>
> Am I missing something?
You're not missing anything; I missed something, and yesterday I
started drafting a change to fix it but haven't finished yet.
What I missed is that the only mechanism we have for interrupting read
(or write or ioctl) and causing it to fail is: close. This means,
with devsw built as it is, we can't wait for all I/O operations to
complete before calling close.
So, two possibilities are:
1. Wait for sn_iocnt to drain _after_ the last .d_close. I think
we'll have to do this, but it's unfortunate because it would be
nice if drivers didn't have to have driver-specific logic to wait
for all reads and writes and stuff to drain.
2. Create a new .d_cancel operation. A driver can implement this with
a function that prevents anything in the driver from issuing new
I/O operations and that interrupts pending ones, such as all
cv_waits and usbd_sync_transfers and whatnot. Then, for drivers
which support it, the sequence in .d_close on last close will be:
.d_cancel
wait for sn_ioccnt to drain
.d_close
Of course, this will only work for drivers that opt into it with a
.d_cancel function, but it will save a good deal of effort to have
the `wait for I/O operations to drain before .d_close' logic
centralized in spec_vnops.c (and perhaps be done with psref to keep
it lightweight).
There's another issue which is that sn_opencnt management in spec_open
is racy -- I had missed the part of chuq's earlier patch dealing with
that. So I'll have an updated patch soon to address these issues, but
I need to do some more work on it first and run through some more
tests.
