Bug 14662

Created attachment 23867 [details]
Pictures with kernel panic

Hi Chris,

I'm slightly worried by this patch which went into Linus's git yesterday.

Thank you for looking into the issue, and coming up with what Mateusz
has found to be a good workaround.

But is it the right fix?

drm/i915 ought to be working without CONFIG_SHMEM: it then falls back
to using ramfs instead of shmem.  So why isn't that working here?

I'd take the NULL function pointer in read_cache_page_async() to be a
NULL filler function; but that should be pointing to simple_readpage(),
as specified in ramfs_aops.

You don't go into more detail in the comments: do you have any more
info on how this came about?  I don't get it.

I'd prefer not to "select SHMEM" there, because it shouldn't be
necessary; and will put CONFIG_SHMEM=y into the .configs of users
who chose not to have it before, and who will forget to reverse it
if we come up with a better fix letter.

If there is a better fix: I just don't understand what happened here.

Thanks,
Hugh

From: Chris Wilson <chris@chris-wilson.co.uk>
Date: Sun, 22 Nov 2009 15:40:31 +0000 (+0000)
Subject: drm/i915: Select CONFIG_SHMEM
X-Git-Url: http://git.kernel.org/gitweb.cgi?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=commitdiff_plain;h=ca9ab10033d190c1ede85fdf456307bdfdabf079

drm/i915: Select CONFIG_SHMEM

The driver requires shmfs as the backing filesystem to handle the buffer
objects, so ensure it is selected if the user chooses to build our
driver.

Fixes: Bug 14662 - Dell E5500 kernel panic with KMS
http://bugzilla.kernel.org/show_bug.cgi?id=14662

The revealing nature of the panic is the NULL function pointer
dereference in read_cache_page_async().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reported-and-tested-by: Mateusz Kaduk <mateusz.kaduk@gmail.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
Cc: stable@kernel.org
---

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index f831ea1..96eddd1 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -92,6 +92,7 @@ config DRM_I830
 config DRM_I915
 	tristate "i915 driver"
 	depends on AGP_INTEL
+	select SHMEM
 	select DRM_KMS_HELPER
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA

On Tue, 2009-12-01 at 19:26 +0000, Hugh Dickins wrote:
> Hi Chris,
> 
> I'm slightly worried by this patch which went into Linus's git yesterday.
> 
> Thank you for looking into the issue, and coming up with what Mateusz
> has found to be a good workaround.
> 
> But is it the right fix?

This really does look like a band-aid. There's either a real problem in
tiny-shmem or alternately something scribbling on its aops? In which
case turning shmem on is just causing something different to get
scribbled on.

Now that you've pointed out the fallback suppport for !SHMEM, yes it does look like a bandaid. As you can probably guess, diagnosis was performed as a simple scan over config in complete ignorance of the ramfs fallback.

If something really was scribbling over the aops, then surely it would also be a problem with SHMEM as well? I'll see if I can reproduce the failure on a test machine and debug the issue truly.

So starting to digest mm/shmem.c and it appears that shmem_aops.readpage is only defined if TMPFS -- with no protection if read_mapping_page() is called without the filler defined.

From that reading it looks like i915 actually requires TMPFS, which depends on SHMEM, and not SHMEM itself.

Tag you're it

I dont experience it anymore. Closing.