Xen devel - Feb 2007 - xenbus_backend_client.c / xenbus_client.c merger

Although I don''t know the full history, it looks like at some point
linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_backend_client.c was
created to hold "backend only" code that would otherwise be in
linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c.

However, the code in xenbus_backend_client.c isn''t currently specific
to
the backend - it just happens to be that no frontends use it.  At least
that''s how it looks to me.

A frontend I''m working on (and have submitted here as a patch a few
times) does use this "backend only" code.  In particular the
xenbus_map_ring family of functions.  I would therefore like to move
this from xenbus_backend_client.c into xenbus_client.c.  This would
leave xenbus_backend_client.c containing just xenbus_dev_is_online()
which also looks potentially useful to a frontend, and so I propose
moving that too and then removing the xenbus_backend_client.c file
altogether.

Does anyone have any comments on this change?  I''m happy to submit a
patch for it (e.g. see attached).  In particular, if anyone is aware of
the reason for this code split that would be interesting as I like to
understand things I''m changing!

Functionally this only affects those using separate domU/dom0 kernels.
If using a unified kernel the symbols from both files were available in
both domains regardless.

Thanks

Kieran


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 19/2/07 14:00, "Kieran Mansley" <kmansley@solarflare.com>
wrote:
> Although I don''t know the full history, it looks like at some
point
> linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_backend_client.c was
> created to hold "backend only" code that would otherwise be in
> linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c.
> 
> However, the code in xenbus_backend_client.c isn''t currently
specific to
> the backend - it just happens to be that no frontends use it.  At least
> that''s how it looks to me.
Currently we have a model that frontends supply memory for ring buffers,
which backends map into their kernel address space. Where is the memory
coming from that your frontend maps? Is it appropriate to be using grant
table entries to refer to and map that memory?

 -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Mon, 2007-02-19 at 15:17 +0000, Keir Fraser wrote:
> On 19/2/07 14:00, "Kieran Mansley"
<kmansley@solarflare.com> wrote:
> 
> > Although I don''t know the full history, it looks like at some
point
> > linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_backend_client.c was
> > created to hold "backend only" code that would otherwise be
in
> > linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c.
> > 
> > However, the code in xenbus_backend_client.c isn''t currently
specific to
> > the backend - it just happens to be that no frontends use it.  At
least
> > that''s how it looks to me.
> 
> Currently we have a model that frontends supply memory for ring buffers,
> which backends map into their kernel address space.
Which is on the whole a very good idea.
> Where is the memory
> coming from that your frontend maps?
It''s host memory in dom0 which is also passed to our virtualisable
network interface cards.  The reason it''s allocated by the backend in
dom0 rather than using the model above is that we need to be able to
allocate two physically contiguous pages, and I this would be tricky
from domU.  If you know of a way of doing this, that would be an
excellent alternative to needing to use the xenbus_backend_client code
in the frontend.

Technically we''re not using the mapped memory as a ring buffer, but
xenbus_map_ring and friends are a convenient API to grants and mapping
them.
> Is it appropriate to be using grant
> table entries to refer to and map that memory?
I wasn''t aware of an alternative mechanism so I''ll hazard a
"yes, it is
appropriate".  However, if given the above information you think
otherwise, let me know.

Kieran


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 19/2/07 16:08, "Kieran Mansley" <kmansley@solarflare.com>
wrote:
> It''s host memory in dom0 which is also passed to our virtualisable
> network interface cards.  The reason it''s allocated by the backend
in
> dom0 rather than using the model above is that we need to be able to
> allocate two physically contiguous pages, and I this would be tricky
> from domU.  If you know of a way of doing this, that would be an
> excellent alternative to needing to use the xenbus_backend_client code
> in the frontend.

Okay, if we go this way then the functions probably need to be given more
general names (since they are not used just for descriptor rings any more)
and also will you not need them to generalise to accepting more than one
grant reference (since you want to map two grant references into a two-page
vm area)?

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Mon, 2007-02-19 at 16:50 +0000, Keir Fraser wrote:
> On 19/2/07 16:08, "Kieran Mansley"
<kmansley@solarflare.com> wrote:
> 
> > It''s host memory in dom0 which is also passed to our
virtualisable
> > network interface cards.  The reason it''s allocated by the
backend in
> > dom0 rather than using the model above is that we need to be able to
> > allocate two physically contiguous pages, and I this would be tricky
> > from domU.  If you know of a way of doing this, that would be an
> > excellent alternative to needing to use the xenbus_backend_client code
> > in the frontend.
> 
> 
> Okay, if we go this way then the functions probably need to be given more
> general names (since they are not used just for descriptor rings any more)
> and also will you not need them to generalise to accepting more than one
> grant reference (since you want to map two grant references into a two-page
> vm area)?
I''m happy to make that change.  An alternative (and much smaller)
change
would be to leave the existing map_ring API alone and augment with a
functionally similar version that could be used by the front end, and
was called something else to avoid confusion.  This would be my
preferred option I think, and would remove the need to move code out of
xenbus_backend_client. 

Accepting one grant reference is not a big deal - I can just get a grant
per page and pass all the grants, then allocate a two page vm_area map
the individual grants at the appropriate offsets into that area to get
them virtually contiguous.

Kieran


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 19/2/07 17:07, "Kieran Mansley" <kmansley@solarflare.com>
wrote:
> Accepting one grant reference is not a big deal - I can just get a grant
> per page and pass all the grants, then allocate a two page vm_area map
> the individual grants at the appropriate offsets into that area to get
> them virtually contiguous.
Then it doesn''t seem that a new helper function is very useful!
You''ll just
end up with alloc_vm_area() followed by open coding of requesting mapping of
two grant references, won''t you? A general function that did all that
would
be reasonable, and then I''d get rid of the special-case
''ring'' functions.

  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel