Re: [PATCH v2] eventfd: convert to ->write_iter()
From: Michal Kubecek
Date: Sat Nov 21 2020 - 12:07:24 EST
On Thu, Nov 19, 2020 at 07:46:10PM +0100, Michal Kubecek wrote:
> On Thu, Nov 19, 2020 at 06:03:15PM +0000, Christoph Hellwig wrote:
> > On Thu, Nov 19, 2020 at 07:00:19PM +0100, Michal Kubecek wrote:
> > > While eventfd ->read() callback was replaced by ->read_iter() recently by
> > > commit 12aceb89b0bc ("eventfd: convert to f_op->read_iter()"), ->write()
> > > was not replaced.
> > >
> > > Convert also ->write() to ->write_iter() to make the interface more
> > > consistent and allow non-blocking writes from e.g. io_uring. Also
> > > reorganize the code and return value handling in a similar way as it was
> > > done in eventfd_read().
> >
> > But this patch does not allow non-blocking writes. I'm really
> > suspicious as you're obviously trying to hide something from us.
>
> I already explained what my original motivation was and explained that
> it's no longer the case as the third party module that inspired me to
> take a look at this can be easily patched not to need kernel_write() to
> eventfd - and that it almost certainly will have to be patched that way
> anyway. BtW, the reason I did not mention out of tree modules in the
> commit message was exactly this: I suspected that any mention of them
> could be a red flag for some people.
>
> I believed - and I still believe - that this patch is useful for other
> reasons and Jens added another. Therefore I resubmitted with commit
> message rewritten as requested, even if I don't need it personally. I'm
> not hiding anything and I don't have time for playing your political
> games and suffer your attacks. If they are more important than improving
> kernel code, so be it. I'm annoyed enough and I don't care any more.
Just few hours later, a new version of the product was released where
the module still calls file->f_op->write() directly as it did before but
they use a dedicated userspace buffer for this kernel write. Whatever
I think about their solution, the result is that right now their module
works with current mainline but it would break with this patch. So much
for hidden agenda...
For the record, I still believe this patch is the right thing to do.
Michal Kubecek