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

[Ian Abbott]

On 2013-12-28 21:31, Bernd Porr wrote:
> Merging the 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' for
> both autoconfigured and legacy devices.
>
> Signed-off-by: Bernd Porr <mail at berndporr.me.uk>
> ---
>   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;
>

This doesn't apply to linux-next any more.  (For example, 
cleanup_device() function was renamed amongst other stuff.)  I've also 
fixed a load of stuff related to this bug since this patch was 
applicable, so possibly the bug has been squashed already.

Bernd, can you reproduce the problem on the latest tagged linux-next branch?

Also, the removal of all that code from cleanup_device() (since renamed 
to comedi_device_detach_cleanup()) would stop the COMEDI_DEVCONFIG ioctl 
with NULL argument (e.g. via the 'comedi_config -r' command to detach a 
device) cleaning up after the detachment, especially for the 
preallocated "legacy" comedi devices that hang around after the 
detachment, ready to be reattached via another instance of the ioctl call.

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