[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20080820123056.8f0c9d3f.akpm@linux-foundation.org>
Date: Wed, 20 Aug 2008 12:30:56 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Ian Campbell <ijc@...lion.org.uk>
Cc: torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org,
stable@...nel.org, jayakumar.lkml@...il.com, npiggin@...e.de,
a.p.zijlstra@...llo.nl, hugh@...itas.com, hannes@...urebad.de,
jeremy@...p.org, kel@...ku42.de, armbru@...hat.com
Subject: Re: [PATCH] fbdefio: add set_page_dirty handler to deferred IO FB
On Wed, 20 Aug 2008 19:57:53 +0100
Ian Campbell <ijc@...lion.org.uk> wrote:
> [correcting stable@]
>
> On Wed, 2008-08-20 at 01:37 -0700, Andrew Morton wrote:
> > On Wed, 20 Aug 2008 09:13:23 +0100 Ian Campbell <ijc@...lion.org.uk> wrote:
> >
> > > Perhaps applying the band-aid at open time instead would be preferred?
> >
> > That would be less racy, I expect.
> [...]
> > <tries to write a patch>
> >
> > OK, seems that fb_ops.fb_open() has no way of getting at the `struct
> > file *' which is being opened (wtf?). Screwed. Need to change
> > fb_ops.fb_open(), or add a new fb_ops.fb_open_sane().
>
> Ah yes, I remember why I did it in mmap() now...
>
> How about this version? Not as clean as overriding fb_open() but
> involves less frobbing with unrelated drivers.
>
> From ae2f7f118518fbfd4006c985b136a5d3d1a314af Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ijc@...lion.org.uk>
> Date: Wed, 20 Aug 2008 19:54:50 +0100
> Subject: [PATCH] fbdefio: add set_page_dirty handler to deferred IO FB
>
> Fixes kernel BUG at lib/radix-tree.c:473.
>
> Previously the handler was incidentally provided by tmpfs but this was
> removed with:
>
> commit 14fcc23fdc78e9d32372553ccf21758a9bd56fa1
> Author: Hugh Dickins <hugh@...itas.com>
> Date: Mon Jul 28 15:46:19 2008 -0700
>
> tmpfs: fix kernel BUG in shmem_delete_inode
>
> relying on this behaviour was incorrect in any case and the BUG
> also appeared when the device node was on an ext3 filesystem.
>
> v2: override a_ops at open() time rather than mmap() time to minimise
> races per AKPM's concerns.
>
> ...
>
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1344,6 +1344,10 @@ fb_open(struct inode *inode, struct file *file)
> if (res)
> module_put(info->fbops->owner);
> }
> +#ifdef CONFIG_FB_DEFERRED_IO
> + if (info->fbdefio)
> + fb_deferred_io_open(info, inode, file);
> +#endif
eww, hacky, but drivers/video/fbmem.c already got hacky:
#ifdef CONFIG_FB_DEFERRED_IO
.fsync = fb_deferred_io_fsync,
#endif
so it's not an original sin.
Does it work?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists