* [PATCH] DM RAID: Explicitly turn off bad block support
@ 2013-04-19 13:30 Jonathan Brassow
2013-04-22 0:52 ` NeilBrown
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Brassow @ 2013-04-19 13:30 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, jbrassow
DM RAID: Explicitly turn off bad block support
DM RAID does not currently use the bad block tracking available in
MD. 'badblocks.shift' must be set to '-1' in order to explicitly
tell MD not to perform bad block related functions, like
narrow_write_error().
Signed-off-by: Jonathan Brassow <jbrassow@redhat.com>
Index: linux-upstream/drivers/md/dm-raid.c
===================================================================
--- linux-upstream.orig/drivers/md/dm-raid.c
+++ linux-upstream/drivers/md/dm-raid.c
@@ -170,8 +170,10 @@ static struct raid_set *context_alloc(st
rs->md.delta_disks = 0;
rs->md.recovery_cp = 0;
- for (i = 0; i < raid_devs; i++)
+ for (i = 0; i < raid_devs; i++) {
md_rdev_init(&rs->dev[i].rdev);
+ rs->dev[i].rdev.badblocks.shift = -1; /* No bad block support */
+ }
/*
* Remaining items to be initialized by further RAID params:
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] DM RAID: Explicitly turn off bad block support 2013-04-19 13:30 [PATCH] DM RAID: Explicitly turn off bad block support Jonathan Brassow @ 2013-04-22 0:52 ` NeilBrown 2013-04-22 16:29 ` Brassow Jonathan 0 siblings, 1 reply; 6+ messages in thread From: NeilBrown @ 2013-04-22 0:52 UTC (permalink / raw) To: Jonathan Brassow; +Cc: linux-raid [-- Attachment #1: Type: text/plain, Size: 2399 bytes --] On Fri, 19 Apr 2013 08:30:24 -0500 Jonathan Brassow <jbrassow@redhat.com> wrote: > DM RAID: Explicitly turn off bad block support > > DM RAID does not currently use the bad block tracking available in > MD. 'badblocks.shift' must be set to '-1' in order to explicitly > tell MD not to perform bad block related functions, like > narrow_write_error(). > > Signed-off-by: Jonathan Brassow <jbrassow@redhat.com> > > Index: linux-upstream/drivers/md/dm-raid.c > =================================================================== > --- linux-upstream.orig/drivers/md/dm-raid.c > +++ linux-upstream/drivers/md/dm-raid.c > @@ -170,8 +170,10 @@ static struct raid_set *context_alloc(st > rs->md.delta_disks = 0; > rs->md.recovery_cp = 0; > > - for (i = 0; i < raid_devs; i++) > + for (i = 0; i < raid_devs; i++) { > md_rdev_init(&rs->dev[i].rdev); > + rs->dev[i].rdev.badblocks.shift = -1; /* No bad block support */ > + } > > /* > * Remaining items to be initialized by further RAID params: > Thanks, but I don't think this is the right fix. md_rdev_init really should set shift to -1, because we can only support badblocks if the metadata explicitly supports it. What do you think of this instead? Thanks, NeilBrown diff --git a/drivers/md/md.c b/drivers/md/md.c index 41502cf..2166486 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -1567,8 +1567,8 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_ sector, count, 1) == 0) return -EINVAL; } - } else if (sb->bblog_offset == 0) - rdev->badblocks.shift = -1; + } else if (sb->bblog_offset != 0) + rdev->badblocks.shift = 0; if (!refdev) { ret = 1; @@ -3227,7 +3227,7 @@ int md_rdev_init(struct md_rdev *rdev) * be used - I wonder if that matters */ rdev->badblocks.count = 0; - rdev->badblocks.shift = 0; + rdev->badblocks.shift = -1; /* disabled until explicitly enabled */ rdev->badblocks.page = kmalloc(PAGE_SIZE, GFP_KERNEL); seqlock_init(&rdev->badblocks.lock); if (rdev->badblocks.page == NULL) @@ -3299,9 +3299,6 @@ static struct md_rdev *md_import_device(dev_t newdev, int super_format, int supe goto abort_free; } } - if (super_format == -1) - /* hot-add for 0.90, or non-persistent: so no badblocks */ - rdev->badblocks.shift = -1; return rdev; [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] DM RAID: Explicitly turn off bad block support 2013-04-22 0:52 ` NeilBrown @ 2013-04-22 16:29 ` Brassow Jonathan 2013-04-23 0:05 ` NeilBrown 0 siblings, 1 reply; 6+ messages in thread From: Brassow Jonathan @ 2013-04-22 16:29 UTC (permalink / raw) To: NeilBrown; +Cc: linux-raid@vger.kernel.org Raid, Jonathan Brassow On Apr 21, 2013, at 7:52 PM, NeilBrown wrote: > On Fri, 19 Apr 2013 08:30:24 -0500 Jonathan Brassow <jbrassow@redhat.com> > wrote: > >> DM RAID: Explicitly turn off bad block support >> >> DM RAID does not currently use the bad block tracking available in >> MD. 'badblocks.shift' must be set to '-1' in order to explicitly >> tell MD not to perform bad block related functions, like >> narrow_write_error(). >> >> Signed-off-by: Jonathan Brassow <jbrassow@redhat.com> >> >> Index: linux-upstream/drivers/md/dm-raid.c >> =================================================================== >> --- linux-upstream.orig/drivers/md/dm-raid.c >> +++ linux-upstream/drivers/md/dm-raid.c >> @@ -170,8 +170,10 @@ static struct raid_set *context_alloc(st >> rs->md.delta_disks = 0; >> rs->md.recovery_cp = 0; >> >> - for (i = 0; i < raid_devs; i++) >> + for (i = 0; i < raid_devs; i++) { >> md_rdev_init(&rs->dev[i].rdev); >> + rs->dev[i].rdev.badblocks.shift = -1; /* No bad block support */ >> + } >> >> /* >> * Remaining items to be initialized by further RAID params: >> > > Thanks, but I don't think this is the right fix. > md_rdev_init really should set shift to -1, because we can only support > badblocks if the metadata explicitly supports it. > > What do you think of this instead? I like it much better. It means that the proper initialization is done centrally, rather than expecting every super block type to know enough to turn off things they may not know about. I considered this, but went along with the idea that it was the responsibility of the individual superblock handlers. thanks! brassow ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] DM RAID: Explicitly turn off bad block support 2013-04-22 16:29 ` Brassow Jonathan @ 2013-04-23 0:05 ` NeilBrown 2013-04-24 3:48 ` Brassow Jonathan 0 siblings, 1 reply; 6+ messages in thread From: NeilBrown @ 2013-04-23 0:05 UTC (permalink / raw) To: Brassow Jonathan; +Cc: linux-raid@vger.kernel.org Raid [-- Attachment #1: Type: text/plain, Size: 1915 bytes --] On Mon, 22 Apr 2013 11:29:48 -0500 Brassow Jonathan <jbrassow@redhat.com> wrote: > > On Apr 21, 2013, at 7:52 PM, NeilBrown wrote: > > > On Fri, 19 Apr 2013 08:30:24 -0500 Jonathan Brassow <jbrassow@redhat.com> > > wrote: > > > >> DM RAID: Explicitly turn off bad block support > >> > >> DM RAID does not currently use the bad block tracking available in > >> MD. 'badblocks.shift' must be set to '-1' in order to explicitly > >> tell MD not to perform bad block related functions, like > >> narrow_write_error(). > >> > >> Signed-off-by: Jonathan Brassow <jbrassow@redhat.com> > >> > >> Index: linux-upstream/drivers/md/dm-raid.c > >> =================================================================== > >> --- linux-upstream.orig/drivers/md/dm-raid.c > >> +++ linux-upstream/drivers/md/dm-raid.c > >> @@ -170,8 +170,10 @@ static struct raid_set *context_alloc(st > >> rs->md.delta_disks = 0; > >> rs->md.recovery_cp = 0; > >> > >> - for (i = 0; i < raid_devs; i++) > >> + for (i = 0; i < raid_devs; i++) { > >> md_rdev_init(&rs->dev[i].rdev); > >> + rs->dev[i].rdev.badblocks.shift = -1; /* No bad block support */ > >> + } > >> > >> /* > >> * Remaining items to be initialized by further RAID params: > >> > > > > Thanks, but I don't think this is the right fix. > > md_rdev_init really should set shift to -1, because we can only support > > badblocks if the metadata explicitly supports it. > > > > What do you think of this instead? > > I like it much better. It means that the proper initialization is done centrally, rather than expecting every super block type to know enough to turn off things they may not know about. I considered this, but went along with the idea that it was the responsibility of the individual superblock handlers. > :-) Does this need to go to -stable? Which versions, do you know? Thanks, NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] DM RAID: Explicitly turn off bad block support 2013-04-23 0:05 ` NeilBrown @ 2013-04-24 3:48 ` Brassow Jonathan 2013-04-24 6:24 ` NeilBrown 0 siblings, 1 reply; 6+ messages in thread From: Brassow Jonathan @ 2013-04-24 3:48 UTC (permalink / raw) To: NeilBrown; +Cc: linux-raid@vger.kernel.org Raid, Jonathan Brassow On Apr 22, 2013, at 7:05 PM, NeilBrown wrote: > On Mon, 22 Apr 2013 11:29:48 -0500 Brassow Jonathan <jbrassow@redhat.com> > wrote: > >> >> On Apr 21, 2013, at 7:52 PM, NeilBrown wrote: >> >>> On Fri, 19 Apr 2013 08:30:24 -0500 Jonathan Brassow <jbrassow@redhat.com> >>> wrote: >>> >>>> DM RAID: Explicitly turn off bad block support >>>> >>>> DM RAID does not currently use the bad block tracking available in >>>> MD. 'badblocks.shift' must be set to '-1' in order to explicitly >>>> tell MD not to perform bad block related functions, like >>>> narrow_write_error(). >>>> >>>> Signed-off-by: Jonathan Brassow <jbrassow@redhat.com> >>>> >>>> Index: linux-upstream/drivers/md/dm-raid.c >>>> =================================================================== >>>> --- linux-upstream.orig/drivers/md/dm-raid.c >>>> +++ linux-upstream/drivers/md/dm-raid.c >>>> @@ -170,8 +170,10 @@ static struct raid_set *context_alloc(st >>>> rs->md.delta_disks = 0; >>>> rs->md.recovery_cp = 0; >>>> >>>> - for (i = 0; i < raid_devs; i++) >>>> + for (i = 0; i < raid_devs; i++) { >>>> md_rdev_init(&rs->dev[i].rdev); >>>> + rs->dev[i].rdev.badblocks.shift = -1; /* No bad block support */ >>>> + } >>>> >>>> /* >>>> * Remaining items to be initialized by further RAID params: >>>> >>> >>> Thanks, but I don't think this is the right fix. >>> md_rdev_init really should set shift to -1, because we can only support >>> badblocks if the metadata explicitly supports it. >>> >>> What do you think of this instead? >> >> I like it much better. It means that the proper initialization is done centrally, rather than expecting every super block type to know enough to turn off things they may not know about. I considered this, but went along with the idea that it was the responsibility of the individual superblock handlers. >> > > :-) > > Does this need to go to -stable? Which versions, do you know? dm-raid was introduce in 2.6.38, I believe. A quick check shows your patch applies to v3.1+ (although I've only compiled and tested 3.9-rc8). I would like to see the patch go into stable. brassow ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] DM RAID: Explicitly turn off bad block support 2013-04-24 3:48 ` Brassow Jonathan @ 2013-04-24 6:24 ` NeilBrown 0 siblings, 0 replies; 6+ messages in thread From: NeilBrown @ 2013-04-24 6:24 UTC (permalink / raw) To: Brassow Jonathan; +Cc: linux-raid@vger.kernel.org Raid [-- Attachment #1: Type: text/plain, Size: 2531 bytes --] On Tue, 23 Apr 2013 22:48:50 -0500 Brassow Jonathan <jbrassow@redhat.com> wrote: > > On Apr 22, 2013, at 7:05 PM, NeilBrown wrote: > > > On Mon, 22 Apr 2013 11:29:48 -0500 Brassow Jonathan <jbrassow@redhat.com> > > wrote: > > > >> > >> On Apr 21, 2013, at 7:52 PM, NeilBrown wrote: > >> > >>> On Fri, 19 Apr 2013 08:30:24 -0500 Jonathan Brassow <jbrassow@redhat.com> > >>> wrote: > >>> > >>>> DM RAID: Explicitly turn off bad block support > >>>> > >>>> DM RAID does not currently use the bad block tracking available in > >>>> MD. 'badblocks.shift' must be set to '-1' in order to explicitly > >>>> tell MD not to perform bad block related functions, like > >>>> narrow_write_error(). > >>>> > >>>> Signed-off-by: Jonathan Brassow <jbrassow@redhat.com> > >>>> > >>>> Index: linux-upstream/drivers/md/dm-raid.c > >>>> =================================================================== > >>>> --- linux-upstream.orig/drivers/md/dm-raid.c > >>>> +++ linux-upstream/drivers/md/dm-raid.c > >>>> @@ -170,8 +170,10 @@ static struct raid_set *context_alloc(st > >>>> rs->md.delta_disks = 0; > >>>> rs->md.recovery_cp = 0; > >>>> > >>>> - for (i = 0; i < raid_devs; i++) > >>>> + for (i = 0; i < raid_devs; i++) { > >>>> md_rdev_init(&rs->dev[i].rdev); > >>>> + rs->dev[i].rdev.badblocks.shift = -1; /* No bad block support */ > >>>> + } > >>>> > >>>> /* > >>>> * Remaining items to be initialized by further RAID params: > >>>> > >>> > >>> Thanks, but I don't think this is the right fix. > >>> md_rdev_init really should set shift to -1, because we can only support > >>> badblocks if the metadata explicitly supports it. > >>> > >>> What do you think of this instead? > >> > >> I like it much better. It means that the proper initialization is done centrally, rather than expecting every super block type to know enough to turn off things they may not know about. I considered this, but went along with the idea that it was the responsibility of the individual superblock handlers. > >> > > > > :-) > > > > Does this need to go to -stable? Which versions, do you know? > > dm-raid was introduce in 2.6.38, I believe. A quick check shows your patch applies to v3.1+ (although I've only compiled and tested 3.9-rc8). I would like to see the patch go into stable. > OK, I've tagged it for -stable. I won't try to get it into 3.9 as I think it is too late, but it'll go into 3.10-rc1 and then to the relevant -stables. Thanks, NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-24 6:24 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-04-19 13:30 [PATCH] DM RAID: Explicitly turn off bad block support Jonathan Brassow 2013-04-22 0:52 ` NeilBrown 2013-04-22 16:29 ` Brassow Jonathan 2013-04-23 0:05 ` NeilBrown 2013-04-24 3:48 ` Brassow Jonathan 2013-04-24 6:24 ` NeilBrown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox