* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.