[PATCH 1/2] staging: comedi: fix auto-unconfig kernel error

[Ian Abbott]

On 27/12/13 23:49, Bernd Porr wrote:
> Signed-off-by: Bernd Porr <mail at berndporr.me.uk>

"Signed-off-by" should be after the description. :)

>
> Merging un-registering of both the subdevices and the
> main comedi device into one function and the module which
> actually associated with it. The kernel oops observed before
> was because the main device was un-registered first and
> then the subdevices which were then no longer valid.
> This has been also tested with 'comedi_config -r'.

It might work with 'comedi_config -r' for auto-configured devices, but I 
don't think it will work for manually configured "legacy" devices (e.g. 
a "comedi_test" device configured by 'comedi_config /dev/comedi0 
comedi_test' when the 'comedi_num_legacy_minors' module parameter is 
greater than 0). comedi_free_board_dev() wouldn't be called in that case 
when the device is unconfigured later, because the legacy comedi devices 
persist until the comedi module is unloaded, whereas the auto-configured 
comedi devices only persist while the low-level device remains attached 
(more or less - they now also persist while referenced by an unreleased 
file object).

> ---
>   drivers/staging/comedi/comedi_fops.c | 19 +++++++++++++++++++
>   drivers/staging/comedi/drivers.c     | 18 ------------------
>   2 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
> index f3d59e2..2680b72 100644
> --- a/drivers/staging/comedi/comedi_fops.c
> +++ b/drivers/staging/comedi/comedi_fops.c
> @@ -141,7 +141,26 @@ static struct comedi_device *comedi_clear_board_minor(unsigned minor)
>
>   static void comedi_free_board_dev(struct comedi_device *dev)
>   {
> +	int i;
> +	struct comedi_subdevice *s;
> +
>   	if (dev) {
> +		if (dev->subdevices) {
> +			for (i = 0; i < dev->n_subdevices; i++) {
> +				s = &dev->subdevices[i];
> +				if (s->runflags & SRF_FREE_SPRIV)
> +					kfree(s->private);
> +				comedi_free_subdevice_minor(s);
> +				if (s->async) {
> +					comedi_buf_alloc(dev, s, 0);
> +					kfree(s->async);
> +				}
> +			}
> +			kfree(dev->subdevices);
> +			dev->subdevices = NULL;
> +			dev->n_subdevices = 0;
> +		}
> +
>   		if (dev->class_dev) {
>   			device_destroy(comedi_class,
>   				       MKDEV(COMEDI_MAJOR, dev->minor));
> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
> index 4964d2a..d6dc58a 100644
> --- a/drivers/staging/comedi/drivers.c
> +++ b/drivers/staging/comedi/drivers.c
> @@ -97,24 +97,6 @@ EXPORT_SYMBOL_GPL(comedi_alloc_subdevices);
>
>   static void cleanup_device(struct comedi_device *dev)
>   {
> -	int i;
> -	struct comedi_subdevice *s;
> -
> -	if (dev->subdevices) {
> -		for (i = 0; i < dev->n_subdevices; i++) {
> -			s = &dev->subdevices[i];
> -			if (s->runflags & SRF_FREE_SPRIV)
> -				kfree(s->private);
> -			comedi_free_subdevice_minor(s);
> -			if (s->async) {
> -				comedi_buf_alloc(dev, s, 0);
> -				kfree(s->async);
> -			}
> -		}
> -		kfree(dev->subdevices);
> -		dev->subdevices = NULL;
> -		dev->n_subdevices = 0;
> -	}
>   	kfree(dev->private);
>   	dev->private = NULL;
>   	dev->driver = NULL;
>


-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-