From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bill Davidsen Subject: Re: Subject:[PATCH 001:013]: md: Raid0 reshape Date: Wed, 17 Jun 2009 09:30:58 -0400 Message-ID: <4A38F012.1070100@tmr.com> References: <1245189112.3478.94.camel@raz> <4A382993.1070702@tmr.com> <1245234307.3333.7.camel@raz> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1245234307.3333.7.camel@raz> Sender: linux-raid-owner@vger.kernel.org To: raz ben yehuda Cc: linux raid , Neil Brown List-Id: linux-raid.ids raz ben yehuda wrote: > On Tue, 2009-06-16 at 19:24 -0400, Bill Davidsen wrote: > >> raz ben yehuda wrote: >> >>> md assumes that personality has all its membes of the same >>> size,A fact that is incorrect for raid0. >>> >>> md.c | 11 +++++++++-- >>> 1 file changed, 9 insertions(+), 2 deletions(-) >>> >>> Signed-off-by: razb >>> --- >>> diff --git a/drivers/md/md.c b/drivers/md/md.c >>> index 0f11fd1..e14fb90 100644 >>> --- a/drivers/md/md.c >>> +++ b/drivers/md/md.c >>> @@ -5683,7 +5683,8 @@ static void status_resync(struct seq_file *seq, mddev_t * mddev) >>> max_sectors = mddev->resync_max_sectors; >>> else >>> max_sectors = mddev->dev_sectors; >>> - >>> + if (mddev->level == 0) >>> + max_sectors = mddev->resync_max_sectors; >>> /* >>> * Should not happen. >>> */ >>> @@ -6280,7 +6281,13 @@ void md_do_sync(mddev_t *mddev) >>> rdev->recovery_offset < j) >>> j = rdev->recovery_offset; >>> } >>> - >>> + /* >>> + * raid0 members may not be of the same size,use array_size. >>> + */ >>> + if (mddev->level == 0) { >>> + max_sectors = mddev->array_sectors; >>> + j = mddev->recovery_cp; >>> + } >>> printk(KERN_INFO "md: %s of RAID array %s\n", desc, mdname(mddev)); >>> printk(KERN_INFO "md: minimum _guaranteed_ speed:" >>> " %d KB/sec/disk.\n", speed_min(mddev)); >>> >>> >> If I admit I only spent about 20 minutes looking at this code will you >> explain why you use different fields of the struct to set max_sectors? I >> > > md_sync uses max_sectors as an ending point of the reshape process. > problem is that md assumes all raid's members are of the same size, so > it uses dev_sectors and this is not true to raid0 with multiple zones > >> guess my real confusion is why resync_max_sectors would be valid, given >> that raid0 has no redundancy. Or are you using it in some obscure way >> for reshape values? The values stored in the field don't really to be >> what you want... Yes, the reshape is new to me. >> > It has nothing to do with redundancy, but with the state machine of md. > I agree that there should be more elegant way to fix this, but it means > bigger fixes in md, and this is something I definitely don't want > to do. > There is always a trade off between cost of implementation and cost of maintenance. If Neil is happy with this I have no objection, it just seems to invite misunderstanding at some future enhancement. Perhaps a an additional comment is desirable, although the name of the field invites confusion, pity there's no reshape_* field to use, for readability if nothing else. -- Bill Davidsen Obscure bug of 2004: BASH BUFFER OVERFLOW - if bash is being run by a normal user and is setuid root, with the "vi" line edit mode selected, and the character set is "big5," an off-by-one error occurs during wildcard (glob) expansion.