lkml.org 
[lkml]   [2000]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: 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/

\
 
 \ /
  Last update: 2005-03-22 12:38    [W:0.037 / U:1.788 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site