Re: [PATCH v2 7/7] rust: file: add abstraction for `poll_table`

[Boqun Feng]

On Wed, Dec 13, 2023 at 09:12:45AM +0000, Benno Lossin wrote:
[...]
> > 
> > Actually, there is an implied safety requirement here, it's about how
> > qproc is implemented. As we can see, PollCondVar::drop() will wait for a
> > RCU grace period, that means the waiter (a file or something) has to use
> > RCU to access the cv.wait_list, otherwise, the synchronize_rcu() in
> > PollCondVar::drop() won't help.
> 
> Good catch, this is rather important. I did not find the implementation
> of `qproc`, since it is a function pointer. Since this pattern is
> common, what is the way to find the implementation of those in general?
> 

Actually I don't find any. Ping vfs ;-)

Personally, it took me a while to get a rough understanding of the API:
it's similar to `Future::poll` (or at least the registering waker part),
it basically should registers a waiter, so that when an event happens
later, the waiter gets notified. Also the waiter registration can have a
(optional?) cancel mechanism (like an async drop of Future ;-)), and
that's what gives us headache here: cancellation needs to remove the
waiter from the wait_queue_head, which means wait_queue_head must be
valid during the removal, and that means the kfree of wait_queue_head
must be delayed to a state where no one can access it in waiter removal.

> I imagine that the pattern is used to enable dynamic selection of the
> concrete implementation, but there must be some general specification of
> what the function does, is this documented somewhere?
> 
> > To phrase it, it's more like:
> > 
> > (in the safety requirement of `PollTable::from_ptr` and the type
> > invariant of `PollTable`):
> > 
> > ", further, if the qproc function in poll_table publishs the pointer of
> > the wait_queue_head, it must publish it in a way that reads on the
> > published pointer have to be in an RCU read-side critical section."
> 
> What do you mean by `publish`?
> 

Publishing a pointer is like `Send`ing a `&T` (or put pointer in a
global variable), so that other threads can access it. Note that since
the cancel mechanism is optional (best to my knowledge), so a qproc call
may not pushlish the pointer.

Regards,
Boqun