Re: [Xen-devel] [RFC PATCH v2 2/9] hyper_dmabuf: architecture specification and reference guide

[Dongwon Kim]

Thanks for your comment, Roger
I will try to polish this doc and resubmit.
(I put some comments below as well.)

On Fri, Feb 23, 2018 at 04:15:00PM +0000, Roger Pau Monnà wrote:
> On Tue, Feb 13, 2018 at 05:50:01PM -0800, Dongwon Kim wrote:
> > Reference document for hyper_DMABUF driver
> > 
> > Documentation/hyper-dmabuf-sharing.txt
> 
> This should likely be patch 1 in order for reviewers to have the
> appropriate context.
> 
> > 
> > Signed-off-by: Dongwon Kim <dongwon.kim@xxxxxxxxx>
> > ---
> >  Documentation/hyper-dmabuf-sharing.txt | 734 +++++++++++++++++++++++++++++++++
> >  1 file changed, 734 insertions(+)
> >  create mode 100644 Documentation/hyper-dmabuf-sharing.txt
> > 
> > diff --git a/Documentation/hyper-dmabuf-sharing.txt b/Documentation/hyper-dmabuf-sharing.txt
> > new file mode 100644
> > index 000000000000..928e411931e3
> > --- /dev/null
> > +++ b/Documentation/hyper-dmabuf-sharing.txt
> > @@ -0,0 +1,734 @@
> > +Linux Hyper DMABUF Driver
> > +
> > +------------------------------------------------------------------------------
> > +Section 1. Overview
> > +------------------------------------------------------------------------------
> > +
> > +Hyper_DMABUF driver is a Linux device driver running on multiple Virtual
> > +achines (VMs), which expands DMA-BUF sharing capability to the VM environment
> > +where multiple different OS instances need to share same physical data without
> > +data-copy across VMs.
> > +
> > +To share a DMA_BUF across VMs, an instance of the Hyper_DMABUF drv on the
> > +exporting VM (so called, âexporterâ) imports a local DMA_BUF from the original
> > +producer of the buffer,
> 
> The usage of export and import in the above sentence makes it almost
> impossible to understand.

Ok, it looks confusing. I think the problem is that those words are used for both
local and cross-VMs cases. I will try to clarify those. 

> 
> > then re-exports it with an unique ID, hyper_dmabuf_id
> > +for the buffer to the importing VM (so called, âimporterâ).
> 
> And this is even worse.
> 
> Maybe it would help to have some kind of flow diagram of all this
> import/export operations, but please read below.

I will add a diagram here.

> 
> > +
> > +Another instance of the Hyper_DMABUF driver on importer registers
> > +a hyper_dmabuf_id together with reference information for the shared physical
> > +pages associated with the DMA_BUF to its database when the export happens.
> > +
> > +The actual mapping of the DMA_BUF on the importerâs side is done by
> > +the Hyper_DMABUF driver when user space issues the IOCTL command to access
> > +the shared DMA_BUF. The Hyper_DMABUF driver works as both an importing and
> > +exporting driver as is, that is, no special configuration is required.
> > +Consequently, only a single module per VM is needed to enable cross-VM DMA_BUF
> > +exchange.
> 
> IMHO I need a more generic view of the problem you are trying to solve
> in the overview section. I've read the full overview, and I still have
> no idea why you need all this.

I will add some more paragrahs here to give some more generic view (and possibly
diagrams) of this driver.

> 
> I think the overview should contain at least:
> 
> 1. A description of the problem you are trying to solve.
> 2. A high level description of the proposed solution.
> 3. How the proposed solution deals with the problem described in 1.
> 
> This overview is not useful for people that don't know which problem
> you are trying to solve, like myself.

Thanks again.

> 
> Thanks, Roger.