On Thu, Aug 18, 2022 at 03:37:01PM +0200, Christian König wrote: > Am 18.08.22 um 15:16 schrieb Jason Gunthorpe: > > On Thu, Aug 18, 2022 at 02:58:10PM +0200, Christian König wrote: > > > > > > > The only thing I'm not 100% convinced of is dma_buf_try_get(), I've seen > > > > > this incorrectly used so many times that I can't count them any more. > > > > > > > > > > Would that be somehow avoidable? Or could you at least explain the use case > > > > > a bit better. > > > > I didn't see a way, maybe you know of one > > > For GEM objects we usually don't use the reference count of the DMA-buf, but > > > rather that of the GEM object for this. But that's not an ideal solution > > > either. > > You can't really ignore the dmabuf refcount. At some point you have to > > deal with the dmabuf being asynchronously released by userspace. > > Yeah, but in this case the dma-buf is just a reference to the real/private > object which holds the backing store. The gem approach is backwards to what I did here. GEM holds a singleton pointer to the dmabuf and holds a reference on it as long as it has the pointer. This means the dmabuf can not be freed until the GEM object is freed. For this I held a "weak reference" on the dmabuf in a list, and we convert the weak reference to a strong reference in the usual way using a try_get. The reason it is different is because the VFIO interface allows creating a DMABUF with unique parameters on every user request. Eg the user can select a BAR index and a slice of the MMIO space unique to each each request and this results in a unique DMABUF. Due to this we have to store a list of DMABUFs and we need the DMABUF's to clean up their memory when the user closes the file. > > So we could delete the try_buf and just rely on move being safe on > > partially destroyed dma_buf's as part of the API design. > > I think that might be the more defensive approach. A comment on the > dma_buf_move_notify() function should probably be a good idea. IMHO, it is an anti-pattern. The caller should hold a strong reference on an object before invoking any API surface. Upgrading a weak reference to a strong reference requires the standard "try get" API. But if you feel strongly I don't mind dropping the try_get around move. Jason
