Re: [Xen-devel] [PATCH] multi-page blkfront/blkback patch

Some comments on infrastructural things, not the actual functionality:

>+      config 1_PAGE
>+              bool "1 page"
>+      config 2_PAGE
>+              bool "2 pages"
>+      config 4_PAGE
>+              bool "4 pages"

These names are too generic. At least XEN_ (and perhaps also BLKFRONT or
some such) should also appear in them.

>-static int blkif_reqs = 64;
>+static int blkif_reqs = 128;

Is this really related to the functionality introduced here?

>+      BUG_ON(BLK_NUM_RING_PAGES != 1 && 
>+             BLK_NUM_RING_PAGES != 2 && 
>+             BLK_NUM_RING_PAGES != 4);

This should be BUILD_BUG_ON().

>+#ifndef CONFIG_XEN_BLKDEV_NUM_RING_PAGES
>+#error "CONFIG_XEN_BLKDEV_NUM_RING_PAGES undefined!"
>+#endif

This won't work when building unmodified_drivers - I'm not sure what value
to default to here, though. But for compatibility it should probably be 1,
and the unmodified drivers' Kbuild could override it if desired.

Jan


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

<Prev in Thread]

Current Thread

[Next in Thread>

Previous by Date:

RE: [Xen-devel] [PATCH] Dom0-kernel: Fix buggy mask_base insaving/restoring MSI-X table during S3, Shan, Haitao

Next by Date:

RE: [Xen-devel] [PATCH] Dom0-kernel: Fix buggy mask_base insaving/restoring MSI-X table during S3, Jan Beulich

Previous by Thread:

[Xen-devel] [PATCH] multi-page blkfront/blkback patch, Geoffrey Lefebvre

Next by Thread:

Re: [Xen-devel] [PATCH] multi-page blkfront/blkback patch, Keir Fraser

Indexes:

[Date] [Thread] [Top] [All Lists]