Re: [PATCH 7/8] md: update_size(): Remove useless variable "fit".

[Andre Noll <maan@systemlinux.org>]

[-- Attachment #1: Type: text/plain, Size: 2388 bytes --]

On 09:08, Neil Brown wrote:
> On Saturday April 5, maan@systemlinux.org wrote:
> > 
> > Signed-off-by: Andre Noll <maan@systemlinux.org>
> > ---
> >  drivers/md/md.c |    3 +--
> >  1 files changed, 1 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 3f22ec7..98a024c 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -4459,7 +4459,6 @@ static int update_size(mddev_t *mddev, unsigned long size)
> >  	mdk_rdev_t * rdev;
> >  	int rv;
> >  	struct list_head *tmp;
> > -	int fit = (size == 0);
> >  
> >  	if (mddev->pers->resize == NULL)
> >  		return -EINVAL;
> > @@ -4479,7 +4478,7 @@ static int update_size(mddev_t *mddev, unsigned long size)
> >  		sector_t avail;
> >  		avail = rdev->size * 2;
> >  
> > -		if (fit && (size == 0 || size > avail/2))
> > +		if (size == 0 || size > avail/2)
> >  			size = avail/2;
> >  		if (avail < ((sector_t)size << 1))
> >  			return -ENOSPC;
> 
> This is wrong.
> The old code tested for "size == 0" once outside a loop.
> The new code does the test inside a loop which potentially changes
> size.

Oops, right you are. Please drop.

> If you are keen to do some more tidying up, I would really love it if
> all internal values that are currently stored as 'K' would instead be
> stored as sectors.
> So for example with update_size, we could make the value that is
> passed in be a number of sectors, get rid of those divisions.

Of course the size unit exported to user space programs via sysfs must
not change. So if we change update_size() as you propose, the division
has to be done in the size_store() instead, which calls update_size()
with a value obtained from sysfs.

Therefore I think it's impossible to completely get rid of the
divisions/multiplications without breaking user space, but it might
still be worth to change the internal representations.

I will have a look at it, but I might need some further advice.

> BTW your patches arrive without a valid "To" field.  The Date is odd
> too.

They were generated by git-format-patch and then bounced to the
list. The date is "correct" in the sense that I checked them in back
then. I'll see that subsequent patches have a proper date and a valid
"To" field.

Thanks
Andre
-- 
The only person who always got his work done by Friday was Robinson Crusoe

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]