| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1324547820.7877.71.camel@zakaz.uk.xensource.com>
Date: Thu, 22 Dec 2011 09:57:00 +0000
From: Ian Campbell <Ian.Campbell@...rix.com>
To: Jan Beulich <JBeulich@...e.com>
CC: Konrad Rzeszutek Wilk <konrad@...nok.org>,
Jeremy Fitzhardinge <jeremy@...p.org>,
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"dmitry.torokhov@...il.com" <dmitry.torokhov@...il.com>,
"FlorianSchandinat@....de" <FlorianSchandinat@....de>,
Jens Axboe <axboe@...nel.dk>,
"xen-devel@...ts.xensource.com" <xen-devel@...ts.xensource.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH, v2] Xen: consolidate and simplify struct xenbus_driver
instantiation
On Thu, 2011-12-22 at 09:08 +0000, Jan Beulich wrote:
> The 'name', 'owner', and 'mod_name' members are redundant with the
> identically named fields in the 'driver' sub-structure. Rather than
> switching each instance to specify these fields explicitly, introduce
> a macro to simplify this.
>
> Eliminate further redundancy by allowing the drvname argument to
> DEFINE_XENBUS_DRIVER() to be blank (in which case the first entry from
> the ID table will be used for .driver.name).
Any reason not to always use DRV_NAME here (which is generally a bit
more specific e.g. "xen-foofront" rather than "foo") and rely on the id
table for the shorter names used in xenstore?
Empty parameters to macros always make me look twice, I think they
deserve at least a comment at which point you might as well just have
the actual desired string there.
Ian.
> Also eliminate the questionable xenbus_register_{back,front}end()
> wrappers - their sole remaining purpose was the checking of the
> 'owner' field, proper setting of which shouldn't be an issue anymore
> when the macro gets used.
>
> v2: Restore DRV_NAME for the driver name in xen-pciback.
>
> Signed-off-by: Jan Beulich <jbeulich@...e.com>
> Cc: Jens Axboe <axboe@...nel.dk>
> Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>
> Cc: Florian Tobias Schandinat <FlorianSchandinat@....de>
> Cc: Ian Campbell <ian.campbell@...rix.com>
> Cc: David S. Miller <davem@...emloft.net>
>
> ---
> drivers/block/xen-blkback/xenbus.c | 9 ++------
> drivers/block/xen-blkfront.c | 11 +++-------
> drivers/input/misc/xen-kbdfront.c | 7 +-----
> drivers/net/xen-netback/xenbus.c | 9 ++------
> drivers/net/xen-netfront.c | 9 ++------
> drivers/pci/xen-pcifront.c | 11 +++-------
> drivers/video/xen-fbfront.c | 9 ++------
> drivers/xen/xen-pciback/xenbus.c | 13 ++++--------
> drivers/xen/xenbus/xenbus_probe.c | 7 ------
> drivers/xen/xenbus/xenbus_probe.h | 4 ---
> drivers/xen/xenbus/xenbus_probe_backend.c | 8 ++-----
> drivers/xen/xenbus/xenbus_probe_frontend.c | 8 ++-----
> include/xen/xenbus.h | 31 ++++++++---------------------
> 13 files changed, 44 insertions(+), 92 deletions(-)
>
> --- 3.2-rc6/drivers/block/xen-blkback/xenbus.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/block/xen-blkback/xenbus.c
> @@ -787,17 +787,14 @@ static const struct xenbus_device_id xen
> };
>
>
> -static struct xenbus_driver xen_blkbk = {
> - .name = "vbd",
> - .owner = THIS_MODULE,
> - .ids = xen_blkbk_ids,
> +static DEFINE_XENBUS_DRIVER(xen_blkbk, ,
> .probe = xen_blkbk_probe,
> .remove = xen_blkbk_remove,
> .otherend_changed = frontend_changed
> -};
> +);
>
>
> int xen_blkif_xenbus_init(void)
> {
> - return xenbus_register_backend(&xen_blkbk);
> + return xenbus_register_backend(&xen_blkbk_driver);
> }
> --- 3.2-rc6/drivers/block/xen-blkfront.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/block/xen-blkfront.c
> @@ -1437,16 +1437,13 @@ static const struct xenbus_device_id blk
> { "" }
> };
>
> -static struct xenbus_driver blkfront = {
> - .name = "vbd",
> - .owner = THIS_MODULE,
> - .ids = blkfront_ids,
> +static DEFINE_XENBUS_DRIVER(blkfront, ,
> .probe = blkfront_probe,
> .remove = blkfront_remove,
> .resume = blkfront_resume,
> .otherend_changed = blkback_changed,
> .is_ready = blkfront_is_ready,
> -};
> +);
>
> static int __init xlblk_init(void)
> {
> @@ -1461,7 +1458,7 @@ static int __init xlblk_init(void)
> return -ENODEV;
> }
>
> - ret = xenbus_register_frontend(&blkfront);
> + ret = xenbus_register_frontend(&blkfront_driver);
> if (ret) {
> unregister_blkdev(XENVBD_MAJOR, DEV_NAME);
> return ret;
> @@ -1474,7 +1471,7 @@ module_init(xlblk_init);
>
> static void __exit xlblk_exit(void)
> {
> - return xenbus_unregister_driver(&blkfront);
> + return xenbus_unregister_driver(&blkfront_driver);
> }
> module_exit(xlblk_exit);
>
> --- 3.2-rc6/drivers/input/misc/xen-kbdfront.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/input/misc/xen-kbdfront.c
> @@ -361,15 +361,12 @@ static const struct xenbus_device_id xen
> { "" }
> };
>
> -static struct xenbus_driver xenkbd_driver = {
> - .name = "vkbd",
> - .owner = THIS_MODULE,
> - .ids = xenkbd_ids,
> +static DEFINE_XENBUS_DRIVER(xenkbd, ,
> .probe = xenkbd_probe,
> .remove = xenkbd_remove,
> .resume = xenkbd_resume,
> .otherend_changed = xenkbd_backend_changed,
> -};
> +);
>
> static int __init xenkbd_init(void)
> {
> --- 3.2-rc6/drivers/net/xen-netback/xenbus.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/net/xen-netback/xenbus.c
> @@ -474,17 +474,14 @@ static const struct xenbus_device_id net
> };
>
>
> -static struct xenbus_driver netback = {
> - .name = "vif",
> - .owner = THIS_MODULE,
> - .ids = netback_ids,
> +static DEFINE_XENBUS_DRIVER(netback, ,
> .probe = netback_probe,
> .remove = netback_remove,
> .uevent = netback_uevent,
> .otherend_changed = frontend_changed,
> -};
> +);
>
> int xenvif_xenbus_init(void)
> {
> - return xenbus_register_backend(&netback);
> + return xenbus_register_backend(&netback_driver);
> }
> --- 3.2-rc6/drivers/net/xen-netfront.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/net/xen-netfront.c
> @@ -1910,7 +1910,7 @@ static void xennet_sysfs_delif(struct ne
>
> #endif /* CONFIG_SYSFS */
>
> -static struct xenbus_device_id netfront_ids[] = {
> +static const struct xenbus_device_id netfront_ids[] = {
> { "vif" },
> { "" }
> };
> @@ -1937,15 +1937,12 @@ static int __devexit xennet_remove(struc
> return 0;
> }
>
> -static struct xenbus_driver netfront_driver = {
> - .name = "vif",
> - .owner = THIS_MODULE,
> - .ids = netfront_ids,
> +static DEFINE_XENBUS_DRIVER(netfront, ,
> .probe = netfront_probe,
> .remove = __devexit_p(xennet_remove),
> .resume = netfront_resume,
> .otherend_changed = netback_changed,
> -};
> +);
>
> static int __init netif_init(void)
> {
> --- 3.2-rc6/drivers/pci/xen-pcifront.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/pci/xen-pcifront.c
> @@ -1126,14 +1126,11 @@ static const struct xenbus_device_id xen
> {""},
> };
>
> -static struct xenbus_driver xenbus_pcifront_driver = {
> - .name = "pcifront",
> - .owner = THIS_MODULE,
> - .ids = xenpci_ids,
> +static DEFINE_XENBUS_DRIVER(xenpci, "pcifront",
> .probe = pcifront_xenbus_probe,
> .remove = pcifront_xenbus_remove,
> .otherend_changed = pcifront_backend_changed,
> -};
> +);
>
> static int __init pcifront_init(void)
> {
> @@ -1142,12 +1139,12 @@ static int __init pcifront_init(void)
>
> pci_frontend_registrar(1 /* enable */);
>
> - return xenbus_register_frontend(&xenbus_pcifront_driver);
> + return xenbus_register_frontend(&xenpci_driver);
> }
>
> static void __exit pcifront_cleanup(void)
> {
> - xenbus_unregister_driver(&xenbus_pcifront_driver);
> + xenbus_unregister_driver(&xenpci_driver);
> pci_frontend_registrar(0 /* disable */);
> }
> module_init(pcifront_init);
> --- 3.2-rc6/drivers/video/xen-fbfront.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/video/xen-fbfront.c
> @@ -671,20 +671,17 @@ InitWait:
> }
> }
>
> -static struct xenbus_device_id xenfb_ids[] = {
> +static const struct xenbus_device_id xenfb_ids[] = {
> { "vfb" },
> { "" }
> };
>
> -static struct xenbus_driver xenfb_driver = {
> - .name = "vfb",
> - .owner = THIS_MODULE,
> - .ids = xenfb_ids,
> +static DEFINE_XENBUS_DRIVER(xenfb, ,
> .probe = xenfb_probe,
> .remove = xenfb_remove,
> .resume = xenfb_resume,
> .otherend_changed = xenfb_backend_changed,
> -};
> +);
>
> static int __init xenfb_init(void)
> {
> --- 3.2-rc6/drivers/xen/xen-pciback/xenbus.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/xen/xen-pciback/xenbus.c
> @@ -707,19 +707,16 @@ static int xen_pcibk_xenbus_remove(struc
> return 0;
> }
>
> -static const struct xenbus_device_id xenpci_ids[] = {
> +static const struct xenbus_device_id xen_pcibk_ids[] = {
> {"pci"},
> {""},
> };
>
> -static struct xenbus_driver xenbus_xen_pcibk_driver = {
> - .name = DRV_NAME,
> - .owner = THIS_MODULE,
> - .ids = xenpci_ids,
> +static DEFINE_XENBUS_DRIVER(xen_pcibk, DRV_NAME,
> .probe = xen_pcibk_xenbus_probe,
> .remove = xen_pcibk_xenbus_remove,
> .otherend_changed = xen_pcibk_frontend_changed,
> -};
> +);
>
> const struct xen_pcibk_backend *__read_mostly xen_pcibk_backend;
>
> @@ -735,11 +732,11 @@ int __init xen_pcibk_xenbus_register(voi
> if (passthrough)
> xen_pcibk_backend = &xen_pcibk_passthrough_backend;
> pr_info(DRV_NAME ": backend is %s\n", xen_pcibk_backend->name);
> - return xenbus_register_backend(&xenbus_xen_pcibk_driver);
> + return xenbus_register_backend(&xen_pcibk_driver);
> }
>
> void __exit xen_pcibk_xenbus_unregister(void)
> {
> destroy_workqueue(xen_pcibk_wq);
> - xenbus_unregister_driver(&xenbus_xen_pcibk_driver);
> + xenbus_unregister_driver(&xen_pcibk_driver);
> }
> --- 3.2-rc6/drivers/xen/xenbus/xenbus_probe.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/xen/xenbus/xenbus_probe.c
> @@ -291,14 +291,9 @@ void xenbus_dev_shutdown(struct device *
> EXPORT_SYMBOL_GPL(xenbus_dev_shutdown);
>
> int xenbus_register_driver_common(struct xenbus_driver *drv,
> - struct xen_bus_type *bus,
> - struct module *owner,
> - const char *mod_name)
> + struct xen_bus_type *bus)
> {
> - drv->driver.name = drv->name;
> drv->driver.bus = &bus->bus;
> - drv->driver.owner = owner;
> - drv->driver.mod_name = mod_name;
>
> return driver_register(&drv->driver);
> }
> --- 3.2-rc6/drivers/xen/xenbus/xenbus_probe.h
> +++ 3.2-rc6-struct-xenbus_driver/drivers/xen/xenbus/xenbus_probe.h
> @@ -53,9 +53,7 @@ extern int xenbus_match(struct device *_
> extern int xenbus_dev_probe(struct device *_dev);
> extern int xenbus_dev_remove(struct device *_dev);
> extern int xenbus_register_driver_common(struct xenbus_driver *drv,
> - struct xen_bus_type *bus,
> - struct module *owner,
> - const char *mod_name);
> + struct xen_bus_type *bus);
> extern int xenbus_probe_node(struct xen_bus_type *bus,
> const char *type,
> const char *nodename);
> --- 3.2-rc6/drivers/xen/xenbus/xenbus_probe_backend.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/xen/xenbus/xenbus_probe_backend.c
> @@ -232,15 +232,13 @@ int xenbus_dev_is_online(struct xenbus_d
> }
> EXPORT_SYMBOL_GPL(xenbus_dev_is_online);
>
> -int __xenbus_register_backend(struct xenbus_driver *drv,
> - struct module *owner, const char *mod_name)
> +int xenbus_register_backend(struct xenbus_driver *drv)
> {
> drv->read_otherend_details = read_frontend_details;
>
> - return xenbus_register_driver_common(drv, &xenbus_backend,
> - owner, mod_name);
> + return xenbus_register_driver_common(drv, &xenbus_backend);
> }
> -EXPORT_SYMBOL_GPL(__xenbus_register_backend);
> +EXPORT_SYMBOL_GPL(xenbus_register_backend);
>
> static int backend_probe_and_watch(struct notifier_block *notifier,
> unsigned long event,
> --- 3.2-rc6/drivers/xen/xenbus/xenbus_probe_frontend.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/xen/xenbus/xenbus_probe_frontend.c
> @@ -230,15 +230,13 @@ static void wait_for_devices(struct xenb
> print_device_status);
> }
>
> -int __xenbus_register_frontend(struct xenbus_driver *drv,
> - struct module *owner, const char *mod_name)
> +int xenbus_register_frontend(struct xenbus_driver *drv)
> {
> int ret;
>
> drv->read_otherend_details = read_backend_details;
>
> - ret = xenbus_register_driver_common(drv, &xenbus_frontend,
> - owner, mod_name);
> + ret = xenbus_register_driver_common(drv, &xenbus_frontend);
> if (ret)
> return ret;
>
> @@ -247,7 +245,7 @@ int __xenbus_register_frontend(struct xe
>
> return 0;
> }
> -EXPORT_SYMBOL_GPL(__xenbus_register_frontend);
> +EXPORT_SYMBOL_GPL(xenbus_register_frontend);
>
> static DECLARE_WAIT_QUEUE_HEAD(backend_state_wq);
> static int backend_state;
> --- 3.2-rc6/include/xen/xenbus.h
> +++ 3.2-rc6-struct-xenbus_driver/include/xen/xenbus.h
> @@ -85,8 +85,6 @@ struct xenbus_device_id
>
> /* A xenbus driver. */
> struct xenbus_driver {
> - char *name;
> - struct module *owner;
> const struct xenbus_device_id *ids;
> int (*probe)(struct xenbus_device *dev,
> const struct xenbus_device_id *id);
> @@ -101,31 +99,20 @@ struct xenbus_driver {
> int (*is_ready)(struct xenbus_device *dev);
> };
>
> -static inline struct xenbus_driver *to_xenbus_driver(struct device_driver *drv)
> -{
> - return container_of(drv, struct xenbus_driver, driver);
> +#define DEFINE_XENBUS_DRIVER(var, drvname, methods...) \
> +struct xenbus_driver var ## _driver = { \
> + .driver.name = drvname + 0 ?: var ## _ids->devicetype, \
> + .driver.owner = THIS_MODULE, \
> + .ids = var ## _ids, ## methods \
> }
>
> -int __must_check __xenbus_register_frontend(struct xenbus_driver *drv,
> - struct module *owner,
> - const char *mod_name);
> -
> -static inline int __must_check
> -xenbus_register_frontend(struct xenbus_driver *drv)
> +static inline struct xenbus_driver *to_xenbus_driver(struct device_driver *drv)
> {
> - WARN_ON(drv->owner != THIS_MODULE);
> - return __xenbus_register_frontend(drv, THIS_MODULE, KBUILD_MODNAME);
> + return container_of(drv, struct xenbus_driver, driver);
> }
>
> -int __must_check __xenbus_register_backend(struct xenbus_driver *drv,
> - struct module *owner,
> - const char *mod_name);
> -static inline int __must_check
> -xenbus_register_backend(struct xenbus_driver *drv)
> -{
> - WARN_ON(drv->owner != THIS_MODULE);
> - return __xenbus_register_backend(drv, THIS_MODULE, KBUILD_MODNAME);
> -}
> +int __must_check xenbus_register_frontend(struct xenbus_driver *);
> +int __must_check xenbus_register_backend(struct xenbus_driver *);
>
> void xenbus_unregister_driver(struct xenbus_driver *drv);
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists