Re: udlfb: remove sysfs framebuffer device with USB .disconnect()
From: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Date: 2012-01-21 13:44:24
Hi Kay, On 01/20/2012 08:18 PM, Kay Sievers wrote:
quoted hunk ↗ jump to hunk
From: Kay Sievers <redacted> Subject: udlfb: remove sysfs framebuffer device with USB .disconnect() The USB graphics card driver delays the unregistering of the framebuffer device to a workqueue, which breaks the userspace visible remove uevent sequence. Recent userspace tools started to support USB graphics card hotplug out-of-the-box and rely on proper events sent by the kernel. The framebuffer device is a direct child of the USB interface which is removed immediately after the USB .disconnect() callback. But the fb device in /sys stays around until its final cleanup, at a time where all the parent devices have been removed already. To work around that, we remove the sysfs fb device directly in the USB .disconnect() callback and leave only the cleanup of the internal fb data to the delayed work. Before: add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2 (usb) add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0 (usb) add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/graphics/fb0 (graphics) remove /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0 (usb) remove /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2 (usb) remove /2-1.2:1.0/graphics/fb0 (graphics) After: add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2 (usb) add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0 (usb) add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/graphics/fb1 (graphics) remove /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/graphics/fb1 (graphics) remove /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0 (usb) remove /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2 (usb) Cc: stable@vger.kernel.org Tested-By: Bernie Thompson <bernie@plugable.com> Acked-By: Bernie Thompson <bernie@plugable.com> Signed-off-by: Kay Sievers <redacted> --- drivers/video/fbmem.c | 18 +++++++++++++++++- drivers/video/udlfb.c | 2 +- include/linux/fb.h | 1 + 3 files changed, 19 insertions(+), 2 deletions(-)--- a/drivers/video/fbmem.c +++ b/drivers/video/fbmem.c@@ -1672,7 +1672,7 @@ static int do_unregister_framebuffer(str registered_fb[i] = NULL;
Here registered_fb[fb_info->node] is set to NULL...
quoted hunk ↗ jump to hunk
num_registered_fb--; fb_cleanup_device(fb_info); - device_destroy(fb_class, MKDEV(FB_MAJOR, i)); + unlink_framebuffer(fb_info); event.info = fb_info; fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);@@ -1681,6 +1681,22 @@ static int do_unregister_framebuffer(str return 0; } +int unlink_framebuffer(struct fb_info *fb_info) +{ + int i; + + i = fb_info->node; + if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info) + return -EINVAL;
...and here you check whether it is still valid (did you copy the check from the do_unregister_framebuffer?). So the code below would be never executed, when called in this context.
quoted hunk ↗ jump to hunk
+ + if (fb_info->dev) { + device_destroy(fb_class, MKDEV(FB_MAJOR, i)); + fb_info->dev = NULL; + } + return 0; +} +EXPORT_SYMBOL(unlink_framebuffer); + void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name, bool primary) {--- a/drivers/video/udlfb.c +++ b/drivers/video/udlfb.c@@ -1739,7 +1739,7 @@ static void dlfb_usb_disconnect(struct u for (i = 0; i < ARRAY_SIZE(fb_device_attrs); i++) device_remove_file(info->dev, &fb_device_attrs[i]); device_remove_bin_file(info->dev, &edid_attr); - + unlink_framebuffer(info); usb_set_intfdata(interface, NULL); /* if clients still have us open, will be freed on last close */ --- a/include/linux/fb.h +++ b/include/linux/fb.h@@ -1003,6 +1003,7 @@ extern ssize_t fb_sys_write(struct fb_in /* drivers/video/fbmem.c */ extern int register_framebuffer(struct fb_info *fb_info); extern int unregister_framebuffer(struct fb_info *fb_info); +extern int unlink_framebuffer(struct fb_info *fb_info); extern void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name, bool primary); extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
Best regards, Florian Tobias Schandinat