On Tue, 28 Oct 2014 16:47:06 -0700 Justin Maggard [off-list ref]
wrote:

On Tue, Oct 28, 2014 at 3:19 PM, NeilBrown [off-list ref] wrote:

quoted

On Fri, 24 Oct 2014 17:55:02 -0700 Justin Maggard [off-list ref]
wrote:

quoted

If the request --size to --grow an array to is larger
than 32bits, then mdadm may make the wrong choice and
use ioctl instead of setting component_size via sysfs
and the change is ignored.

Can you explain exactly why the  current code is not sufficient?  When does
it fail?
If you include the explanation in a re-submission of the patch, and I am
convinced, then I will gladly apply your patch.

To be honest, I can't figure out how the current comparison would have
ever worked at all. To illustrate, here's a simple test program:
-----
$ ./test 0xfffffffff
cast comparison did not detect truncation
bit comparison detected truncation

$ cat test.c
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>

int main(int argc, char **argv)
{
unsigned long long ssize = strtoull(argv[1], NULL, 0);
int asize;

asize = ssize;
if (asize != (signed)ssize)
printf("cast comparison detected truncation\n");
else
printf("cast comparison did not detect truncation \n");
if (ssize & ~INT32_MAX)
printf("bit comparison detected truncation\n");
else
printf("bit comparison did not detect truncation \n");

return 0;
}
-----

I plugged lots of numbers in there, and I was never able to get the
current cast comparison to see a difference.

I ran into the issue by trying to grow the component size of a RAID
array from 1TB to 3TB, and it wouldn't work if I specified the size;
only using "max" worked.

I'm happy to re-submit if you'd like; I just thought it was a pretty
straightforward bug.  I guess what I'm saying is, I don't understand
why there *should* be a difference between assigning a unsigned long
long to an int variable, and casting that unsigned long long to a
signed type.  But

-Justin

quoted

Thanks,
NeilBrown

quoted

Instead of using casts to check for a 32-bit overflow,
just check for set bits outside of INT32_MAX.
---
 Grow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Grow.c b/Grow.c
index a9c8589..a614102 100644
--- a/Grow.c
+++ b/Grow.c

@@ -1818,7 +1818,7 @@ int Grow_reshape(char *devname, int fd,
              if (s->size == MAX_SIZE)
                      s->size = 0;
              array.size = s->size;
-             if (array.size != (signed)s->size) {
+             if (s->size & ~INT32_MAX) {
                      /* got truncated to 32bit, write to
                       * component_size instead
                       */

Thanks.
Looks like I broke it in July.

http://git.neil.brown.name/?p=mdadm.git;a=commitdiff;h=4e9a3dd16d656b269f5602624ac4f7109a571368

I probably should have made it
    if (s->size != (signed long long)s->size)

I've applied your patch.

Thanks,
NeilBrown