From mboxrd@z Thu Jan 1 00:00:00 1970 From: Harry Butterworth Subject: Re: [PATCH] skeleton frontend/backend examples and a deadlock Date: Wed, 02 Nov 2005 12:37:46 +0000 Message-ID: <1130935066.4719.22.camel@localhost> References: <1130908964.18258.25.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-ME7VBe6s3Jqks0w9aHsx" Return-path: In-Reply-To: <1130908964.18258.25.camel@localhost.localdomain> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Rusty Russell Cc: Xen Mailing List , mark.williamson@cl.cam.ac.uk List-Id: xen-devel@lists.xenproject.org --=-ME7VBe6s3Jqks0w9aHsx Content-Type: text/plain Content-Transfer-Encoding: 7bit On Wed, 2005-11-02 at 16:22 +1100, Rusty Russell wrote: > Here are example frontend and backend driver skeletons. They're > *designed* to handle driver restart and module unloading. However, > device_unregister deadlocks. I guess noone tried testing > unregister_xenbus_watch when not called from a watch callback. Thanks, that'll be useful for me to know when it comes to testing my code. For comparison, I've knocked up a skeleton FE/BE driver based on the xenidc API that I'm using for my USB driver. You'll see that my patch is smaller, simpler and provides significantly more functionality: enough to send messages and transactions bi-directionally between the FE and BE. For a fair comparison, yours would require interrupt handlers, use of the RING macros, correct memory barriers etc. Hopefully this helps to explain why I've split out the IDC functionality from my USB driver into a generic service. Harry. --=-ME7VBe6s3Jqks0w9aHsx Content-Disposition: attachment; filename=xenidc-skeleton-patch Content-Type: text/x-patch; name=xenidc-skeleton-patch; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit diff -r e376b14ac870 -r 4cc0fb2ce859 linux-2.6-xen-sparse/arch/xen/Kconfig --- a/linux-2.6-xen-sparse/arch/xen/Kconfig Wed Nov 2 10:25:12 2005 +++ b/linux-2.6-xen-sparse/arch/xen/Kconfig Wed Nov 2 12:01:27 2005 @@ -210,6 +210,22 @@ If security is not a concern then you may increase performance by saying N. +config XEN_SKELETON_FE + tristate "Compile Xen skeleton example frontend driver code" + default m + help + There is an example skeleton driver frontend in drivers/xen/skeleton + which you can use as a basis for your own xenbus-aware + drivers. + +config XEN_SKELETON_BE + tristate "Compile Xen skeleton example backend driver code" + default m + help + There is an example skeleton driver backend in drivers/xen/skeleton + which you can use as a basis for your own xenbus-aware + drivers. + choice prompt "Processor Type" default XEN_X86 diff -r e376b14ac870 -r 4cc0fb2ce859 linux-2.6-xen-sparse/drivers/xen/Makefile --- a/linux-2.6-xen-sparse/drivers/xen/Makefile Wed Nov 2 10:25:12 2005 +++ b/linux-2.6-xen-sparse/drivers/xen/Makefile Wed Nov 2 12:01:27 2005 @@ -6,6 +6,7 @@ obj-y += privcmd/ obj-y += xenbus/ obj-y += xenidc/ +obj-y += skeleton/ obj-$(CONFIG_XEN_BLKDEV_BACKEND) += blkback/ obj-$(CONFIG_XEN_NETDEV_BACKEND) += netback/ diff -r e376b14ac870 -r 4cc0fb2ce859 linux-2.6-xen-sparse/drivers/xen/skeleton/Makefile --- /dev/null Wed Nov 2 10:25:12 2005 +++ b/linux-2.6-xen-sparse/drivers/xen/skeleton/Makefile Wed Nov 2 12:01:27 2005 @@ -0,0 +1,2 @@ +obj-$(CONFIG_XEN_SKELETON_FE) += skeleton_fe.o +obj-$(CONFIG_XEN_SKELETON_BE) += skeleton_be.o diff -r e376b14ac870 -r 4cc0fb2ce859 linux-2.6-xen-sparse/drivers/xen/skeleton/skeleton_be.c --- /dev/null Wed Nov 2 10:25:12 2005 +++ b/linux-2.6-xen-sparse/drivers/xen/skeleton/skeleton_be.c Wed Nov 2 12:01:27 2005 @@ -0,0 +1,299 @@ +/*****************************************************************************/ +/* Copyright (c) 2005 Harry Butterworth IBM Corporation */ +/* */ +/* This program is free software; you can redistribute it and/or modify it */ +/* under the terms of the GNU General Public License as published by the */ +/* Free Software Foundation; either version 2 of the License, or (at your */ +/* option) any later version. */ +/* */ +/* This program is distributed in the hope that it will be useful, but */ +/* WITHOUT ANY WARRANTY; without even the implied warranty of */ +/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General */ +/* Public License for more details. */ +/* */ +/* You should have received a copy of the GNU General Public License along */ +/* with this program; if not, write to the Free Software Foundation, Inc., */ +/* 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +/* */ +/*****************************************************************************/ + +#include +#include +#include + +/* Skeleton driver doesn't actually send any messages or transactions. */ + +#define SKELETON_BE_INITIATOR_QUOTA 0 +#define SKELETON_BE_INITIATOR_MAXIMUM_BYTE_COUNT 0 +#define SKELETON_BE_TARGET_QUOTA 0 +#define SKELETON_BE_TARGET_MAXIMUM_BYTE_COUNT 0 + +struct skeleton_be_device +{ + struct xenbus_device * dev; + xenidc_address address; + long int frontend_id; + char * frontend; + xenidc_endpoint endpoint; + xenidc_callback * endpoint_disconnect_callback; +}; + +static inline struct skeleton_be_device * skeleton_be_device_endpoint_to + ( xenidc_endpoint * endpoint ) +{ + return container_of( endpoint, struct skeleton_be_device, endpoint ); +} + +static void skeleton_be_device_endpoint_connect( xenidc_endpoint * endpoint ) +{ + /* Between connect and completion of the disconnect callback we are */ + /* allowed to issue messages and transactions. */ +} + +static void skeleton_be_device_endpoint_disconnect + ( xenidc_endpoint * endpoint, xenidc_callback * callback ) +{ + /* We must stop issuing messages and transactions and complete the */ + /* callback once all of the messages and transactions we are issuing */ + /* have completed or failed back to us. */ + + /* The skeleton driver doesn't issue any messages or transactions so can */ + /* complete the callback immediately. */ + + xenidc_callback_success( callback ); +} + +static void skeleton_be_device_endpoint_message + ( xenidc_endpoint * endpoint, xenidc_endpoint_message * message ) +{ + /* Handle inbound message. */ + + xenidc_callback_success( xenidc_endpoint_message_to_callback( message ) ); +} + +static void skeleton_be_device_endpoint_transaction + ( xenidc_endpoint * endpoint, xenidc_endpoint_transaction * transaction ) +{ + /* Handle inbound transaction. */ + + xenidc_callback_success + ( xenidc_endpoint_transaction_to_callback( transaction ) ); +} + +static int skeleton_be_device_suspend( struct xenbus_device * dev ) +{ + struct skeleton_be_device * device = dev->data; + + xenidc_endpoint_destroy( &device->endpoint ); + + kfree( device->frontend ); + + return 0; +} + +static int skeleton_be_device_resume( struct xenbus_device * dev ) +{ + struct skeleton_be_device * device = dev->data; + + int return_value = xenbus_gather + ( + NULL, /* FIXME? */ + dev->nodename, + "frontend-id", "%li", &device->frontend_id, + "frontend", NULL, &device->frontend, + NULL + ); + + if( XENBUS_EXIST_ERR( return_value ) ) + { + goto EXIT_NO_FRONTEND; + } + + if( return_value < 0 ) + { + xenbus_dev_error + ( + dev, + return_value, + "reading %s/frontend", + dev->nodename + ); + + goto EXIT_NO_FRONTEND; + } + + xenidc_address_init + ( + &device->address, + dev->nodename, + device->frontend, + device->frontend_id + ); + + xenidc_endpoint_create( &device->endpoint, device->address ); + + return 0; + + EXIT_NO_FRONTEND: + + return return_value; +} + +static int skeleton_be_device_init_or_exit + ( struct skeleton_be_device * device, struct xenbus_device * dev, int exit ) +{ + int return_value = 0; + + if( exit ) + { + goto EXIT; + } + + memset( device, 0, sizeof( *device ) ); + + device->dev = dev; + + return_value = xenidc_endpoint_init + ( + &device->endpoint, + skeleton_be_device_endpoint_connect, + skeleton_be_device_endpoint_message, + skeleton_be_device_endpoint_transaction, + skeleton_be_device_endpoint_disconnect, + SKELETON_BE_INITIATOR_QUOTA, + SKELETON_BE_INITIATOR_MAXIMUM_BYTE_COUNT, + SKELETON_BE_TARGET_QUOTA, + SKELETON_BE_TARGET_MAXIMUM_BYTE_COUNT + ); + + if( return_value != 0 ) + { + goto EXIT_NO_ENDPOINT; + } + + return_value = skeleton_be_device_resume( dev ); + + if( return_value != 0 ) + { + goto EXIT_NO_RESUME; + } + + return 0; + + EXIT: + + skeleton_be_device_suspend( device->dev ); + + EXIT_NO_RESUME: + + xenidc_endpoint_exit( &device->endpoint ); + + EXIT_NO_ENDPOINT: + + return return_value; +} + +static int skeleton_be_device_init + ( struct skeleton_be_device * device, struct xenbus_device * dev ) +{ + return skeleton_be_device_init_or_exit( device, dev, 0 ); +} + +static void skeleton_be_device_exit( struct skeleton_be_device * device ) +{ + (void)skeleton_be_device_init_or_exit( device, NULL, 1 ); +} + +static int skeleton_be_device_probe_or_remove + ( struct xenbus_device * dev, int remove ) +{ + int return_value = 0; + + struct skeleton_be_device * device; + + if( remove ) + { + goto REMOVE; + } + + device = kmalloc( sizeof( *device ), GFP_KERNEL ); + + if( device == NULL ) + { + xenbus_dev_error( dev, -ENOMEM, "allocating backend structure" ); + + return_value = -ENOMEM; + + goto EXIT_NO_DEVICE; + } + + dev->data = device; + + return_value = skeleton_be_device_init( device, dev ); + + if( return_value != 0 ) + { + goto EXIT_INIT_FAILED; + } + + return 0; + + REMOVE: + + device = dev->data; + + skeleton_be_device_exit( device ); + + EXIT_INIT_FAILED: + + dev->data = NULL; + + kfree( device ); + + EXIT_NO_DEVICE: + + return return_value; +} + +static int skeleton_be_device_probe + ( struct xenbus_device * dev, const struct xenbus_device_id * id ) +{ + return skeleton_be_device_probe_or_remove( dev, 0 ); +} + +static int skeleton_be_device_remove( struct xenbus_device * dev ) +{ + return skeleton_be_device_probe_or_remove( dev, 1 ); +} + +static struct xenbus_device_id skeleton_be_device_ids[] = +{ + { "skeleton" }, + { "" } +}; + +static struct xenbus_driver skeleton_be_device_driver = +{ + .name = "skeleton_be", + .owner = THIS_MODULE, + .ids = skeleton_be_device_ids, + .probe = skeleton_be_device_probe, + .remove = skeleton_be_device_remove, + .suspend = skeleton_be_device_suspend, + .resume = skeleton_be_device_resume, +}; + +static int __init skeleton_be_module_init( void ) +{ + return xenbus_register_driver( &skeleton_be_device_driver ); +} + +static void __exit skeleton_be_module_exit( void ) +{ + xenbus_unregister_driver( &skeleton_be_device_driver ); +} + +module_init( skeleton_be_module_init ); +module_exit( skeleton_be_module_exit ); + +MODULE_LICENSE( "GPL" ); diff -r e376b14ac870 -r 4cc0fb2ce859 linux-2.6-xen-sparse/drivers/xen/skeleton/skeleton_fe.c --- /dev/null Wed Nov 2 10:25:12 2005 +++ b/linux-2.6-xen-sparse/drivers/xen/skeleton/skeleton_fe.c Wed Nov 2 12:01:27 2005 @@ -0,0 +1,299 @@ +/*****************************************************************************/ +/* Copyright (c) 2005 Harry Butterworth IBM Corporation */ +/* */ +/* This program is free software; you can redistribute it and/or modify it */ +/* under the terms of the GNU General Public License as published by the */ +/* Free Software Foundation; either version 2 of the License, or (at your */ +/* option) any later version. */ +/* */ +/* This program is distributed in the hope that it will be useful, but */ +/* WITHOUT ANY WARRANTY; without even the implied warranty of */ +/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General */ +/* Public License for more details. */ +/* */ +/* You should have received a copy of the GNU General Public License along */ +/* with this program; if not, write to the Free Software Foundation, Inc., */ +/* 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +/* */ +/*****************************************************************************/ + +#include +#include +#include + +/* Skeleton driver doesn't actually send any messages or transactions. */ + +#define SKELETON_FE_INITIATOR_QUOTA 0 +#define SKELETON_FE_INITIATOR_MAXIMUM_BYTE_COUNT 0 +#define SKELETON_FE_TARGET_QUOTA 0 +#define SKELETON_FE_TARGET_MAXIMUM_BYTE_COUNT 0 + +struct skeleton_fe_device +{ + struct xenbus_device * dev; + xenidc_address address; + long int backend_id; + char * backend; + xenidc_endpoint endpoint; + xenidc_callback * endpoint_disconnect_callback; +}; + +static inline struct skeleton_fe_device * skeleton_fe_device_endpoint_to + ( xenidc_endpoint * endpoint ) +{ + return container_of( endpoint, struct skeleton_fe_device, endpoint ); +} + +static void skeleton_fe_device_endpoint_connect( xenidc_endpoint * endpoint ) +{ + /* Between connect and completion of the disconnect callback we are */ + /* allowed to issue messages and transactions. */ +} + +static void skeleton_fe_device_endpoint_disconnect + ( xenidc_endpoint * endpoint, xenidc_callback * callback ) +{ + /* We must stop issuing messages and transactions and complete the */ + /* callback once all of the messages and transactions we are issuing */ + /* have completed or failed back to us. */ + + /* The skeleton driver doesn't issue any messages or transactions so can */ + /* complete the callback immediately. */ + + xenidc_callback_success( callback ); +} + +static void skeleton_fe_device_endpoint_message + ( xenidc_endpoint * endpoint, xenidc_endpoint_message * message ) +{ + /* Handle inbound message. */ + + xenidc_callback_success( xenidc_endpoint_message_to_callback( message ) ); +} + +static void skeleton_fe_device_endpoint_transaction + ( xenidc_endpoint * endpoint, xenidc_endpoint_transaction * transaction ) +{ + /* Handle inbound transaction. */ + + xenidc_callback_success + ( xenidc_endpoint_transaction_to_callback( transaction ) ); +} + +static int skeleton_fe_device_suspend( struct xenbus_device * dev ) +{ + struct skeleton_fe_device * device = dev->data; + + xenidc_endpoint_destroy( &device->endpoint ); + + kfree( device->backend ); + + return 0; +} + +static int skeleton_fe_device_resume( struct xenbus_device * dev ) +{ + struct skeleton_fe_device * device = dev->data; + + int return_value = xenbus_gather + ( + NULL, /* FIXME? */ + dev->nodename, + "backend-id", "%li", &device->backend_id, + "backend", NULL, &device->backend, + NULL + ); + + if( XENBUS_EXIST_ERR( return_value ) ) + { + goto EXIT_NO_BACKEND; + } + + if( return_value < 0 ) + { + xenbus_dev_error + ( + dev, + return_value, + "reading %s/backend", + dev->nodename + ); + + goto EXIT_NO_BACKEND; + } + + xenidc_address_init + ( + &device->address, + dev->nodename, + device->backend, + device->backend_id + ); + + xenidc_endpoint_create( &device->endpoint, device->address ); + + return 0; + + EXIT_NO_BACKEND: + + return return_value; +} + +static int skeleton_fe_device_init_or_exit + ( struct skeleton_fe_device * device, struct xenbus_device * dev, int exit ) +{ + int return_value = 0; + + if( exit ) + { + goto EXIT; + } + + memset( device, 0, sizeof( *device ) ); + + device->dev = dev; + + return_value = xenidc_endpoint_init + ( + &device->endpoint, + skeleton_fe_device_endpoint_connect, + skeleton_fe_device_endpoint_message, + skeleton_fe_device_endpoint_transaction, + skeleton_fe_device_endpoint_disconnect, + SKELETON_FE_INITIATOR_QUOTA, + SKELETON_FE_INITIATOR_MAXIMUM_BYTE_COUNT, + SKELETON_FE_TARGET_QUOTA, + SKELETON_FE_TARGET_MAXIMUM_BYTE_COUNT + ); + + if( return_value != 0 ) + { + goto EXIT_NO_ENDPOINT; + } + + return_value = skeleton_fe_device_resume( dev ); + + if( return_value != 0 ) + { + goto EXIT_NO_RESUME; + } + + return 0; + + EXIT: + + skeleton_fe_device_suspend( device->dev ); + + EXIT_NO_RESUME: + + xenidc_endpoint_exit( &device->endpoint ); + + EXIT_NO_ENDPOINT: + + return return_value; +} + +static int skeleton_fe_device_init + ( struct skeleton_fe_device * device, struct xenbus_device * dev ) +{ + return skeleton_fe_device_init_or_exit( device, dev, 0 ); +} + +static void skeleton_fe_device_exit( struct skeleton_fe_device * device ) +{ + (void)skeleton_fe_device_init_or_exit( device, NULL, 1 ); +} + +static int skeleton_fe_device_probe_or_remove + ( struct xenbus_device * dev, int remove ) +{ + int return_value = 0; + + struct skeleton_fe_device * device; + + if( remove ) + { + goto REMOVE; + } + + device = kmalloc( sizeof( *device ), GFP_KERNEL ); + + if( device == NULL ) + { + xenbus_dev_error( dev, -ENOMEM, "allocating frontend structure" ); + + return_value = -ENOMEM; + + goto EXIT_NO_DEVICE; + } + + dev->data = device; + + return_value = skeleton_fe_device_init( device, dev ); + + if( return_value != 0 ) + { + goto EXIT_INIT_FAILED; + } + + return 0; + + REMOVE: + + device = dev->data; + + skeleton_fe_device_exit( device ); + + EXIT_INIT_FAILED: + + dev->data = NULL; + + kfree( device ); + + EXIT_NO_DEVICE: + + return return_value; +} + +static int skeleton_fe_device_probe + ( struct xenbus_device * dev, const struct xenbus_device_id * id ) +{ + return skeleton_fe_device_probe_or_remove( dev, 0 ); +} + +static int skeleton_fe_device_remove( struct xenbus_device * dev ) +{ + return skeleton_fe_device_probe_or_remove( dev, 1 ); +} + +static struct xenbus_device_id skeleton_fe_device_ids[] = +{ + { "skeleton" }, + { "" } +}; + +static struct xenbus_driver skeleton_fe_device_driver = +{ + .name = "skeleton_fe", + .owner = THIS_MODULE, + .ids = skeleton_fe_device_ids, + .probe = skeleton_fe_device_probe, + .remove = skeleton_fe_device_remove, + .suspend = skeleton_fe_device_suspend, + .resume = skeleton_fe_device_resume, +}; + +static int __init skeleton_fe_module_init( void ) +{ + return xenbus_register_driver( &skeleton_fe_device_driver ); +} + +static void __exit skeleton_fe_module_exit( void ) +{ + xenbus_unregister_driver( &skeleton_fe_device_driver ); +} + +module_init( skeleton_fe_module_init ); +module_exit( skeleton_fe_module_exit ); + +MODULE_LICENSE( "GPL" ); --=-ME7VBe6s3Jqks0w9aHsx Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --=-ME7VBe6s3Jqks0w9aHsx--