Messages in this thread Patch in this message |  | | From | Neil Brown <> | Date | Wed, 27 Sep 2000 14:19:02 +1100 (EST) | Subject | Re: Bug and patch for md driver in 2.2.x. Could somebody please review? |
| |
On Tuesday September 19, bressler@mushroom.ca.boeing.com wrote: > Problem: > > The md driver doesn't handle large physical blocks in 2.2.x > ... > > A quick look at the 2.4pre code shows that this has been addressed so I > have taken a crack at adding a few similar lines of code to md.c in > order to get things working. I have to confess that except for a couple > of other trivial Linux patches I haven't done any OS programming since > my IBM/S370 days, so I'm somewhat uncomfortable at touching the I/O > subsystem. Still the Linux philosophy is if you need it and it doesn't > work go fix it, and I have tried. .... > > If that person(s) with some experience with the md driver and/or I/O > subsystem could review this, I'd sure appreciate it. Since it is useful > I'd like to submit it to Alan for inclusion in 2.2.1[89].
I addressed that issue in the 2.4pre code so I'm probably a good person to comment. The code looks OK except for one thinko which I will point out below. > > One more thing, I've always hated when folks post to a list but claim > they need email replies because they aren't on the list. Unfortunately > while I followed the list when I could get digests from vger the > hundreds of messages from the new non-digest list give my employer > heartburn. I have not yet gotten a workaround in place so I find myself > pleading for email responses. Sigh.
I think that it is common courtesy to reply to the author and cc to the list if that seem appropriately.
It might be worth your while joining linux-raid@vger.kernel.org It is very low volume compared to linux-kernel, and would be of interest if you are using md.
> > Anyway, here is the patch. If you have suggestions or can test it, I'd > sure appreciate it. > > There is also one other small patch that is part of the S390 system. > You'll need to apply it first to keep the context the same (unless > you're running a S390 Linux system with the August patches) but it is > conditional on the 390 system and shouldn't have an effect on > anything else, except to get my patch to apply. > > Thanks in advance! > > Here is the IBM patch to md.c Apply first in the drivers/block > directory. > > -------------------- cut here ---------------------------------- > diff -u -r --new-file linux-2.2.16/drivers/block/md.c linux-2.2.16-s390/drivers/block/md.c > --- md.c Wed Jun 7 23:26:42 2000 > +++ md.c Fri Jun 16 11:51:35 2000 > @@ -445,8 +445,13 @@ > } > > factor = min = 1 << FACTOR_SHIFT(FACTOR((md_dev+minor))); > - > + > +#ifdef CONFIG_ARCH_S390 > md_blocksizes[minor] <<= FACTOR_SHIFT(FACTOR((md_dev+minor))); > + if ( md_blocksizes[minor] > PAGE_SIZE ) { > + md_blocksizes[minor] = PAGE_SIZE; > + } > +#endif > > for (i=0; i<md_dev[minor].nb_dev; i++) > if (md_dev[minor].devices[i].size<min)
The line > md_blocksizes[minor] <<= FACTOR_SHIFT(FACTOR((md_dev+minor))); doesn't make any sense to me at all. FACTOR_SHIFT(FACTOR((md_dev+minor))) give the shift to get from kilobytes to the RAID cluster size. Why you would shift up the block size by this amount I cannot imagine.
I'm glad that it is being kept inside CONFIG_ARCH_S390!!
> -------------------- cut here ---------------------------------- > > > Here is my update, also applied in the drivers/block directory. > > > -------------------- cut here ---------------------------------- > --- md.c Tue Sep 19 19:44:52 2000 > +++ md.c.new Tue Sep 19 19:49:04 2000 > @@ -76,6 +76,7 @@ > > int md_size[MAX_MD_DEV]={0, }; > > +static int md_hardsect_sizes[MAX_MD_DEV]; > static void md_geninit (struct gendisk *); > > static struct gendisk md_gendisk= > @@ -464,7 +465,13 @@ > for (i=0; i<md_dev[minor].nb_dev; i++) { > fsync_dev(md_dev[minor].devices[i].dev); > invalidate_buffers(md_dev[minor].devices[i].dev); > + md_hardsect_sizes[minor]=512;
The above line should go OUTSIDE the loop (if at all), not inside.
> + if(get_hardblocksize(md_dev[minor].devices[i].dev) > md_hardsect_sizes[minor]) > + md_hardsect_sizes[minor] = get_hardblocksize(md_dev[minor].devices[i].dev); > } > + > + if(md_blocksizes[minor] < md_hardsect_sizes[minor]) > + md_blocksizes[minor] = md_hardsect_sizes[minor]; > > /* Resize devices according to the factor. It is used to align > partitions size on a given chunk size. */ > @@ -933,6 +940,7 @@ > for(i=0;i<MAX_MD_DEV;i++) > { > md_blocksizes[i] = 1024; > + md_hardsect_sizes[i] = 512; > md_maxreadahead[i] = MD_DEFAULT_DISK_READAHEAD; > md_gendisk.part[i].start_sect=-1; /* avoid partition check */ > md_gendisk.part[i].nr_sects=0; > @@ -941,6 +949,7 @@ > > blksize_size[MD_MAJOR] = md_blocksizes; > max_readahead[MD_MAJOR] = md_maxreadahead; > + hardsect_size[MD_MAJOR] = md_hardsect_sizes; > > #ifdef CONFIG_PROC_FS > proc_register(&proc_root, &proc_md); > -------------------- cut here ---------------------------------- > --
NeilBrown - 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/
|  |