breada bug

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

  • Next message: Matthew Wilcox: "Re: [RFC] Imminent death of /proc/locks predicted; film at 11"

    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:31:57 EDT