On Fri, Jun 13, 2025 at 04:04:46PM +0200, Simona Vetter wrote: > On Fri, Jun 13, 2025 at 03:12:01PM +0200, Christian König wrote: > > It is possible through flink or IOCTLs like MODE_GETFB2 to create > > multiple handles for the same underlying GEM object. > > > > But in prime we explicitely don't want to have multiple handles for the > > same DMA-buf. So just ignore it if a DMA-buf is exported with another > > handle. > > > > This was made obvious by removing the extra check in > > drm_gem_prime_handle_to_dmabuf() to not add the handle if we could already > > find it in the housekeeping structures. > > > > Signed-off-by: Christian König <[email protected]> > > --- > > drivers/gpu/drm/drm_prime.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > > index 1d93b44c00c4..f5f30d947b61 100644 > > --- a/drivers/gpu/drm/drm_prime.c > > +++ b/drivers/gpu/drm/drm_prime.c > > @@ -113,6 +113,17 @@ static int drm_prime_add_buf_handle(struct > > drm_prime_file_private *prime_fpriv, > > > > rb = *p; > > pos = rb_entry(rb, struct drm_prime_member, dmabuf_rb); > > + > > + /* > > + * Just ignore the new handle if we already have an handle for > > + * this DMA-buf. > > + */ > > + if (dma_buf == pos->dma_buf) { > > + dma_buf_put(dma_buf); > > + kfree(member); > > + return 0; > > This feels a bit brittle, because this case should only be possible when > called from drm_gem_prime_handle_to_dmabuf and not from > drm_gem_prime_fd_to_handle() (where it would indicate a real race and > hence bug in our code). > > I think drm_gem_prime_fd_to_handle() should WARN_ON if it hits this case.
Simplest would be to return -EEXISTS here and then either silence that errno or warn about it in the two call sites. Not pretty, but everything else looks worse. -Sima > > Otherwise yes this is the functional change that I've missed :-/ Note that > there's no race in the original code, because it's all protected by the > file_priv->prime.lock. Which means I think you're claim that you've only > widened the race with your patch is wrong. > > Cheers, Sima > > > + > > + } > > if (dma_buf > pos->dma_buf) > > p = &rb->rb_right; > > else > > -- > > 2.34.1 > > > > -- > Simona Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Simona Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
