diff mbox series

[v2,1/4] dma-buf: Add dma_buf_try_get()

Message ID 1-v2-472615b3877e+28f7-vfio_dma_buf_jgg@nvidia.com (mailing list archive)
State New, archived
Headers show
Series Allow MMIO regions to be exported through dma-buf | expand

Commit Message

Jason Gunthorpe Aug. 31, 2022, 11:12 p.m. UTC
Used to increment the refcount of the dma buf's struct file, only if the
refcount is not zero. Useful to allow the struct file's lifetime to
control the lifetime of the dmabuf while still letting the driver to keep
track of created dmabufs.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 include/linux/dma-buf.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Christian König Sept. 1, 2022, 7:55 a.m. UTC | #1
Am 01.09.22 um 01:12 schrieb Jason Gunthorpe:
> Used to increment the refcount of the dma buf's struct file, only if the
> refcount is not zero. Useful to allow the struct file's lifetime to
> control the lifetime of the dmabuf while still letting the driver to keep
> track of created dmabufs.
>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>   include/linux/dma-buf.h | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 71731796c8c3a8..a35f1554f2fb36 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -618,6 +618,19 @@ int dma_buf_fd(struct dma_buf *dmabuf, int flags);
>   struct dma_buf *dma_buf_get(int fd);
>   void dma_buf_put(struct dma_buf *dmabuf);
>   
> +/**
> + * dma_buf_try_get - try to get a reference on a dmabuf
> + * @dmabuf - the dmabuf to get
> + *
> + * Returns true if a reference was successfully obtained. The caller must
> + * interlock with the dmabuf's release function in some way, such as RCU, to
> + * ensure that this is not called on freed memory.

I still have a bad feeling about this, but I also see that we can only 
choose between evils here.

Could you just call get_file_rcu() from the exporter with a comment 
explaining why this works instead?

That would at least not give importers the opportunity to abuse this.

Thanks,
Christian.

> + */
> +static inline bool dma_buf_try_get(struct dma_buf *dmabuf)
> +{
> +	return get_file_rcu(dmabuf->file);
> +}
> +
>   struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *,
>   					enum dma_data_direction);
>   void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *,
Jason Gunthorpe Sept. 6, 2022, 4:44 p.m. UTC | #2
On Thu, Sep 01, 2022 at 09:55:08AM +0200, Christian König wrote:
> Am 01.09.22 um 01:12 schrieb Jason Gunthorpe:
> > Used to increment the refcount of the dma buf's struct file, only if the
> > refcount is not zero. Useful to allow the struct file's lifetime to
> > control the lifetime of the dmabuf while still letting the driver to keep
> > track of created dmabufs.
> > 
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > ---
> >   include/linux/dma-buf.h | 13 +++++++++++++
> >   1 file changed, 13 insertions(+)
> > 
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index 71731796c8c3a8..a35f1554f2fb36 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -618,6 +618,19 @@ int dma_buf_fd(struct dma_buf *dmabuf, int flags);
> >   struct dma_buf *dma_buf_get(int fd);
> >   void dma_buf_put(struct dma_buf *dmabuf);
> > +/**
> > + * dma_buf_try_get - try to get a reference on a dmabuf
> > + * @dmabuf - the dmabuf to get
> > + *
> > + * Returns true if a reference was successfully obtained. The caller must
> > + * interlock with the dmabuf's release function in some way, such as RCU, to
> > + * ensure that this is not called on freed memory.
> 
> I still have a bad feeling about this, but I also see that we can only
> choose between evils here.
> 
> Could you just call get_file_rcu() from the exporter with a comment
> explaining why this works instead?

I guess, are you sure? It seems very hacky.

Jason
Christian König Sept. 6, 2022, 5:52 p.m. UTC | #3
Am 06.09.22 um 18:44 schrieb Jason Gunthorpe:
> On Thu, Sep 01, 2022 at 09:55:08AM +0200, Christian König wrote:
>> Am 01.09.22 um 01:12 schrieb Jason Gunthorpe:
>>> Used to increment the refcount of the dma buf's struct file, only if the
>>> refcount is not zero. Useful to allow the struct file's lifetime to
>>> control the lifetime of the dmabuf while still letting the driver to keep
>>> track of created dmabufs.
>>>
>>> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
>>> ---
>>>    include/linux/dma-buf.h | 13 +++++++++++++
>>>    1 file changed, 13 insertions(+)
>>>
>>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
>>> index 71731796c8c3a8..a35f1554f2fb36 100644
>>> --- a/include/linux/dma-buf.h
>>> +++ b/include/linux/dma-buf.h
>>> @@ -618,6 +618,19 @@ int dma_buf_fd(struct dma_buf *dmabuf, int flags);
>>>    struct dma_buf *dma_buf_get(int fd);
>>>    void dma_buf_put(struct dma_buf *dmabuf);
>>> +/**
>>> + * dma_buf_try_get - try to get a reference on a dmabuf
>>> + * @dmabuf - the dmabuf to get
>>> + *
>>> + * Returns true if a reference was successfully obtained. The caller must
>>> + * interlock with the dmabuf's release function in some way, such as RCU, to
>>> + * ensure that this is not called on freed memory.
>> I still have a bad feeling about this, but I also see that we can only
>> choose between evils here.
>>
>> Could you just call get_file_rcu() from the exporter with a comment
>> explaining why this works instead?
> I guess, are you sure? It seems very hacky.

Yes, it's still better than exposing a dma_buf_try_get() interface to 
everyone.

Keep in mind that those functions here are mostly supposed to be used by 
the importer and not the exporter.

Christian.

>
> Jason
diff mbox series

Patch

diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 71731796c8c3a8..a35f1554f2fb36 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -618,6 +618,19 @@  int dma_buf_fd(struct dma_buf *dmabuf, int flags);
 struct dma_buf *dma_buf_get(int fd);
 void dma_buf_put(struct dma_buf *dmabuf);
 
+/**
+ * dma_buf_try_get - try to get a reference on a dmabuf
+ * @dmabuf - the dmabuf to get
+ *
+ * Returns true if a reference was successfully obtained. The caller must
+ * interlock with the dmabuf's release function in some way, such as RCU, to
+ * ensure that this is not called on freed memory.
+ */
+static inline bool dma_buf_try_get(struct dma_buf *dmabuf)
+{
+	return get_file_rcu(dmabuf->file);
+}
+
 struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *,
 					enum dma_data_direction);
 void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *,