On Thu, Jan 02, 2025 at 09:39:51AM -0400, Jason Gunthorpe wrote: > On Thu, Dec 19, 2024 at 11:04:54AM +0100, Christian König wrote: > > > > Based on all the above, I think we can conclude that since dma_buf_put() > > > does not directly (or synchronously) call the f_op->release() handler, a > > > deadlock is unlikely to occur in the scenario you described. > > Right, there is no deadlock here, and there is nothing inhernetly > wrong with using try_get like this. The locking here is ugly ugly > ugly, I forget why, but this was the best I came up with to untangle > it without deadlocks or races. Yeah, imo try_get is perfectly fine pattern. With that sorted my only request is to make the try_get specific to the dma_ops, because it only works if both ->release and the calling context of try_get follow the same rules, which by necessity are exporter specific. In full generality as a dma_buf.c interface it's just busted and there's no way to make it work, unless we inflict that locking ugliness on absolutely every exporter. > > Yeah, I agree. > > > > Interesting to know, I wasn't aware that the task_work functionality exists > > for that use case. > > IIRC it was changed a while back because call chains were getting kind of > crazy long with the file release functions and stack overflow was a > problem in some cases. Hm I thought it was also a "oh fuck deadlocks" moment, because usually most of the very deep fput calls are for temporary reference and not the final one. Unless userspace is sneaky and drops its own fd reference in a separate thread with close(), then everything goes boom. And untangling all these was impossible. -Sima -- Simona Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
