[lkml]   [2002]   [Feb]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: another IDE cleanup: kill duplicated code
Vojtech Pavlik wrote:

>On Tue, Feb 12, 2002 at 01:45:10PM +0100, Martin Dalecki wrote:
>>>>The first does control an array of values, which doesn't make sense in
>>>>first place. I.e. changing it doesn't
>>>>change ANY behaviour of the kernel.
>>>Actually HFS uses it ...
>>Please note that the use in HFS is inappriopriate. It's supposed to
>>optimize the read throughput there, which is something that shouldn't
>>be done at the fs setup layer at all. The usage of read_ahead there
>>can be just removed and everything should be fine (modulo the fact
>>that in current state the HFS filesystem code is just non-working
>>obsolete code garbage anyway ;-).
>Yes; I just commented on that it does change some behavior of the
>>>>The second of them is trying to control a file-system level constant
>>>>inside the actual block device driver. This is a blatant violation of
>>>>the layering principle in software design, and should go as soon as
>>>Yes. But still block device drivers allocate the array ...
>>Well if we do:
>>find ./ -name "*.c" -exec grep max_readahead /dev/null {} \;
>>One can already easly see that apart from ide, md, and acorn drivers
>>this has been already abstracted away one level up at the block device
>>handling as well. My suspiction is that there is now already *double*
>>initialization of the sub-slices of this array there. Anyway ide
>>should be adapted here to the same way as it's done now for scsi.
>>Will you look after this or should me? (I can't until afternoon,
>>becouse I'm at my "true live" job now and have other things to do...)
>Am I understanding you correctly that we can just remove the
>max_readahead references in IDE (and possibly also md and acorn)?

Yes they can certailny be killed there. However please note that I'm not
quite sure whatever some other
adjustments may be needed. I suggest to double check with the way the
scsi driver middle layer
deals with them.

OK. I can't resist. Let's have a closer look together:

~/tmp/trans/linux-2.5.4# find ./ -name "*.[ch]" -exec grep max_readahead
/dev/null {} \;
./drivers/acorn/block/mfmhd.c: max_readahead[major][minor] = arg;
./drivers/acorn/block/mfmhd.c: return
put_user(max_readahead[major][minor], (long *) arg);

1. ^^^^ Here apparently no additional allocation is needed.

./drivers/block/blkpg.c: if (!(iptr =
./drivers/block/blkpg.c: if (!(iptr =

2. ^^^^ This is just read out to the user space through ioctl()..

./drivers/block/ll_rw_blk.c:int * max_readahead[MAX_BLKDEV];
./drivers/block/ll_rw_blk.c: memset(max_readahead, 0,

3. ^^^^ This is just initialisation.

./drivers/ide/ide-probe.c: max_readahead[hwif->major] = max_ra;
./drivers/ide/ide-disk.c: ide_add_setting(drive, "fi ....
1024, &max_readahead[major][minor], NULL);
./drivers/ide/ide-floppy.c: ide_add_setting(drive, "....,
&max_readahead[major][minor], NULL);
./drivers/ide/ide-cd.c: ide_add_setting(drive, "file_readahead",. ... ,
&max_readahead[major][minor], NULL);
./drivers/ide/ide.c: kfree(max_readahead[hwif->major]);
./drivers/ide/ataraid.c: max_readahead[ATAMAJOR] = NULL;

4. ^^^^ Still just initialization.

./drivers/md/md.c: max_readahead[MAJOR_NR] = md_maxreadahead;
./include/linux/blkdev.h:extern int * max_readahead[MAX_BLKDEV];
./include/linux/blkdev.h: max_readahead[major] = NULL;

5. Still no true usage.

./mm/filemap.c:static inline int get_max_readahead(struct inode * inode)
./mm/filemap.c: if (kdev_none(inode->i_dev) ||
./mm/filemap.c: return
./mm/filemap.c: int max_readahead = get_max_readahead(inode);
./mm/filemap.c: if (filp->f_ramax > max_readahead)
./mm/filemap.c: filp->f_ramax = max_readahead;
./mm/filemap.c: int max_readahead = get_max_readahead(inode);
./mm/filemap.c: if (filp->f_ramax > max_readahead)
./mm/filemap.c: filp->f_ramax = max_readahead;
./mm/filemap.c: ra_window =

6. Hmm I wonder how the above is working in case somebody didn't care
about this array.

Let's have a closer look at it there (mm/filemap.c).

If max_readahead array is not set at all, the filemap code just assumes
a constant
MAX_READAHEAD value. max_readahead is an upper bound value for the number
for consecutive file sectors read at once.

static inline int get_max_readahead(struct inode * inode)
if (kdev_none(inode->i_dev) || !max_readahead[major(inode->i_dev)])
return max_readahead[major(inode->i_dev)][minor(inode->i_dev)];

Other then this we have the following code in nopage_sequention_readahead:

ra_window = get_max_readahead(vma->vm_file->f_dentry->d_inode);
ra_window = CLUSTER_OFFSET(ra_window + CLUSTER_PAGES - 1);

Obviously the first file of the two can be just killed without any harm.

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.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2005-03-22 13:24    [W:0.895 / U:1.728 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site