Re: another IDE cleanup: kill duplicated code

Martin Dalecki
2002-02-12T13:59:34Z

Vojtech Pavlik wrote:

>On Tue, Feb 12, 2002 at 02:17:48PM +0100, Martin Dalecki wrote:
>
>>So the conclusions is that not just the read_ahead array is bogous now.
>>The max_readahead array can be killed entierly from the kernel as well ;-).
>>
>>The answer is: I'm now confident that you can just remove all the
>>max_readahead initialization from the ide code.
>>
>
>Since I've come to the same conclusion, here is the patch. It removes
>read_ahead, max_readahead, BLKRAGET, BLKRASET, BLKFRAGET and BLKFRASET
>completely.
>
Welcome the the "Write Only Variable" haters club ;-).

Please note that the lvm code is playing mental with himself on the
lv_read_ahead struct member as well.
This member get's set saved preserved, but it's never used nowhere there:

./include/linux/lvm.h: uint lv_read_ahead;
./include/linux/lvm.h: uint32_t lv_read_ahead; /* HM */

>diff -urN linux-2.5.4/drivers/block/ll_rw_blk.c linux-2.5.4-readahead/drivers/block/ll_rw_blk.c
>--- linux-2.5.4/drivers/block/ll_rw_blk.c Thu Jan 31 16:45:20 2002
>+++ linux-2.5.4-readahead/drivers/block/ll_rw_blk.c Tue Feb 12 14:27:32 2002
>@@ -56,8 +56,6 @@
>
> /* This specifies how many sectors to read ahead on the disk. */
>
>-int read_ahead[MAX_BLKDEV];
>-
>

You did miss the comment by shooting above!

>diff -urN linux-2.5.4/include/linux/fs.h linux-2.5.4-readahead/include/linux/fs.h
>--- linux-2.5.4/include/linux/fs.h Tue Feb 12 12:22:54 2002
>+++ linux-2.5.4-readahead/include/linux/fs.h Tue Feb 12 14:32:32 2002
>@@ -173,10 +173,6 @@
> #define BLKRRPART _IO(0x12,95) /* re-read partition table */
> #define BLKGETSIZE _IO(0x12,96) /* return device size /512 (long *arg) */
> #define BLKFLSBUF _IO(0x12,97) /* flush buffer cache */
>-#define BLKRASET _IO(0x12,98) /* Set read ahead for block device */
>-#define BLKRAGET _IO(0x12,99) /* get current read ahead setting */
>-#define BLKFRASET _IO(0x12,100)/* set filesystem (mm/filemap.c) read-ahead */
>-#define BLKFRAGET _IO(0x12,101)/* get filesystem (mm/filemap.c) read-ahead */
> #define BLKSECTSET _IO(0x12,102)/* set max sectors per request (ll_rw_blk.c) */
> #define BLKSECTGET _IO(0x12,103)/* get max sectors per request (ll_rw_blk.c) */
> #define BLKSSZGET _IO(0x12,104)/* get block device sector size */
>@@ -1490,8 +1486,6 @@
>

I would rather suggest to to #if 0 ... #endif instead with a note about
those values beeing no longer used.

>diff -urN linux-2.5.4/mm/filemap.c linux-2.5.4-readahead/mm/filemap.c
>--- linux-2.5.4/mm/filemap.c Tue Feb 12 12:22:54 2002
>+++ linux-2.5.4-readahead/mm/filemap.c Tue Feb 12 14:29:58 2002
>@@ -1131,13 +1131,6 @@
> * 64k if defined (4K page size assumed).
> */
>
>-static inline int get_max_readahead(struct inode * inode)
>-{
>- if (kdev_none(inode->i_dev) || !max_readahead[major(inode->i_dev)])
>- return MAX_READAHEAD;
>- return max_readahead[major(inode->i_dev)][minor(inode->i_dev)];
>-}
>-
> static void generic_file_readahead(int reada_ok,
> struct file * filp, struct inode * inode,
> struct page * page)
>@@ -1146,7 +1139,7 @@
> unsigned long index = page->index;
> unsigned long max_ahead, ahead;
> unsigned long raend;
>- int max_readahead = get_max_readahead(inode);
>+ int max_readahead = MAX_READAHEAD;
>

This wonders me a bit, why you didn't just propagate the constant deeper.