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)