Linux-kernel mailing list archive 2000-40,: breada bug

# breada bug

Andries Brouwer (aeb@veritas.com)
Sun, 1 Oct 2000 03:30:33 +0200

- Messages sorted by: [ date ][ thread ][ subject ][ author ]
- Next message: Matthew Wilcox: "Re: [RFC] Imminent death of /proc/locks predicted; film at 11"
- Previous message: Peter Samuelson: "Re: We interrupt you regularly scheduled catfight for.. Linux 2.2.18pre13"

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/

- Next message: Matthew Wilcox: "Re: [RFC] Imminent death of /proc/locks predicted; film at 11"
- Previous message: Peter Samuelson: "Re: We interrupt you regularly scheduled catfight for.. Linux 2.2.18pre13"