All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Piggin <nickpiggin@yahoo.com.au>
To: Phillip Lougher <phillip@lougher.demon.co.uk>
Cc: linux-kernel@vger.kernel.org, Andrew Morton <akpm@osdl.org>
Subject: Re: [PATCH] VFS readahead bug in 2.6.8-rc[1-3]
Date: Fri, 06 Aug 2004 10:55:25 +1000	[thread overview]
Message-ID: <4112D6FD.4030707@yahoo.com.au> (raw)
In-Reply-To: <41127371.1000603@lougher.demon.co.uk>

Phillip Lougher wrote:

> Hi,
>
> There is a readahead bug in do_generic_mapping_read (filemap.c).  This
> bug appears to have been introduced in 2.6.8-rc1.  Specifically the bug
> is caused by an incorrect code change which causes VFS to call
> readpage() for indexes beyond the end of files where the file length is
> zero or a 4k multiple.
>
> In Squashfs this causes a variety of almost immediate OOPes because
> Squashfs trusts the VFS not to pass invalid index values.  For other
> filesystems it may also be causing subtle bugs.  I have received
> prune_dcache oopes similar to Gene Heskett's (which was also
> pointer corruption), and so it may fix this and other reported
> readahead bugs.
>
> The patch is against 2.6.8-rc3.
>

Good work - bug is mine, sorry.

You actually re-introduce a bug where read can return incorrect
data due to i_size changing from under it (I introduced this bug
while fixing that one).

My fix was to re-check i_size and update 'nr' after doing the
->readpage. You could probably fix up both problems with your
patch and also copying the hunk down to after i_size gets rechecked.
Does that sound ok?


The root of the problem is that i_size gets checked from multiple
places that it can get out of synch. A nice fix would be to snapshot
i_size once, and pass that around everywhere. Unfortunately this is
very intrusive.


  reply	other threads:[~2004-08-06  0:57 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-08-05 17:50 [PATCH] VFS readahead bug in 2.6.8-rc[1-3] Phillip Lougher
2004-08-06  0:55 ` Nick Piggin [this message]
2004-08-06  2:19   ` Nick Piggin
2004-08-06 16:58     ` Phillip Lougher
2004-08-06 18:58       ` Nick Piggin
2004-08-06 19:14         ` Phillip Lougher
2004-08-06 19:31           ` viro
2004-08-06 19:18         ` Phillip Lougher
2004-08-06 19:46           ` Andrew Morton
2004-08-16  7:55             ` [PATCH] " Ram Pai
2004-08-07 14:21         ` Pozsar Balazs
     [not found] <Pine.LNX.4.44.0408052104420.2241-100000@dyn319181.beaverton.ibm.com>
     [not found] ` <411322E8.4000503@yahoo.com.au>
2004-08-06 10:47   ` Ram
2004-08-06 17:05   ` Phillip Lougher
2004-08-06 18:02     ` Ram Pai
2004-08-06 19:09     ` Nick Piggin
2004-08-06 19:39       ` Phillip Lougher
2004-08-06 20:21         ` Nick Piggin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4112D6FD.4030707@yahoo.com.au \
    --to=nickpiggin@yahoo.com.au \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=phillip@lougher.demon.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.