From: Bernie Thompson <ber...@plugable.com>
This is a request to backport a 3.4 udlfb driver patch to 3.3.*
Existing upstream (3.4) commit ID: 8d21547d3c9c3bc653261f26d554cfabc4a083de
This patch fixes a critical panic (in hcd_buffer_free) with unplug/replug
of USB graphics devices. Prior to this patch, were doing too much in USB
probe(), including starting big transfers to paint the screen of the USB
graphics device. This both was triggering race conditions with disconnect()
and teardown. and probably holding up USB enumeration generally. With this
patch, probe() and disconnect() do the minimum necessary, and move longer
device-specific steps to delayed work.
The race condition that this patch fixes has turned out to be more frequent
than originally realized. This patch is effectively required for stable USB
multiseat support (e.g. in Fedora 17).
This patch is a little longer than the 100 line guideline (81+ 66-), because
of the need to move some lines.
This patch has been in use since 3.4rc1 and has proven both to have fixed the
original issue, and have no known side effects. It generally improves the
stability of DisplayLink USB graphics devices on Linux.
It should apply to 3.3.* cleanly, as there aren't significant intervening
patches.
It has no pre-requisites for 3.3.6+
original patch as taken in 3.4:
Fix race conditions with unplug/replug behavior, in particular
take care not to hold up USB probe/disconnect for long-running
framebuffer operations and rely on usb to handle teardown.
Fix for kernel panic reported with new F17 multiseat support.
Reported-by: Kay Sievers <kay.siev...@vrfy.org>
Signed-off-by: Bernie Thompson <ber...@plugable.com>
---
drivers/video/udlfb.c | 146 +++++++++++++++++++++++++++----------------------
include/video/udlfb.h | 1 +
2 files changed, 81 insertions(+), 66 deletions(-)
diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c
index 04aea20..cbf030d 100644
--- a/drivers/video/udlfb.c
+++ b/drivers/video/udlfb.c
@@ -918,10 +918,6 @@ static void dlfb_free(struct kref *kref)
{
struct dlfb_data *dev = container_of(kref, struct dlfb_data, kref);
- /* this function will wait for all in-flight urbs to complete */
- if (dev->urbs.count > 0)
- dlfb_free_urb_list(dev);
-
if (dev->backing_buffer)
vfree(dev->backing_buffer);
@@ -940,35 +936,42 @@ static void dlfb_release_urb_work(struct work_struct
*work)
up(&unode->dev->urbs.limit_sem);
}
-static void dlfb_free_framebuffer_work(struct work_struct *work)
+static void dlfb_free_framebuffer(struct dlfb_data *dev)
{
- struct dlfb_data *dev = container_of(work, struct dlfb_data,
- free_framebuffer_work.work);
struct fb_info *info = dev->info;
- int node = info->node;
- unregister_framebuffer(info);
+ if (info) {
+ int node = info->node;
- if (info->cmap.len != 0)
- fb_dealloc_cmap(&info->cmap);
- if (info->monspecs.modedb)
- fb_destroy_modedb(info->monspecs.modedb);
- if (info->screen_base)
- vfree(info->screen_base);
+ unregister_framebuffer(info);
- fb_destroy_modelist(&info->modelist);
+ if (info->cmap.len != 0)
+ fb_dealloc_cmap(&info->cmap);
+ if (info->monspecs.modedb)
+ fb_destroy_modedb(info->monspecs.modedb);
+ if (info->screen_base)
+ vfree(info->screen_base);
+
+ fb_destroy_modelist(&info->modelist);
- dev->info = 0;
+ dev->info = NULL;
- /* Assume info structure is freed after this point */
- framebuffer_release(info);
+ /* Assume info structure is freed after this point */
+ framebuffer_release(info);
- pr_warn("fb_info for /dev/fb%d has been freed\n", node);
+ pr_warn("fb_info for /dev/fb%d has been freed\n", node);
+ }
/* ref taken in probe() as part of registering framebfufer */
kref_put(&dev->kref, dlfb_free);
}
+static void dlfb_free_framebuffer_work(struct work_struct *work)
+{
+ struct dlfb_data *dev = container_of(work, struct dlfb_data,
+ free_framebuffer_work.work);
+ dlfb_free_framebuffer(dev);
+}
/*
* Assumes caller is holding info->lock mutex (for open and release at least)
*/
@@ -1571,14 +1574,15 @@ success:
kfree(buf);
return true;
}
+
+static void dlfb_init_framebuffer_work(struct work_struct *work);
+
static int dlfb_usb_probe(struct usb_interface *interface,
const struct usb_device_id *id)
{
struct usb_device *usbdev;
struct dlfb_data *dev = 0;
- struct fb_info *info = 0;
int retval = -ENOMEM;
- int i;
/* usb initialization */
@@ -1590,9 +1594,7 @@ static int dlfb_usb_probe(struct usb_interface *interface,
goto error;
}
- /* we need to wait for both usb and fbdev to spin down on disconnect */
kref_init(&dev->kref); /* matching kref_put in usb .disconnect fn */
- kref_get(&dev->kref); /* matching kref_put in free_framebuffer_work */
dev->udev = usbdev;
dev->gdev = &usbdev->dev; /* our generic struct device * */
@@ -1620,10 +1622,39 @@ static int dlfb_usb_probe(struct usb_interface
*interface,
goto error;
}
+ kref_get(&dev->kref); /* matching kref_put in free_framebuffer_work */
+
/* We don't register a new USB class. Our client interface is fbdev */
+ /* Workitem keep things fast & simple during USB enumeration */
+ INIT_DELAYED_WORK(&dev->init_framebuffer_work,
+ dlfb_init_framebuffer_work);
+ schedule_delayed_work(&dev->init_framebuffer_work, 0);
+
+ return 0;
+
+error:
+ if (dev) {
+
+ kref_put(&dev->kref, dlfb_free); /* ref for framebuffer */
+ kref_put(&dev->kref, dlfb_free); /* last ref from kref_init */
+
+ /* dev has been deallocated. Do not dereference */
+ }
+
+ return retval;
+}
+
+static void dlfb_init_framebuffer_work(struct work_struct *work)
+{
+ struct dlfb_data *dev = container_of(work, struct dlfb_data,
+ init_framebuffer_work.work);
+ struct fb_info *info;
+ int retval;
+ int i;
+
/* allocates framebuffer driver structure, not framebuffer memory */
- info = framebuffer_alloc(0, &interface->dev);
+ info = framebuffer_alloc(0, dev->gdev);
if (!info) {
retval = -ENOMEM;
pr_err("framebuffer_alloc failed\n");
@@ -1669,15 +1700,13 @@ static int dlfb_usb_probe(struct usb_interface
*interface,
for (i = 0; i < ARRAY_SIZE(fb_device_attrs); i++) {
retval = device_create_file(info->dev, &fb_device_attrs[i]);
if (retval) {
- pr_err("device_create_file failed %d\n", retval);
- goto err_del_attrs;
+ pr_warn("device_create_file failed %d\n", retval);
}
}
retval = device_create_bin_file(info->dev, &edid_attr);
if (retval) {
- pr_err("device_create_bin_file failed %d\n", retval);
- goto err_del_attrs;
+ pr_warn("device_create_bin_file failed %d\n", retval);
}
pr_info("DisplayLink USB device /dev/fb%d attached. %dx%d resolution."
@@ -1685,38 +1714,10 @@ static int dlfb_usb_probe(struct usb_interface
*interface,
info->var.xres, info->var.yres,
((dev->backing_buffer) ?
info->fix.smem_len * 2 : info->fix.smem_len) >> 10);
- return 0;
-
-err_del_attrs:
- for (i -= 1; i >= 0; i--)
- device_remove_file(info->dev, &fb_device_attrs[i]);
+ return;
error:
- if (dev) {
-
- if (info) {
- if (info->cmap.len != 0)
- fb_dealloc_cmap(&info->cmap);
- if (info->monspecs.modedb)
- fb_destroy_modedb(info->monspecs.modedb);
- if (info->screen_base)
- vfree(info->screen_base);
-
- fb_destroy_modelist(&info->modelist);
-
- framebuffer_release(info);
- }
-
- if (dev->backing_buffer)
- vfree(dev->backing_buffer);
-
- kref_put(&dev->kref, dlfb_free); /* ref for framebuffer */
- kref_put(&dev->kref, dlfb_free); /* last ref from kref_init */
-
- /* dev has been deallocated. Do not dereference */
- }
-
- return retval;
+ dlfb_free_framebuffer(dev);
}
static void dlfb_usb_disconnect(struct usb_interface *interface)
@@ -1736,12 +1737,24 @@ static void dlfb_usb_disconnect(struct usb_interface
*interface)
/* When non-active we'll update virtual framebuffer, but no new urbs */
atomic_set(&dev->usb_active, 0);
- /* remove udlfb's sysfs interfaces */
- 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);
+ /* this function will wait for all in-flight urbs to complete */
+ dlfb_free_urb_list(dev);
+
+ if (info) {
+
+ /* remove udlfb's sysfs interfaces */
+ 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);
+
+ /* it's safe to uncomment next line if your kernel
+ doesn't yet have this function exported */
+ unlink_framebuffer(info);
+ }
+
usb_set_intfdata(interface, NULL);
+ dev->udev = NULL;
+ dev->gdev = NULL;
/* if clients still have us open, will be freed on last close */
if (dev->fb_count == 0)
@@ -1807,12 +1820,12 @@ static void dlfb_free_urb_list(struct dlfb_data *dev)
int ret;
unsigned long flags;
- pr_notice("Waiting for completes and freeing all render urbs\n");
+ pr_notice("Freeing all render urbs\n");
/* keep waiting and freeing, until we've got 'em all */
while (count--) {
- /* Getting interrupted means a leak, but ok at shutdown*/
+ /* Getting interrupted means a leak, but ok at disconnect */
ret = down_interruptible(&dev->urbs.limit_sem);
if (ret)
break;
@@ -1834,6 +1847,7 @@ static void dlfb_free_urb_list(struct dlfb_data *dev)
kfree(node);
}
+ dev->urbs.count = 0;
}
static int dlfb_alloc_urb_list(struct dlfb_data *dev, int count, size_t size)
diff --git a/include/video/udlfb.h b/include/video/udlfb.h
index c41f308..f9466fa 100644
--- a/include/video/udlfb.h
+++ b/include/video/udlfb.h
@@ -41,6 +41,7 @@ struct dlfb_data {
char *backing_buffer;
int fb_count;
bool virtualized; /* true when physical usb device not present */
+ struct delayed_work init_framebuffer_work;
struct delayed_work free_framebuffer_work;
atomic_t usb_active; /* 0 = update virtual buffer, but no usb traffic */
atomic_t lost_pixels; /* 1 = a render op failed. Need screen refresh */
--
1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
