breada bug

From: Andries Brouwer (aeb@veritas.com)
Date: Sat Sep 30 2000 - 20:30:33 EST


On Tue, 29 Feb 2000 11:10:20, Mikulas Patocka wrote:

: This code in breada
:
: if (blocks < (read_ahead[MAJOR(dev)] >> index))
: blocks = read_ahead[MAJOR(dev)] >> index;
:
: will increase number of block that are read ahead. However the code
: doesn't check for end of device and so it can generate accesses beyond
: end of device.

In patch-2.3.99-pre1 he, or someone else, added

----------------------------------------------------------------------
--- v2.3.51/linux/fs/hpfs/buffer.c Tue Jun 1 23:25:47 1999
+++ linux/fs/hpfs/buffer.c Mon Mar 13 12:27:51 2000
@@ -125,7 +125,8 @@
        kdev_t dev = s->s_dev;
        struct buffer_head *bh;

- if (!ahead || secno + ahead >= s->s_hpfs_fs_size)
+ /* vvvv - workaround for the breada bug */
+ if (!ahead || secno + ahead + (read_ahead[MAJOR(dev)] >> 9)
+ >= s->s_hpfs_fs_size)
                *bhp = bh = bread(dev, secno, 512);
        else *bhp = bh = breada(dev, secno, 512, 0, (ahead + 1) << 9);
----------------------------------------------------------------------

Two remarks:

(i) This code in hpfs/buffer.c is bogus:
The last argument of breada is a bytecount, read_ahead[]
and ahead are sector counts, and "(read_ahead[MAJOR(dev)] >> 9)"
has the wrong unit. Probably it is zero, and hence harmless,
but still this patch fragment and a similar one farther down
in the same file should be reverted.

(ii) Concerning the original comment, I agree, but disagree
with the proposed patch (not quoted here). Instead, I think
that the above test in fs/buffer.c should just be deleted.
In other words:

--- buffer.c~ Sun Sep 24 20:53:57 2000
+++ buffer.c Sun Oct 1 03:17:33 2000
@@ -1038,12 +1038,8 @@
 
        blocks = (filesize - pos) >> (9+index);
 
- if (blocks < (read_ahead[MAJOR(dev)] >> index))
- blocks = read_ahead[MAJOR(dev)] >> index;
        if (blocks > NBUF)
                blocks = NBUF;
-
-/* if (blocks) printk("breada (new) %d blocks\n",blocks); */
 
        bhlist[0] = bh;
        j = 1;

[pasted from another window]
(Then the logic becomes:
read ahead until end-of-file, but no more than NBUF blocks.)
I think that breada is used only in isofs and hpfs.
Perhaps the entire routine can be deleted.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sat Sep 30 2000 - 21:00:28 EST