Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-15 Thread Jan Beulich
>>> On 15.03.12 at 09:51, Ian Campbell wrote: > On Thu, 2012-03-15 at 08:03 +, Jan Beulich wrote: >> >>> On 14.03.12 at 18:01, Justin Gibbs wrote: >> > While we're talking about fixing ring data structures, can RING_IDX >> > be defined as a "uint32_t" instead of "unsigned int". The structure

Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-15 Thread Ian Campbell
On Thu, 2012-03-15 at 08:03 +, Jan Beulich wrote: > >>> On 14.03.12 at 18:01, Justin Gibbs wrote: > > While we're talking about fixing ring data structures, can RING_IDX > > be defined as a "uint32_t" instead of "unsigned int". The structure > > padding in the ring macros assumes RING_IDX is

Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-15 Thread Jan Beulich
>>> On 14.03.12 at 18:17, "Justin T. Gibbs" wrote: > On Mar 6, 2012, at 1:34 AM, Jan Beulich wrote: > > On 05.03.12 at 22:49, Santosh Jodh wrote: > > … > >>> + } >>> + >>>/* Create shared ring, alloc event channel. */ >>>err = setup_blkring(dev, info); >>>if (

Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-15 Thread Jan Beulich
>>> On 14.03.12 at 18:01, Justin Gibbs wrote: > While we're talking about fixing ring data structures, can RING_IDX > be defined as a "uint32_t" instead of "unsigned int". The structure > padding in the ring macros assumes RING_IDX is exactly 4 bytes, > so this should be made explicit. ILP64 mac

Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-14 Thread Justin T. Gibbs
On Mar 6, 2012, at 1:34 AM, Jan Beulich wrote: On 05.03.12 at 22:49, Santosh Jodh wrote: … >> + } >> + >>/* Create shared ring, alloc event channel. */ >>err = setup_blkring(dev, info); >>if (err) >> @@ -889,12 +916,35 @@ again: >>goto destroy_

Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-14 Thread Justin Gibbs
On Mar 14, 2012, at 9:34 AM, Jan Beulich wrote: On 14.03.12 at 07:32, Justin Gibbs wrote: >> There's another problem here that I brought up during the Xen >> Hack-a-thon. The ring macros require that the ring element count >> be a power of two. This doesn't mean that the ring will be a pow

Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-14 Thread Justin Gibbs
On Mar 7, 2012, at 2:33 AM, Jan Beulich wrote: On 06.03.12 at 18:20, Konrad Rzeszutek Wilk wrote: >> -> XENBUS_MAX_RING_PAGES - why 2? Why not 4? What is the optimal >> default size for SSD usage? 16? > > What do SSDs have to do with a XenBus definition? Imo it's wrong (and > unnecessary)

Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-14 Thread Wei Liu
On Mon, 2012-03-05 at 21:49 +, Santosh Jodh wrote: > From: Santosh Jodh > > Add support for multi page ring for block devices. > The number of pages is configurable for blkback via module parameter. > blkback reports max-ring-page-order to blkfront via xenstore. > blkfront reports its support

RE: [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-14 Thread Santosh Jodh
Great feedback. I removed unsigned for the first, changed the error code and added module param name in the printk. Please see latest patch: --- diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c index 0088bf6..cc238e7 100644 --- a/drivers/block/xen-blkback/b

[PATCH 0001/001] xen: multi page ring support for block devices

2012-03-14 Thread Santosh Jodh
From: Santosh Jodh Add support for multi page ring for block devices. The number of pages is configurable for blkback via module parameter. blkback reports max-ring-page-order to blkfront via xenstore. blkfront reports its supported ring-page-order to blkback via xenstore. blkfront reports multi

Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-14 Thread Jan Beulich
>>> On 14.03.12 at 07:32, Justin Gibbs wrote: > There's another problem here that I brought up during the Xen > Hack-a-thon. The ring macros require that the ring element count > be a power of two. This doesn't mean that the ring will be a power > of 2 pages in size. To illustrate this point, I

Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-14 Thread Jan Beulich
>>> On 14.03.12 at 07:32, Justin Gibbs wrote: > There's another problem here that I brought up during the Xen > Hack-a-thon. The ring macros require that the ring element count > be a power of two. This doesn't mean that the ring will be a power > of 2 pages in size. To illustrate this point, I

Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-07 Thread Konrad Rzeszutek Wilk
On Mar 7, 2012 4:33 AM, "Jan Beulich" wrote: > > >>> On 06.03.12 at 18:20, Konrad Rzeszutek Wilk wrote: > > -> the usage of XenbusStateInitWait? Why do we introduce that? Looks > > like a fix to something. > > No, this is required to get the negotiation working (the frontend must > not try to re

Re: [Xen-devel] [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-07 Thread Jan Beulich
>>> On 06.03.12 at 18:20, Konrad Rzeszutek Wilk wrote: > -> the usage of XenbusStateInitWait? Why do we introduce that? Looks > like a fix to something. No, this is required to get the negotiation working (the frontend must not try to read the new nodes until it can be certain that the backend p

Re: [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-06 Thread Jan Beulich
>>> On 05.03.12 at 22:49, Santosh Jodh wrote: Could this be split up into 3 patches, for easier reviewing: - one adjusting the xenbus interface to allow for multiple ring pages (and maybe even that one should be split into the backend and frontend related parts), syncing with the similar netb

Re: [PATCH 0001/001] xen: multi page ring support for block devices

2012-03-05 Thread Rusty Russell
On Mon, 5 Mar 2012 13:49:07 -0800, Santosh Jodh wrote: > +/* Order of maximum shared ring size advertised to the front end. */ > +int xen_blkif_max_ring_order = XENBUS_MAX_RING_ORDER; > + > +#define BLK_RING_SIZE(_order) __CONST_RING_SIZE(blkif, PAGE_SIZE << (_order)) > + > +static int set_max_rin