Ian Abbott wrote:
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 <m...@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?
I can. Still the same problem as before.

Linux bp1-Dimension-9100 3.13.0-rc7-next-20140106 #1 SMP Tue Jan 7 06:36:37 GMT 2014 x86_64 x86_64 x86_64 GNU/Linux

[   74.183729] ------------[ cut here ]------------
[ 74.183741] WARNING: CPU: 0 PID: 28 at fs/sysfs/group.c:216 sysfs_remove_group+0x93/0xa0() [ 74.183744] sysfs group ffffffff81caa9e0 not found for kobject 'comedi0_subd0' [ 74.183747] Modules linked in: radeon bnep rfcomm bluetooth snd_hda_codec_idt snd_hda_codec_generic snd_hda_intel 6lowpan_iphc(P) snd_hda_codec rc_hauppauge ir_kbd_i2c ttm tuner drm_kms_helper msp3400 snd_hwdep snd_bt87x drm snd_pcm bttv hid_generic gpio_ich dcdbas microcode usbduxsigma(C) snd_page_alloc comedi_fc(C) comedi(C) usbhid btcx_risc snd_seq_midi tveeprom videobuf_dma_sg snd_seq_midi_event snd_rawmidi rc_core psmouse snd_seq v4l2_common pcmcia pcmcia_core hid snd_seq_device videobuf_core snd_timer serio_raw videodev snd lpc_ich i2c_algo_bit soundcore ahci e100 libahci mii [ 74.183798] CPU: 0 PID: 28 Comm: khubd Tainted: P WC 3.13.0-rc7-next-20140106 #1 [ 74.183801] Hardware name: Dell Inc. Dimension 9100 /0X8582, BIOS A01 05/25/2005 [ 74.183804] 0000000000000009 ffff88005cfb1a30 ffffffff8172cf2d ffff88005cfb1a78 [ 74.183809] ffff88005cfb1a68 ffffffff81066cbd 0000000000000000 ffffffff81caa9e0 [ 74.183813] ffff880058562010 ffff880058413400 0000000000000000 ffff88005cfb1ac8
[   74.183817] Call Trace:
[   74.183827]  [<ffffffff8172cf2d>] dump_stack+0x45/0x56
[   74.183832]  [<ffffffff81066cbd>] warn_slowpath_common+0x7d/0xa0
[   74.183836]  [<ffffffff81066d2c>] warn_slowpath_fmt+0x4c/0x50
[   74.183841]  [<ffffffff81236b98>] ? kernfs_find_and_get_ns+0x48/0x60
[   74.183845]  [<ffffffff812358e3>] sysfs_remove_group+0x93/0xa0
[   74.183850]  [<ffffffff814afc53>] dpm_sysfs_remove+0x43/0x50
[   74.183854]  [<ffffffff814a5635>] device_del+0x45/0x1c0
[   74.183857]  [<ffffffff814a57ce>] device_unregister+0x1e/0x60
[   74.183861]  [<ffffffff814a588c>] device_destroy+0x3c/0x50
[ 74.183873] [<ffffffffa016d0e5>] comedi_free_subdevice_minor+0x75/0xa0 [comedi] [ 74.183882] [<ffffffffa016db88>] comedi_device_detach+0x68/0x180 [comedi] [ 74.183889] [<ffffffffa0169238>] comedi_device_cleanup+0x38/0x80 [comedi] [ 74.183896] [<ffffffffa016acd6>] comedi_free_board_dev+0x36/0x60 [comedi] [ 74.183903] [<ffffffffa016cf70>] comedi_release_hardware_device+0x80/0x90 [comedi]
[   74.183911]  [<ffffffffa016d653>] comedi_auto_unconfig+0x13/0x20 [comedi]
[ 74.183920] [<ffffffffa016ee92>] comedi_usb_auto_unconfig+0x12/0x20 [comedi]
[   74.183925]  [<ffffffff8155d854>] usb_unbind_interface+0x64/0x1c0
[   74.183931]  [<ffffffff814a931f>] __device_release_driver+0x7f/0xf0
[   74.183934]  [<ffffffff814a93b3>] device_release_driver+0x23/0x30
[   74.183938]  [<ffffffff814a8c38>] bus_remove_device+0x108/0x180
[   74.183941]  [<ffffffff814a5719>] device_del+0x129/0x1c0
[   74.183945]  [<ffffffff8155b220>] usb_disable_device+0xb0/0x290
[   74.183951]  [<ffffffff8154f8bd>] usb_disconnect+0xad/0x200
[   74.183955]  [<ffffffff81552c3d>] hub_thread+0x70d/0x1750
[   74.183961]  [<ffffffff8109e0a8>] ? sched_clock_cpu+0xa8/0x100
[   74.183966]  [<ffffffff810ac5d0>] ? prepare_to_wait_event+0x100/0x100
[   74.183970]  [<ffffffff81552530>] ? usb_reset_device+0x1d0/0x1d0
[   74.183975]  [<ffffffff8108af22>] kthread+0xd2/0xf0
[   74.183979]  [<ffffffff8108ae50>] ? kthread_create_on_node+0x190/0x190
[   74.183984]  [<ffffffff8173e1fc>] ret_from_fork+0x7c/0xb0
[   74.183988]  [<ffffffff8108ae50>] ? kthread_create_on_node+0x190/0x190
[   74.183990] ---[ end trace d84882d84ae81169 ]---
[   74.188631] ------------[ cut here ]------------
[ 74.188644] WARNING: CPU: 0 PID: 28 at fs/sysfs/group.c:216 sysfs_remove_group+0x93/0xa0() [ 74.188647] sysfs group ffffffff81caa9e0 not found for kobject 'comedi0_subd1' [ 74.188649] Modules linked in: radeon bnep rfcomm bluetooth snd_hda_codec_idt snd_hda_codec_generic snd_hda_intel 6lowpan_iphc(P) snd_hda_codec rc_hauppauge ir_kbd_i2c ttm tuner drm_kms_helper msp3400 snd_hwdep snd_bt87x drm snd_pcm bttv hid_generic gpio_ich dcdbas microcode usbduxsigma(C) snd_page_alloc comedi_fc(C) comedi(C) usbhid btcx_risc snd_seq_midi tveeprom videobuf_dma_sg snd_seq_midi_event snd_rawmidi rc_core psmouse snd_seq v4l2_common pcmcia pcmcia_core hid snd_seq_device videobuf_core snd_timer serio_raw videodev snd lpc_ich i2c_algo_bit soundcore ahci e100 libahci mii [ 74.188700] CPU: 0 PID: 28 Comm: khubd Tainted: P WC 3.13.0-rc7-next-20140106 #1 [ 74.188703] Hardware name: Dell Inc. Dimension 9100 /0X8582, BIOS A01 05/25/2005 [ 74.188706] 0000000000000009 ffff88005cfb1a30 ffffffff8172cf2d ffff88005cfb1a78 [ 74.188712] ffff88005cfb1a68 ffffffff81066cbd 0000000000000000 ffffffff81caa9e0 [ 74.188717] ffff880058562410 ffff880058413400 0000000000000000 ffff88005cfb1ac8
[   74.188721] Call Trace:
[   74.188731]  [<ffffffff8172cf2d>] dump_stack+0x45/0x56
[   74.188737]  [<ffffffff81066cbd>] warn_slowpath_common+0x7d/0xa0
[   74.188740]  [<ffffffff81066d2c>] warn_slowpath_fmt+0x4c/0x50
[   74.188745]  [<ffffffff81236b98>] ? kernfs_find_and_get_ns+0x48/0x60
[   74.188749]  [<ffffffff812358e3>] sysfs_remove_group+0x93/0xa0
[   74.188754]  [<ffffffff814afc53>] dpm_sysfs_remove+0x43/0x50
[   74.188759]  [<ffffffff814a5635>] device_del+0x45/0x1c0
[   74.188762]  [<ffffffff814a57ce>] device_unregister+0x1e/0x60
[   74.188765]  [<ffffffff814a588c>] device_destroy+0x3c/0x50
[ 74.188777] [<ffffffffa016d0e5>] comedi_free_subdevice_minor+0x75/0xa0 [comedi] [ 74.188786] [<ffffffffa016db88>] comedi_device_detach+0x68/0x180 [comedi] [ 74.188793] [<ffffffffa0169238>] comedi_device_cleanup+0x38/0x80 [comedi] [ 74.188800] [<ffffffffa016acd6>] comedi_free_board_dev+0x36/0x60 [comedi] [ 74.188807] [<ffffffffa016cf70>] comedi_release_hardware_device+0x80/0x90 [comedi]
[   74.188815]  [<ffffffffa016d653>] comedi_auto_unconfig+0x13/0x20 [comedi]
[ 74.188823] [<ffffffffa016ee92>] comedi_usb_auto_unconfig+0x12/0x20 [comedi]
[   74.188829]  [<ffffffff8155d854>] usb_unbind_interface+0x64/0x1c0
[   74.188835]  [<ffffffff814a931f>] __device_release_driver+0x7f/0xf0
[   74.188838]  [<ffffffff814a93b3>] device_release_driver+0x23/0x30
[   74.188842]  [<ffffffff814a8c38>] bus_remove_device+0x108/0x180
[   74.188845]  [<ffffffff814a5719>] device_del+0x129/0x1c0
[   74.188849]  [<ffffffff8155b220>] usb_disable_device+0xb0/0x290
[   74.188855]  [<ffffffff8154f8bd>] usb_disconnect+0xad/0x200
[   74.188859]  [<ffffffff81552c3d>] hub_thread+0x70d/0x1750
[   74.188864]  [<ffffffff8109e0a8>] ? sched_clock_cpu+0xa8/0x100
[   74.188870]  [<ffffffff810ac5d0>] ? prepare_to_wait_event+0x100/0x100
[   74.188874]  [<ffffffff81552530>] ? usb_reset_device+0x1d0/0x1d0
[   74.188881]  [<ffffffff8108af22>] kthread+0xd2/0xf0
[   74.188885]  [<ffffffff8108ae50>] ? kthread_create_on_node+0x190/0x190
[   74.188890]  [<ffffffff8173e1fc>] ret_from_fork+0x7c/0xb0
[   74.188894]  [<ffffffff8108ae50>] ? kthread_create_on_node+0x190/0x190
[   74.188896] ---[ end trace d84882d84ae8116a ]---
[   75.960062] usb 1-7: new high-speed USB device number 5 using ehci-pci
[   76.092327] usb 1-7: New USB device found, idVendor=13d8, idProduct=0020
[ 76.092332] usb 1-7: New USB device strings: Mfr=0, Product=0, SerialNumber=0
[   76.136094] comedi comedi0: attached, ADC_zero = 8000f8
[  820.528409] audit_printk_skb: 15 callbacks suppressed
[ 820.528416] type=1006 audit(1389088301.191:31): pid=1888 uid=0 old auid=4294967295 new auid=1000 old ses=4294967295 new ses=1 res=1
bp1@bp1-Dimension-9100:~$


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.

What's the best solution to sort this out for both legacy and newer device?

/Bernd
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to