On Tue, May 19, 2009 at 10:47:56AM +1000, Neil Brown wrote:

And 'temp' should be 'sector_t'.  'sector_div' requires a 'sector_t'
for the first argument.

...

Again, temp must be sector_t.

How about rolling our own md_sector_div() which at least checks for
such bugs via the

        (void)(((typeof((temp)) *)0) == ((sector_t *)0))

trick?

quoted

@@ -3996,14 +4001,23 @@ static int do_md_run(mddev_t * mddev)
 				chunk_size, MAX_CHUNK_SIZE);
 			return -EINVAL;
 		}
+
 		/*
 		 * chunk-size has to be a power of 2
 		 */
-		if ( (1 << ffz(~chunk_size)) != chunk_size) {
+		if ((1 << ffz(~chunk_size)) != chunk_size &&
+			 mddev->level != 0) {
 			printk(KERN_ERR "chunk_size of %d not valid\n", chunk_size);
 			return -EINVAL;
 		}

I wold really like to remove any knowledge about specific raid levels
from the common (md.c) code and keep it all in the personality modules
(is that a job for you Andre ??).
So I definitely don't want to add a test for ->level here.

So I would like to see the tests for chunk_size removed do_md_run and
included in each personalities ->run function.  This would be a series
of patches that adds the checks in ->run where needed, then removes it
from md.c.  Would you like to do that?

Sure, I can give it a try. Though I'm not sure I fully understand
what would be the difference between the checks in the individual
->run functions. Currently, in do_md_run() we check that

        * chunk_size <= MAX_CHUNK_SIZE
        * chunk_size is a power of two
        * the rdev is at least one chunk large

None of these checks depend on the raid level, so the above change
that allows chunk sizes which are not a power of two for raid0 would
be the only difference. Are you anticipating that the requirements
of the various raid levels with respect to chunk size will further
diverge in the future?

Thanks
Andre