On Sun, May 05, 2024 at 09:46:05AM -0700, Linus Torvalds wrote: > WHY? > > Why cannot you and Al just admit that the problem is in epoll. Always > has been, always will be.
Nobody (well, nobody who'd ever read epoll) argues that epoll is not a problem. > The fact is, it's not dma-buf that is violating any rules. Now, that is something I've a trouble with. Use of get_file() in there actually looks rather fishy, regardless of epoll. At the very least it needs a comment discouraging other instances from blindly copying this. A reference to struct file pins down more than driver-internal objects; if nothing else, it pins down a mount and if you don't have SB_NOUSER on file_inode(file)->i_sb->s_flags, it's really not a good idea. What's more, the reason for that get_file() is, AFAICS, that nodes we put into callback queue for fence(s) in question[*] are embedded into dmabuf and we don't want them gone before the callbacks have happened. Which looks fishy - it would make more sense to cancel these callbacks and drop the fence(s) in question from ->release(). I've no problem whatsoever with fs/eventpoll.c grabbing/dropping file reference around vfs_poll() calls. And I don't believe that "try to grab" has any place in dma_buf_poll(); it's just that I'm not happy about get_file() call being there in the first place. Sure, the call of ->poll() can bloody well lead to references being grabbed - by the pollwait callback, which the caller of ->poll() is aware of. It's ->poll() instance *itself* grabbing such references with vfs_poll() caller having no idea what's going on that has potential for being unpleasant. And we can't constify 'file' argument of ->poll() because of poll_wait(), so it's hard to catch those who do that kind of thing; I've explicitly said so upthread, I believe. But similar calls of get_file() in ->poll() instances (again, not the ones that are made by pollwait callback) are something to watch out for and having the caller pin struct file does not solve the problem. [*] at most one per direction, and I've no idea whether there can be more than one signalling fence for given dmabuf)
