Re: [PATCH 0/7] HID: picoLCD updates

Hello Tejun,

[Tejun: sorry for duplicate, did hit "reply" instead of "reply to all"]

On Mon, 13 Aug 2012 16:27:08 Tejun Heo [off-list ref] wrote:

On Thu, Aug 09, 2012 at 08:09:47PM +0200, Bruno Prémont wrote:

quoted

As you are working on workqueues and related code, could you have a look
at my usage of them in combination with db_defio?

The delayed memory corruptions or system reboots after unbinding/unplugging
the PicoLCD seem very much related to workqueue used to handle the deferred
IO to framebuffer.

I think things don't get cleaned-up as they should though I'm not sure
where the trouble lies.

For ease of reading, I'm inlineing below the framebuffer related code of
PicoLCD (for complete driver after this patch series apply the whole series
on top of 3.5 https://lkml.org/lkml/2012/7/30/375 )

...

quoted

void picolcd_exit_framebuffer(struct picolcd_data *data)
{
	struct fb_info *info = data->fb_info;
	u8 *fb_vbitmap = data->fb_vbitmap;

	if (!info)
		return;

	device_remove_file(&data->hdev->dev, &dev_attr_fb_update_rate);
	info->par = NULL;
	unregister_framebuffer(info);
	data->fb_vbitmap = NULL;
	data->fb_bitmap  = NULL;
	data->fb_bpp     = 0;
	data->fb_info    = NULL;
	kfree(fb_vbitmap);
}

I'm kinda shooting in the dark but who flushes / cancels
fb_info->deferred_work?

fb_deferred_io_cleanup() does so and is called by destroy fbops
(when last reference to fb_info is returned):


static void picolcd_fb_destroy(struct fb_info *info)
{
	/* make sure no work is deferred */
	fb_deferred_io_cleanup(info);

	vfree((u8 *)info->fix.smem_start);
	framebuffer_release(info);
	printk(KERN_DEBUG "picolcd_fb_destroy(%p)\n", info);
}


=== drivers/video/fb_defio.c ==void fb_deferred_io_cleanup(struct fb_info *info)
{
        struct fb_deferred_io *fbdefio = info->fbdefio;
        struct page *page;
        int i;

        BUG_ON(!fbdefio);
        cancel_delayed_work_sync(&info->deferred_work);

        /* clear out the mapping that we setup */
        for (i = 0 ; i < info->fix.smem_len; i += PAGE_SIZE) {
                page = fb_deferred_io_page(info, i);
                page->mapping = NULL;
        }

        info->fbops->fb_mmap = NULL;
        mutex_destroy(&fbdefio->lock);
}



Thanks,
Bruno