Hi Subash,

On Sep 23, 2011 5:20 PM, "Subash Patel" <subashrp@gmail.com> wrote:
>
> Hi Sumit,
>
> Dont we need to have a locking member for the struct dma_buf. Below is one scenario I can think of:
>
> When the client drivers call attach() on a dma_buf object, you would possibly be modifying the attachments list. If more than one driver calls attach() for the same buffer at the same time, should'nt it be good to lock the buffer before we operate on it? The same may even apply for detach and other ops calls.

I would say since attach/detach and other ops are implementation-dependent on the exporter of buffers, if locking is required by the exporter, it can be a part of dma_buf->priv? That leaves the choice of lock implementation also on the exporter.

Please let me know if you feel otherwise.
>
> Regards,
> Subash
Thanks and regards,
Sumit.
>
>
> On 09/13/2011 07:59 PM, Sumit Semwal wrote:
>
>> +/**
>> + * struct dma_buf_ops - operations possible on struct dma_buf
>> + * @create: creates a struct dma_buf of a fixed size. Actual allocation
>> + *         does not happen here.
>> + * @attach: allows different devices to 'attach' themselves to the given
>> + *         buffer. It might return -EBUSY to signal that backing storage
>> + *         is already allocated and incompatible with the requirements
>> + *         of requesting device.
>> + * @detach: detach a given device from this buffer.
>> + * @get_scatterlist: returns list of scatter pages allocated, increases
>> + *                  usecount of the buffer. Requires atleast one attach to be
>> + *                  called before.
>> + * @put_scatterlist: decreases usecount of buffer, might deallocate scatter
>> + *                  pages.
>> + * @mmap: memory map this buffer - optional.
>> + * @release: release this buffer; to be called after the last dma_buf_put.
>> + * @sync_sg_for_cpu: sync the sg list for cpu.
>> + * @sync_sg_for_device: synch the sg list for device.
>> + */
>> +struct dma_buf_ops {
>> +       void (*attach)(struct dma_buf *, struct device *,
>> +                       struct dma_buf_attachment *);
>> +
>> +       void (*detach)(struct dma_buf *, struct dma_buf_attachment *);
>> +
>> +       struct scatterlist * (*get_scatterlist)(struct dma_buf *,
>> +                                               struct dma_buf_attachment *,
>> +                                               enum dma_buf_optype,
>> +                                               enum dma_buf_attr_flags);
>> +       void (*put_scatterlist)(struct dma_buf *, struct dma_buf_attachment *,
>> +                                               struct scatterlist *);
>> +
>> +       /* allow mmap optionally for devices that need it */
>> +       int (*mmap)(struct dma_buf *, struct vm_area_struct *);
>> +       /* after final dma_buf_put() */
>> +       void (*release)(struct dma_buf *);
>> +
>> +       /* allow allocator to take care of cache ops */
>> +       void (*sync_sg_for_cpu) (struct dma_buf *, struct device *);
>> +       void (*sync_sg_for_device)(struct dma_buf *, struct device *);
>> +};
>> +
>> +/**
>> + * struct dma_buf - shared buffer object
>> + * @file: file pointer used for sharing buffers across, and for refcounting.
>> + * @attachments: list of dma_buf_attachment that denotes all devices attached.
>> + * @ops: dma_buf_ops associated with this buffer object
>> + * @priv: user specific private data
>> + */
>> +struct dma_buf {
>> +       size_t size;
>> +       struct file *file;
>> +       struct list_head attachments;
>> +       struct dma_buf_ops *ops;
>> +       void *priv;
>> +};
>> +