On Wed, Jan 30, 2013 at 01:08:57PM -0600, Brassow Jonathan wrote: > > On Jan 30, 2013, at 3:47 AM, Greg KH wrote: > > > On Tue, Jan 29, 2013 at 09:22:44AM +1100, NeilBrown wrote: > >> > >> From: Jonathan Brassow <[email protected]> > >> Subject: [PATCH] DM-RAID: Fix RAID10's check for sufficient redundancy > >> > >> commit 55ebbb59c1c6eb1b040f62b8c4ae0b724de6e55a upstream. > >> > >> Before attempting to activate a RAID array, it is checked for sufficient > >> redundancy. That is, we make sure that there are not too many failed > >> devices - or devices specified for rebuild - to undermine our ability to > >> activate the array. The current code performs this check twice - once to > >> ensure there were not too many devices specified for rebuild by the user > >> ('validate_rebuild_devices') and again after possibly experiencing a > >> failure > >> to read the superblock ('analyse_superblocks'). Neither of these checks > >> are > >> sufficient. The first check is done properly but with insufficient > >> information about the possible failure state of the devices to make a good > >> determination if the array can be activated. The second check is simply > >> done wrong in the case of RAID10 because it doesn't account for the > >> independence of the stripes (i.e. mirror sets). The solution is to use the > >> properly written check ('validate_rebuild_devices'), but perform the check > >> after the superblocks have been read and we know which devices have failed. > >> This gives us one check instead of two and performs it in a location where > >> it can be done right. > >> > >> Only RAID10 was affected and it was affected in the following ways: > >> - the code did not properly catch the condition where a user specified > >> a device for rebuild that already had a failed device in the same mirror > >> set. (This condition would, however, be caught at a deeper level in MD.) > >> - the code triggers a false positive and denies activation when devices in > >> independent mirror sets have failed - counting the failures as though they > >> were all in the same set. > >> > >> The most likely place this error was introduced (or this patch should have > >> been included) is in commit 4ec1e369 - first introduced in v3.7-rc1. > >> Consequently this fix should also go in v3.7.y, however there is a > >> small conflict on the .version in raid_target, so this is a separate patch > >> for -stable. > >> > >> Signed-off-by: Jonathan Brassow <[email protected]> > >> Signed-off-by: NeilBrown <[email protected]> > > > > > > I've applied this, but I now get the following build warning. Are you > > sure it's ok? > > > > drivers/md/dm-raid.c: In function ‘raid_ctr’: > > drivers/md/dm-raid.c:397:53: warning: ‘rebuilds_per_group’ may be used > > uninitialized in this function [-Wmaybe-uninitialized] > > drivers/md/dm-raid.c:352:11: note: ‘rebuilds_per_group’ was declared here > > It's fine. That variable is reset during the first instance of a > loop. I can quiet the warning with a separate patch (or modify the > first patch) if you like.
Does the warning also show up in Linus's tree? If so, get a cleanup patch in there first and then I can pick it up. If it doesn't show up in Linus's tree, why not? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
