Re: [GIT PULL] pidfd updates

[Christian Brauner <brauner@kernel.org>]

On Thu, Apr 27, 2023 at 09:59:52AM +0100, Al Viro wrote:
> On Thu, Apr 27, 2023 at 10:33:38AM +0200, Christian Brauner wrote:
> 
> > File descriptor installation is not core functionality for drivers. It's
> > just something that they have to do and so it's not that people usually
> > put a lot of thought into it. So that's why I think an API has to be
> > dumb enough. A three call api may still be simpler to use than an overly
> > clever single call api.
> 
> Grep and you will see...  Seriously, for real mess take a look at e.g.
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c.  See those close_fd() calls in
> there?  That's completely wrong - you can't undo the insertion into
> descriptor table.

I mean, yes. There's literally a description how that's wrong in my pr
message...

> 
> I'm not suggesting that for the core kernel, but there's a plenty of
> drivers that do descriptor allocation.

That's undisputed. What I tried to say was that this is not their main
focus. The point is their design thinking is not centered on fd handling
that's just something they have to do and so they don't think about
cleanly handling for example installing an fd and the closing it again
in some failure path. And I'm not sure one should be just harshly
judging them. They're almost bound to get this wrong. You'd need to know
a bit about file descriptors to have this in mind. File descriptor
handling is subtle.

I mean, look at cachefiles_ondemand_get_fd()...