* Re: [PATCH 03/10] Create n bitmaps for clustered mode
From: Goldwyn Rodrigues @ 2015-04-30 12:44 UTC (permalink / raw)
To: NeilBrown; +Cc: gqjiang, linux-raid
In-Reply-To: <20150430125153.428f4884@notabene.brown>
On 04/29/2015 09:51 PM, NeilBrown wrote:
> On Tue, 28 Apr 2015 21:41:47 -0500 Goldwyn Rodrigues <rgoldwyn@suse.de> wrote:
>
>>
>>
>> On 04/28/2015 08:36 PM, NeilBrown wrote:
>>> On Fri, 24 Apr 2015 15:30:34 +0800 gqjiang@suse.com wrote:
>>>
>>>> From: Guoqing Jiang <gqjiang@suse.com>
>>>>
>>>> For a clustered MD, create bitmaps equal to number of nodes so
>>>> each node has an independent bitmap.
>>>>
>>>> Only the first bitmap is has the bits set so that the first node
>>>> that assembles the device also performs the sync.
>>>>
>>>> The bitmaps are aligned to 4k boundaries.
>>>>
>>>> On-disk format:
>>>>
>>>> 0 4k 8k 12k
>>>> -------------------------------------------------------------------
>>>> | idle | md super | bm super [0] + bits |
>>>> | bm bits[0, contd] | bm super[1] + bits | bm bits[1, contd] |
>>>> | bm super[2] + bits | bm bits [2, contd] | bm super[3] + bits |
>>>> | bm bits [3, contd] | | |
>>>>
>>>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
>>>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>>>> ---
>>>> Create.c | 3 ++-
>>>> bitmap.h | 7 +++++--
>>>> mdadm.8.in | 7 ++++++-
>>>> mdadm.c | 17 ++++++++++++++++-
>>>> super1.c | 59 +++++++++++++++++++++++++++++++++++++++++------------------
>>>> 5 files changed, 70 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/Create.c b/Create.c
>>>> index cd5485b..9663dc4 100644
>>>> --- a/Create.c
>>>> +++ b/Create.c
>>>> @@ -752,7 +752,8 @@ int Create(struct supertype *st, char *mddev,
>>>> #endif
>>>> }
>>>>
>>>> - if (s->bitmap_file && strcmp(s->bitmap_file, "internal")==0) {
>>>> + if (s->bitmap_file && (strcmp(s->bitmap_file, "internal")==0
>>>> + || strcmp(s->bitmap_file, "clustered")==0)) {
>>>> if ((vers%100) < 2) {
>>>> pr_err("internal bitmaps not supported by this kernel.\n");
>>>> goto abort_locked;
>>>> diff --git a/bitmap.h b/bitmap.h
>>>> index c8725a3..adbf0b4 100644
>>>> --- a/bitmap.h
>>>> +++ b/bitmap.h
>>>> @@ -154,8 +154,11 @@ typedef struct bitmap_super_s {
>>>> __u32 chunksize; /* 52 the bitmap chunk size in bytes */
>>>> __u32 daemon_sleep; /* 56 seconds between disk flushes */
>>>> __u32 write_behind; /* 60 number of outstanding write-behind writes */
>>>> -
>>>> - __u8 pad[256 - 64]; /* set to zero */
>>>> + __u32 sectors_reserved; /* 64 number of 512-byte sectors that are
>>>> + * reserved for the bitmap. */
>>>> + __u32 nodes; /* 68 the maximum number of nodes in cluster. */
>>>> + __u8 cluster_name[64]; /* 72 cluster name to which this md belongs */
>>>> + __u8 pad[256 - 136]; /* set to zero */
>>>> } bitmap_super_t;
>>>>
>>>> /* notes:
>>>> diff --git a/mdadm.8.in b/mdadm.8.in
>>>> index a0e8288..c015cbf 100644
>>>> --- a/mdadm.8.in
>>>> +++ b/mdadm.8.in
>>>> @@ -700,7 +700,12 @@ and so is replicated on all devices. If the word
>>>> .B "none"
>>>> is given with
>>>> .B \-\-grow
>>>> -mode, then any bitmap that is present is removed.
>>>> +mode, then any bitmap that is present is removed. If the word
>>>> +.B "clustered"
>>>> +is given, the array is created for a clustered environment. One bitmap
>>>> +is created for each node as defined by the
>>>> +.B \-\-nodes
>>>> +parameter and are stored internally.
>>>>
>>>> To help catch typing errors, the filename must contain at least one
>>>> slash ('/') if it is a real file (not 'internal' or 'none').
>>>> diff --git a/mdadm.c b/mdadm.c
>>>> index e4f8568..6963a09 100644
>>>> --- a/mdadm.c
>>>> +++ b/mdadm.c
>>>> @@ -1111,6 +1111,15 @@ int main(int argc, char *argv[])
>>>> s.bitmap_file = optarg;
>>>> continue;
>>>> }
>>>> + if (strcmp(optarg, "clustered")== 0) {
>>>> + s.bitmap_file = optarg;
>>>> + /* Set the default number of cluster nodes
>>>> + * to 4 if not already set by user
>>>> + */
>>>> + if (c.nodes < 1)
>>>> + c.nodes = 4;
>>>> + continue;
>>>> + }
>>>> /* probable typo */
>>>> pr_err("bitmap file must contain a '/', or be 'internal', or 'none'\n"
>>>> " not '%s'\n", optarg);
>>>> @@ -1404,7 +1413,13 @@ int main(int argc, char *argv[])
>>>> if (c.delay == 0)
>>>> c.delay = DEFAULT_BITMAP_DELAY;
>>>>
>>>> - if (!strncmp(s.bitmap_file, "internal", 9) ||
>>>> + if (!strncmp(s.bitmap_file, "clustered", 9)) {
>>>> + if (s.level != 1) {
>>>> + pr_err("--bitmap=clustered is currently supported with RAID mirror only\n");
>>>> + rv = 1;
>>>> + break;
>>>> + }
>>>> + } else if (!strncmp(s.bitmap_file, "internal", 9) ||
>>>> !strncmp(s.bitmap_file,"none", 4)) {
>>>> if (c.nodes) {
>>>> pr_err("--nodes argument is incompatible with --bitmap=%s.\n",
>>>> diff --git a/super1.c b/super1.c
>>>> index f0508fe..ac1b011 100644
>>>> --- a/super1.c
>>>> +++ b/super1.c
>>>> @@ -2144,6 +2144,10 @@ add_internal_bitmap1(struct supertype *st,
>>>> bms->daemon_sleep = __cpu_to_le32(delay);
>>>> bms->sync_size = __cpu_to_le64(size);
>>>> bms->write_behind = __cpu_to_le32(write_behind);
>>>> + bms->nodes = __cpu_to_le32(st->nodes);
>>>> + if (st->cluster_name)
>>>> + strncpy((char *)bms->cluster_name,
>>>> + st->cluster_name, strlen(st->cluster_name));
>>>>
>>>> *chunkp = chunk;
>>>> return 1;
>>>> @@ -2177,6 +2181,7 @@ static int write_bitmap1(struct supertype *st, int fd)
>>>> void *buf;
>>>> int towrite, n;
>>>> struct align_fd afd;
>>>> + unsigned int i;
>>>>
>>>> init_afd(&afd, fd);
>>>>
>>>> @@ -2185,27 +2190,45 @@ static int write_bitmap1(struct supertype *st, int fd)
>>>> if (posix_memalign(&buf, 4096, 4096))
>>>> return -ENOMEM;
>>>>
>>>> - memset(buf, 0xff, 4096);
>>>> - memcpy(buf, (char *)bms, sizeof(bitmap_super_t));
>>>> -
>>>> - towrite = __le64_to_cpu(bms->sync_size) / (__le32_to_cpu(bms->chunksize)>>9);
>>>> - towrite = (towrite+7) >> 3; /* bits to bytes */
>>>> - towrite += sizeof(bitmap_super_t);
>>>> - towrite = ROUND_UP(towrite, 512);
>>>> - while (towrite > 0) {
>>>> - n = towrite;
>>>> - if (n > 4096)
>>>> - n = 4096;
>>>> - n = awrite(&afd, buf, n);
>>>> - if (n > 0)
>>>> - towrite -= n;
>>>> + /* We use bms->nodes as opposed to st->nodes to
>>>> + * be compatible with write-after-reads such as
>>>> + * the GROW operation.
>>>> + */
>>>> + for (i = 0; i < __le32_to_cpu(bms->nodes); i++) {
>>>> + /* Only the first bitmap should resync
>>>> + * the whole device
>>>> + */
>>>> + if (i)
>>>> + memset(buf, 0x00, 4096);
>>>> else
>>>> + memset(buf, 0xff, 4096);
>>>
>>> Why is the first bitmap initialised to 0x00 and the others to 0xff?
>>> If there is a good reason it should be documented either in a comment in the
>>> code or in the changelog entry.
>>
>>
>> Rather, it is the reverse. The first one is initialized to 0xff and the
>> rest are set to 0x00.
>>
>> The reason is only the first node to assemble the device should perform
>> the resync (if --assume-clean is not provided). The comment is right
>> above the code. Perhaps I should be more elaborate with the comment.
>>
>>
>
> Hmmm... Perhaps I should read code with my eyes open!
>
> Not sure I agree though. Why should the first node be special?
> What if node '0' doesn't get activated?
> I guess it always well because of the way numbers are assigned, but I'm not
> feeling very comfortable...
Yes, the first (zero'th) one will get activated first. The cluster
communication will guarantee that.
In any case, we do have fallback scenarios:
- In case of failure of the first node, the "alive" node will take over
- All bitmaps are checked by the kernel while assembling. This works for
a total cluster failure as well.
> Thinking a bit more ... why do we set any bits to '1'?
This is how the original non-clustered code is, I just moved it to the
zeroth node :)
> Why not just set BITMAP_STALE, and let the kernel figure things out.
>
> For the single-node case, BITMAP_STALE is the same as setting all the bits to
> one.
> For the cluster case, we can get BITMAP_STALE to do whatever we want. and we
> should make sure we handle it correctly anyway.
>
> So maybe mdadm should set BITMAP_STALE, and leave all the bits as 0.
>
> Thoughts?
Should it be set for all bitmaps? If yes, how should the second node
behave on observing that BITMAP_STALE is set while assembling? I suppose
we can clear the flag when the (first) node is reading all bitmaps.
I suppose with --assume-clean, we just not set the BITMAP_STALE. Right?
--
Goldwyn
^ permalink raw reply
* Re: [PATCH 09/10] mdadm: change the num of cluster node
From: Guoqing Jiang @ 2015-04-30 10:04 UTC (permalink / raw)
To: NeilBrown; +Cc: Guoqing Jiang, linux-raid, rgoldwyn
In-Reply-To: <20150430164742.6cf78f6c@notabene.brown>
NeilBrown wrote:
> On Thu, 30 Apr 2015 11:34:05 +0800 Guoqing Jiang <jgq516@gmail.com> wrote:
>
>
>> NeilBrown wrote:
>>
>>> On Fri, 24 Apr 2015 15:30:40 +0800 gqjiang@suse.com wrote:
>>>
>>>
>>>
>>>> From: Guoqing Jiang <gqjiang@suse.com>
>>>>
>>>> This extends nodes option for assemble mode, make the num of
>>>> cluster node could be change by user.
>>>>
>>>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>>>> ---
>>>> Assemble.c | 4 ++++
>>>> ReadMe.c | 2 +-
>>>> mdadm.c | 3 +++
>>>> mdadm.h | 1 +
>>>> super1.c | 6 ++++++
>>>> 5 files changed, 15 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Assemble.c b/Assemble.c
>>>> index e1b846c..22042a9 100644
>>>> --- a/Assemble.c
>>>> +++ b/Assemble.c
>>>> @@ -649,6 +649,10 @@ static int load_devices(struct devs *devices, char *devmap,
>>>> devname, 0, 0, c->homecluster);
>>>> tst->ss->write_bitmap(tst, dfd, NameUpdate);
>>>> }
>>>> + if (strcmp(c->update, "nodes") == 0) {
>>>> + tst->nodes = c->nodes;
>>>> + tst->ss->write_bitmap(tst, dfd, NodeNumUpdate);
>>>> + }
>>>>
>>>>
>>> Doesn't there need to be some test that there is enough free space on all
>>> devices to store the extra bitmaps (when nodes is increasing)??
>>>
>>>
>>>
>> Agree, could you pls elaborate more about the test? I guess the test
>> need to be run
>> before write_bitmap1call the awrite(&afd, buf, n).
>>
>
> Normally we don't increase the size of a bitmap while it is alive.
> When an array is reshaped, bitmap_resize in the kernel actually changes the
> bitmap chunk size if necessary so that the bitmap will fit in the available
> space.
> To make the bitmap larger, the only current approach is to delete the bitmap
> and add a new one. This is handled by add_internal_bitmap1 in mdadm.
> So make the bitmap bigger you will need to perform similar calculations taht
> add_internal_bitmap1 performs, though instead of choosing a size or
> chunksize, you see need to check if the space is sufficient.
>
>
Thanks a lot for the detailed infos. Seems the size of bitmap is one of
4k, 64k and
128k (per choose_bm_space), is it ok to call choose_bm_space to get
current bitmap
size? And since the size of each bitmap can be get by bitmap_bits, then
we can
know the current size of bitmap is enough or not by something like
int room = choose_bm_space(__le64_to_cpu(sb->size));
int total_bm_size_byte = room / 2 * 1024;
if (total_bm_size_byte > nodenums * bitmap_bits() / 8) {
/* enough space */
continue the write_bitmap
} else {
/* no enough space*/
Just warn it.
}
I think we should not allocate more space for extra nodes if current
size is not bigger
than required, otherwise it is possible to violate the meaning of
choose_bm_space.
Please correct me if I misunderstood something.
Thanks,
Guoqing
> Hope that helps,
> NeilBrown
>
^ permalink raw reply
* Re: [PATCH stable] block: discard bdi_unregister() in favour of bdi_destroy()
From: Peter Zijlstra @ 2015-04-30 8:35 UTC (permalink / raw)
To: NeilBrown
Cc: Jens Axboe, Christoph Hellwig, Mike Snitzer, Azat Khuzhin,
Kernel.org-Linux-RAID, Guoqing Jiang, Tejun Heo, Jan Kara, lkml,
device-mapper development
In-Reply-To: <20150430103233.75227693@notabene.brown>
On Thu, Apr 30, 2015 at 10:32:33AM +1000, NeilBrown wrote:
>
> bdi_unregister() now contains very little functionality.
>
> It contains a "WARN_ON" if bdi->dev is NULL. This warning is of no
> real consequence as bdi->dev isn't needed by anything else in the function,
> and it triggers if
> blk_cleanup_queue() -> bdi_destroy()
> is called before bdi_unregister, which a subsequent patch will make happen.
> So this isn't wanted.
>
> It also calls bdi_set_min_ratio(). This needs to be called after
> writes through the bdi have all been flushed, and before the bdi is destroyed.
> Calling it early is better than calling it late as it frees up a global
> resource.
>
> Calling it immediately after bdi_wb_shutdown() in bdi_destroy()
> perfectly fits these requirements.
>
> So bdi_unregister can be discarded with the important content moved to
> bdi_destroy, as can the
> writeback_bdi_unregister
> event which is already not used.
>
> This is tagged for 'stable' as it is a pre-requisite for a subsequent
> patch which moves calls to blk_cleanup_queue() before calls to
> del_gendisk(). The commit identified as 'Fixes' removed a lot of
> other functionality from bdi_unregister(), and made a change which
> necessitated moving the blk_cleanup_queue() calls.
>
> Reported-by: Mike Snitzer <snitzer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: stable@vger.kernel.org (v4.0)
> Fixes: c4db59d31e39ea067c32163ac961e9c80198fd37
Fixes: c4db59d31e39 ("fs: don't reassign dirty inodes to default_backing_dev_info")
> Signed-off-by: NeilBrown <neilb@suse.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply
* Re: [PATCH 2/2] md/raid5: trivial coding style fix
From: Yuanhan Liu @ 2015-04-30 7:23 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, linux-kernel
In-Reply-To: <20150430171650.4db1bcca@notabene.brown>
On Thu, Apr 30, 2015 at 05:16:50PM +1000, NeilBrown wrote:
> On Thu, 30 Apr 2015 15:01:17 +0800 Yuanhan Liu <yuanhan.liu@linux.intel.com>
> wrote:
>
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > ---
> > drivers/md/raid5.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > index 2651bda..bae3e2c 100644
> > --- a/drivers/md/raid5.c
> > +++ b/drivers/md/raid5.c
> > @@ -5789,8 +5789,7 @@ static void raid5d(struct md_thread *thread)
> > if (released)
> > clear_bit(R5_DID_ALLOC, &conf->cache_state);
> >
> > - if (
> > - !list_empty(&conf->bitmap_list)) {
> > + if (!list_empty(&conf->bitmap_list)) {
> > /* Now is a good time to flush some bitmap updates */
> > conf->seq_flush++;
> > spin_unlock_irq(&conf->device_lock);
>
>
> I'm happy for these sorts of changes when you are fixing up nearby code, or
> if the change significantly improves readability.
> But I'd rather not bother is one-off trivial fixes like this.
Got it.
--yliu
^ permalink raw reply
* Re: [PATCH 1/2] md/raid5: fix typo
From: Yuanhan Liu @ 2015-04-30 7:22 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, linux-kernel
In-Reply-To: <20150430171426.46d03aa1@notabene.brown>
On Thu, Apr 30, 2015 at 05:14:26PM +1000, NeilBrown wrote:
> On Thu, 30 Apr 2015 15:01:16 +0800 Yuanhan Liu <yuanhan.liu@linux.intel.com>
> wrote:
>
> > bion -> bios
> >
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > ---
> > drivers/md/raid5.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > index 697d77a..2651bda 100644
> > --- a/drivers/md/raid5.c
> > +++ b/drivers/md/raid5.c
> > @@ -2919,7 +2919,7 @@ schedule_reconstruction(struct stripe_head *sh, struct stripe_head_state *s,
> > }
> >
> > /*
> > - * Each stripe/dev can have one or more bion attached.
> > + * Each stripe/dev can have one or more bios attached.
> > * toread/towrite point to the first in a chain.
> > * The bi_next chain must be in order.
> > */
>
> That was intentional. "bios" as a plural looks too much like "BIOS" which is
> in the ROM of computers.
>
> Children and oxen are plurals with an 'n' at the end. So I used 'bion'.
> Private joke?
Interesting.
>
> I'd rather leave it as it is.
Okay, and sorry for the noise.
--yliu
^ permalink raw reply
* Re: [PATCH 2/2] md/raid5: trivial coding style fix
From: NeilBrown @ 2015-04-30 7:16 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: linux-raid, linux-kernel
In-Reply-To: <1430377277-6167-2-git-send-email-yuanhan.liu@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 989 bytes --]
On Thu, 30 Apr 2015 15:01:17 +0800 Yuanhan Liu <yuanhan.liu@linux.intel.com>
wrote:
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
> drivers/md/raid5.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 2651bda..bae3e2c 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -5789,8 +5789,7 @@ static void raid5d(struct md_thread *thread)
> if (released)
> clear_bit(R5_DID_ALLOC, &conf->cache_state);
>
> - if (
> - !list_empty(&conf->bitmap_list)) {
> + if (!list_empty(&conf->bitmap_list)) {
> /* Now is a good time to flush some bitmap updates */
> conf->seq_flush++;
> spin_unlock_irq(&conf->device_lock);
I'm happy for these sorts of changes when you are fixing up nearby code, or
if the change significantly improves readability.
But I'd rather not bother is one-off trivial fixes like this.
Thanks anyway,
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] md/raid5: fix typo
From: NeilBrown @ 2015-04-30 7:14 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: linux-raid, linux-kernel
In-Reply-To: <1430377277-6167-1-git-send-email-yuanhan.liu@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 999 bytes --]
On Thu, 30 Apr 2015 15:01:16 +0800 Yuanhan Liu <yuanhan.liu@linux.intel.com>
wrote:
> bion -> bios
>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
> drivers/md/raid5.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 697d77a..2651bda 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -2919,7 +2919,7 @@ schedule_reconstruction(struct stripe_head *sh, struct stripe_head_state *s,
> }
>
> /*
> - * Each stripe/dev can have one or more bion attached.
> + * Each stripe/dev can have one or more bios attached.
> * toread/towrite point to the first in a chain.
> * The bi_next chain must be in order.
> */
That was intentional. "bios" as a plural looks too much like "BIOS" which is
in the ROM of computers.
Children and oxen are plurals with an 'n' at the end. So I used 'bion'.
Private joke?
I'd rather leave it as it is.
Thanks,
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* [PATCH 2/2] md/raid5: trivial coding style fix
From: Yuanhan Liu @ 2015-04-30 7:01 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, linux-kernel, Yuanhan Liu
In-Reply-To: <1430377277-6167-1-git-send-email-yuanhan.liu@linux.intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
drivers/md/raid5.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 2651bda..bae3e2c 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5789,8 +5789,7 @@ static void raid5d(struct md_thread *thread)
if (released)
clear_bit(R5_DID_ALLOC, &conf->cache_state);
- if (
- !list_empty(&conf->bitmap_list)) {
+ if (!list_empty(&conf->bitmap_list)) {
/* Now is a good time to flush some bitmap updates */
conf->seq_flush++;
spin_unlock_irq(&conf->device_lock);
--
1.9.0
^ permalink raw reply related
* [PATCH 1/2] md/raid5: fix typo
From: Yuanhan Liu @ 2015-04-30 7:01 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, linux-kernel, Yuanhan Liu
bion -> bios
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
drivers/md/raid5.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 697d77a..2651bda 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2919,7 +2919,7 @@ schedule_reconstruction(struct stripe_head *sh, struct stripe_head_state *s,
}
/*
- * Each stripe/dev can have one or more bion attached.
+ * Each stripe/dev can have one or more bios attached.
* toread/towrite point to the first in a chain.
* The bi_next chain must be in order.
*/
--
1.9.0
^ permalink raw reply related
* Re: [PATCH 09/10] mdadm: change the num of cluster node
From: NeilBrown @ 2015-04-30 6:47 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: gqjiang, linux-raid, rgoldwyn
In-Reply-To: <5541A2AD.3030506@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]
On Thu, 30 Apr 2015 11:34:05 +0800 Guoqing Jiang <jgq516@gmail.com> wrote:
> NeilBrown wrote:
> > On Fri, 24 Apr 2015 15:30:40 +0800 gqjiang@suse.com wrote:
> >
> >
> >> From: Guoqing Jiang <gqjiang@suse.com>
> >>
> >> This extends nodes option for assemble mode, make the num of
> >> cluster node could be change by user.
> >>
> >> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> >> ---
> >> Assemble.c | 4 ++++
> >> ReadMe.c | 2 +-
> >> mdadm.c | 3 +++
> >> mdadm.h | 1 +
> >> super1.c | 6 ++++++
> >> 5 files changed, 15 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Assemble.c b/Assemble.c
> >> index e1b846c..22042a9 100644
> >> --- a/Assemble.c
> >> +++ b/Assemble.c
> >> @@ -649,6 +649,10 @@ static int load_devices(struct devs *devices, char *devmap,
> >> devname, 0, 0, c->homecluster);
> >> tst->ss->write_bitmap(tst, dfd, NameUpdate);
> >> }
> >> + if (strcmp(c->update, "nodes") == 0) {
> >> + tst->nodes = c->nodes;
> >> + tst->ss->write_bitmap(tst, dfd, NodeNumUpdate);
> >> + }
> >>
> >
> > Doesn't there need to be some test that there is enough free space on all
> > devices to store the extra bitmaps (when nodes is increasing)??
> >
> >
> Agree, could you pls elaborate more about the test? I guess the test
> need to be run
> before write_bitmap1call the awrite(&afd, buf, n).
Normally we don't increase the size of a bitmap while it is alive.
When an array is reshaped, bitmap_resize in the kernel actually changes the
bitmap chunk size if necessary so that the bitmap will fit in the available
space.
To make the bitmap larger, the only current approach is to delete the bitmap
and add a new one. This is handled by add_internal_bitmap1 in mdadm.
So make the bitmap bigger you will need to perform similar calculations taht
add_internal_bitmap1 performs, though instead of choosing a size or
chunksize, you see need to check if the space is sufficient.
Hope that helps,
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 04/10] Show all bitmaps while examining bitmap
From: NeilBrown @ 2015-04-30 4:45 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid, rgoldwyn
In-Reply-To: <55419EC8.4030407@suse.com>
[-- Attachment #1: Type: text/plain, Size: 2395 bytes --]
On Thu, 30 Apr 2015 11:17:28 +0800 Guoqing Jiang <gqJiang@suse.com> wrote:
> NeilBrown wrote:
> > On Fri, 24 Apr 2015 15:30:35 +0800 gqjiang@suse.com wrote:
> >
> >
> >> From: Guoqing Jiang <gqjiang@suse.com>
> >>
> >> This adds capability of exmining bitmaps corresponding to all
> >> nodes/slots on the device.
> >>
> >> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> >> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> >> ---
> >> bitmap.c | 35 ++++++++++++++++++++++++++---------
> >> 1 file changed, 26 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/bitmap.c b/bitmap.c
> >> index b1d54a6..ab83f4e 100644
> >> --- a/bitmap.c
> >> +++ b/bitmap.c
> >> @@ -258,7 +258,7 @@ int ExamineBitmap(char *filename, int brief, struct supertype *st)
> >> int rv = 1;
> >> char buf[64];
> >> int swap;
> >> - int fd;
> >> + int fd, i;
> >> __u32 uuid32[4];
> >>
> >> fd = bitmap_file_open(filename, &st);
> >> @@ -315,9 +315,6 @@ int ExamineBitmap(char *filename, int brief, struct supertype *st)
> >> uuid32[2],
> >> uuid32[3]);
> >>
> >> - printf(" Events : %llu\n", (unsigned long long)sb->events);
> >> - printf(" Events Cleared : %llu\n", (unsigned long long)sb->events_cleared);
> >> - printf(" State : %s\n", bitmap_state(sb->state));
> >>
> >
> > Can we please leave this where it is in the case where there is only one node?
> > Only move it down if there are multiple nodes to report on.
> >
> >
> Yes, it could be possible. Just to make sure understand you correctly,
> what about the below?
>
> if (sb->nodes == 0) {
> /* code for more than one node */
> } else {
> /* original code */
> }
>
Something a bit like that, though !=0 of course.
The original code put some info at the top. When there are multiple nodes
that makes more sense at the bottom.
So:
if (sb->nodes == 0) {
original code that will be moved
}
more original code that isn't changing
if (sb->nodes == 0) {
other original code
} else {
new code
}
or something like that.
Thanks,
NeilBrown
> Thanks,
> Guoqing
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 09/10] mdadm: change the num of cluster node
From: Guoqing Jiang @ 2015-04-30 3:34 UTC (permalink / raw)
To: NeilBrown; +Cc: gqjiang, linux-raid, rgoldwyn
In-Reply-To: <20150429115151.0763e980@notabene.brown>
NeilBrown wrote:
> On Fri, 24 Apr 2015 15:30:40 +0800 gqjiang@suse.com wrote:
>
>
>> From: Guoqing Jiang <gqjiang@suse.com>
>>
>> This extends nodes option for assemble mode, make the num of
>> cluster node could be change by user.
>>
>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>> ---
>> Assemble.c | 4 ++++
>> ReadMe.c | 2 +-
>> mdadm.c | 3 +++
>> mdadm.h | 1 +
>> super1.c | 6 ++++++
>> 5 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/Assemble.c b/Assemble.c
>> index e1b846c..22042a9 100644
>> --- a/Assemble.c
>> +++ b/Assemble.c
>> @@ -649,6 +649,10 @@ static int load_devices(struct devs *devices, char *devmap,
>> devname, 0, 0, c->homecluster);
>> tst->ss->write_bitmap(tst, dfd, NameUpdate);
>> }
>> + if (strcmp(c->update, "nodes") == 0) {
>> + tst->nodes = c->nodes;
>> + tst->ss->write_bitmap(tst, dfd, NodeNumUpdate);
>> + }
>>
>
> Doesn't there need to be some test that there is enough free space on all
> devices to store the extra bitmaps (when nodes is increasing)??
>
>
Agree, could you pls elaborate more about the test? I guess the test
need to be run
before write_bitmap1call the awrite(&afd, buf, n).
Thanks,
Guoqing
^ permalink raw reply
* Re: [PATCH 08/10] mdadm: add the ability to change cluster name
From: Guoqing Jiang @ 2015-04-30 3:22 UTC (permalink / raw)
To: NeilBrown; +Cc: gqjiang, linux-raid, rgoldwyn
In-Reply-To: <20150429115033.1191a357@notabene.brown>
NeilBrown wrote:
> On Fri, 24 Apr 2015 15:30:39 +0800 gqjiang@suse.com wrote:
>
>
>> --- a/super1.c
>> +++ b/super1.c
>> @@ -1073,6 +1073,25 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
>> info->name[32] = 0;
>> }
>>
>> + if (strcmp(update, "home-cluster") == 0 &&
>> + homehost) {
>> + /* Note that 'home-cluster' is to change the name of cluster,
>> + * it is another "name" update.
>> + */
>> + char *new_name = xmalloc(sizeof(sb->set_name));
>> + if (strrchr(sb->set_name, ':')) {
>> + strcpy(new_name, strchr(sb->set_name, ':'));
>> + }
>> +
>> + memset(sb->set_name, 0, sizeof(sb->set_name));
>> + strcpy(sb->set_name, homehost);
>> + if (new_name)
>> + strcat(sb->set_name, new_name);
>> +
>> + free(new_name);
>> + goto out;
>> + }
>> +
>>
>
> Please get rid of the 'goto out' and put an 'else' in here.
>
> "homehost" is special because it translates to "name".
> "home-cluster" is not special.
>
>
Got it.
>
>> if (strcmp(update, "force-one")==0) {
>> /* Not enough devices for a working array,
>> * so bring this one up-to-date
>> @@ -1313,6 +1332,7 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
>> else
>> rv = -1;
>>
>> +out:
>> sb->sb_csum = calc_sb_1_csum(sb);
>> return rv;
>> }
>> @@ -1691,7 +1711,7 @@ static int write_init_super1(struct supertype *st)
>> sb->sb_csum = calc_sb_1_csum(sb);
>> rv = store_super1(st, di->fd);
>> if (rv == 0 && (__le32_to_cpu(sb->feature_map) & 1))
>> - rv = st->ss->write_bitmap(st, di->fd);
>> + rv = st->ss->write_bitmap(st, di->fd, NoUpdate);
>> close(di->fd);
>> di->fd = -1;
>> if (rv)
>> @@ -2175,7 +2195,7 @@ static void locate_bitmap1(struct supertype *st, int fd)
>> lseek64(fd, offset<<9, 0);
>> }
>>
>> -static int write_bitmap1(struct supertype *st, int fd)
>> +static int write_bitmap1(struct supertype *st, int fd, enum bitmap_update update)
>> {
>> struct mdp_superblock_1 *sb = st->sb;
>> bitmap_super_t *bms = (bitmap_super_t*)(((char*)sb)+MAX_SB_SIZE);
>> @@ -2185,6 +2205,28 @@ static int write_bitmap1(struct supertype *st, int fd)
>> struct align_fd afd;
>> unsigned int i;
>>
>> + switch (update) {
>> + case NameUpdate:
>> + {
>> + char *new_name = xmalloc(sizeof(sb->set_name));
>> +
>> + strncpy(new_name, sb->set_name, sizeof(sb->set_name));
>> + memset((char *)bms->cluster_name, 0, sizeof(bms->cluster_name));
>> +
>> + if (strtok(new_name, ":"))
>> + strncpy((char *)bms->cluster_name, new_name, strlen(sb->set_name));
>> + else
>> + /* In case the original set_name doesn't like aaa:md* */
>> + strncpy((char *)bms->cluster_name, sb->set_name, strlen(sb->set_name));
>> +
>> + free(new_name);
>> + break;
>> + }
>>
>
> I don't like the braces there - too confusing.
> Just make 'new_name' a top-level variable and get rid of the {}
>
>
No problem, will do it.
Thanks,
Guoqing
^ permalink raw reply
* Re: [PATCH 05/10] Add a new clustered disk
From: Guoqing Jiang @ 2015-04-30 3:20 UTC (permalink / raw)
To: NeilBrown; +Cc: gqjiang, linux-raid, rgoldwyn
In-Reply-To: <20150429114532.568d58bd@notabene.brown>
NeilBrown wrote:
> On Fri, 24 Apr 2015 15:30:36 +0800 gqjiang@suse.com wrote:
>
>
>> From: Guoqing Jiang <gqjiang@suse.com>
>>
>> A clustered disk is added by the traditional --add sequence.
>> However, other nodes need to acknowledge that they can "see"
>> the device. This is done by --cluster-confirm:
>>
>> --cluster-confirm Y:/dev/whatever (if disk is found)
>> or
>> --cluster-confirm Y:missing (if disk is not found)
>>
>> The node initiating the --add, has the disk state tagged with
>> MD_DISK_CLUSTER_ADD and the one confirming tag the disk with
>> MD_DISK_CANDIDATE.
>>
>
> You haven't explained 'Y' here. It looks like it means 'Yes', but it doesn't.
>
>
>
Right, actually 'Y' stands for the slot, will modify it.
>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>>
>>
[snip]
>>
>> +int parse_cluster_confirm_arg(char *input, char **devname, int *slot)
>> +{
>> + char *dev;
>> + *slot = strtoul(input, &dev, 10);
>> + if (dev[0] == ':')
>> + *devname = dev+1;
>> + else
>> + return -1;
>> + return 0;
>> +}
>>
>
> The logic here hurts my brain :-(
>
> *slot = strtoul(input, &dev, 10);
> if (dev == input || dev[0] != ':')
> return -1;
> *devname = dev+1;
> return 0;
>
>
Thanks for above, :)
Regards,
Guoqing
^ permalink raw reply
* Re: [PATCH 04/10] Show all bitmaps while examining bitmap
From: Guoqing Jiang @ 2015-04-30 3:17 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, rgoldwyn
In-Reply-To: <20150429114140.07b20a08@notabene.brown>
NeilBrown wrote:
> On Fri, 24 Apr 2015 15:30:35 +0800 gqjiang@suse.com wrote:
>
>
>> From: Guoqing Jiang <gqjiang@suse.com>
>>
>> This adds capability of exmining bitmaps corresponding to all
>> nodes/slots on the device.
>>
>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>> ---
>> bitmap.c | 35 ++++++++++++++++++++++++++---------
>> 1 file changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/bitmap.c b/bitmap.c
>> index b1d54a6..ab83f4e 100644
>> --- a/bitmap.c
>> +++ b/bitmap.c
>> @@ -258,7 +258,7 @@ int ExamineBitmap(char *filename, int brief, struct supertype *st)
>> int rv = 1;
>> char buf[64];
>> int swap;
>> - int fd;
>> + int fd, i;
>> __u32 uuid32[4];
>>
>> fd = bitmap_file_open(filename, &st);
>> @@ -315,9 +315,6 @@ int ExamineBitmap(char *filename, int brief, struct supertype *st)
>> uuid32[2],
>> uuid32[3]);
>>
>> - printf(" Events : %llu\n", (unsigned long long)sb->events);
>> - printf(" Events Cleared : %llu\n", (unsigned long long)sb->events_cleared);
>> - printf(" State : %s\n", bitmap_state(sb->state));
>>
>
> Can we please leave this where it is in the case where there is only one node?
> Only move it down if there are multiple nodes to report on.
>
>
Yes, it could be possible. Just to make sure understand you correctly,
what about the below?
if (sb->nodes == 0) {
/* code for more than one node */
} else {
/* original code */
}
Thanks,
Guoqing
^ permalink raw reply
* Re: [PATCH 03/10] Create n bitmaps for clustered mode
From: NeilBrown @ 2015-04-30 2:53 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid, rgoldwyn
In-Reply-To: <55419707.9050302@suse.com>
[-- Attachment #1: Type: text/plain, Size: 3608 bytes --]
On Thu, 30 Apr 2015 10:44:23 +0800 Guoqing Jiang <gqJiang@suse.com> wrote:
> NeilBrown wrote:
> > On Fri, 24 Apr 2015 15:30:34 +0800 gqjiang@suse.com wrote:
> >
> >
> >> From: Guoqing Jiang <gqjiang@suse.com>
> >>
> >> For a clustered MD, create bitmaps equal to number of nodes so
> >> each node has an independent bitmap.
> >>
> >> Only the first bitmap is has the bits set so that the first node
> >> that assembles the device also performs the sync.
> >>
> >> The bitmaps are aligned to 4k boundaries.
> >>
> >> On-disk format:
> >>
> >> 0 4k 8k 12k
> >> -------------------------------------------------------------------
> >> | idle | md super | bm super [0] + bits |
> >> | bm bits[0, contd] | bm super[1] + bits | bm bits[1, contd] |
> >> | bm super[2] + bits | bm bits [2, contd] | bm super[3] + bits |
> >> | bm bits [3, contd] | | |
> >>
> >> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> >> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> >> ---
> >> Create.c | 3 ++-
> >> bitmap.h | 7 +++++--
> >> mdadm.8.in | 7 ++++++-
> >> mdadm.c | 17 ++++++++++++++++-
> >> super1.c | 59 +++++++++++++++++++++++++++++++++++++++++------------------
> >> 5 files changed, 70 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/Create.c b/Create.c
> >> index cd5485b..9663dc4 100644
> >> --- a/Create.c
> >> +++ b/Create.c
> >> @@ -752,7 +752,8 @@ int Create(struct supertype *st, char *mddev,
> >> #endif
> >> }
> >>
> >> - if (s->bitmap_file && strcmp(s->bitmap_file, "internal")==0) {
> >> + if (s->bitmap_file && (strcmp(s->bitmap_file, "internal")==0
> >> + || strcmp(s->bitmap_file, "clustered")==0)) {
> >> if ((vers%100) < 2) {
> >> pr_err("internal bitmaps not supported by this kernel.\n");
> >> goto abort_locked;
> >> diff --git a/bitmap.h b/bitmap.h
> >> index c8725a3..adbf0b4 100644
> >> --- a/bitmap.h
> >> +++ b/bitmap.h
> >> @@ -154,8 +154,11 @@ typedef struct bitmap_super_s {
> >> __u32 chunksize; /* 52 the bitmap chunk size in bytes */
> >> __u32 daemon_sleep; /* 56 seconds between disk flushes */
> >> __u32 write_behind; /* 60 number of outstanding write-behind writes */
> >> -
> >> - __u8 pad[256 - 64]; /* set to zero */
> >> + __u32 sectors_reserved; /* 64 number of 512-byte sectors that are
> >> + * reserved for the bitmap. */
> >> + __u32 nodes; /* 68 the maximum number of nodes in cluster. */
> >> + __u8 cluster_name[64]; /* 72 cluster name to which this md belongs */
> >> + __u8 pad[256 - 136]; /* set to zero */
> >> } bitmap_super_t;
> >>
> >
> > I missed this the first time, but these fields that you have added need to be
> > added to sb_le_to_cpu().
> >
> >
> I guess only nodes need to be added as follows:
> sb->nodes = __le32_to_cpu(sb->nodes);
Why not "sectors_reserved". It may not be used, but best to be consistent.
>
> Does the cluster_name need to be addressed too? Like.
> for (i = 0; i < 64; i++)
> sb->cluster_name[i] = (unsigned
> char)__le16_to_cpu(sb->cluster_name[i]);
No, cluster_name is __u8, and they aren't affected by CPU endian-ness.
Thanks,
NeilBrown
>
> Thanks,
> Guoqing
> > NeilBrown
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 03/10] Create n bitmaps for clustered mode
From: NeilBrown @ 2015-04-30 2:51 UTC (permalink / raw)
To: Goldwyn Rodrigues; +Cc: gqjiang, linux-raid
In-Reply-To: <554044EB.5050303@suse.de>
[-- Attachment #1: Type: text/plain, Size: 7995 bytes --]
On Tue, 28 Apr 2015 21:41:47 -0500 Goldwyn Rodrigues <rgoldwyn@suse.de> wrote:
>
>
> On 04/28/2015 08:36 PM, NeilBrown wrote:
> > On Fri, 24 Apr 2015 15:30:34 +0800 gqjiang@suse.com wrote:
> >
> >> From: Guoqing Jiang <gqjiang@suse.com>
> >>
> >> For a clustered MD, create bitmaps equal to number of nodes so
> >> each node has an independent bitmap.
> >>
> >> Only the first bitmap is has the bits set so that the first node
> >> that assembles the device also performs the sync.
> >>
> >> The bitmaps are aligned to 4k boundaries.
> >>
> >> On-disk format:
> >>
> >> 0 4k 8k 12k
> >> -------------------------------------------------------------------
> >> | idle | md super | bm super [0] + bits |
> >> | bm bits[0, contd] | bm super[1] + bits | bm bits[1, contd] |
> >> | bm super[2] + bits | bm bits [2, contd] | bm super[3] + bits |
> >> | bm bits [3, contd] | | |
> >>
> >> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> >> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> >> ---
> >> Create.c | 3 ++-
> >> bitmap.h | 7 +++++--
> >> mdadm.8.in | 7 ++++++-
> >> mdadm.c | 17 ++++++++++++++++-
> >> super1.c | 59 +++++++++++++++++++++++++++++++++++++++++------------------
> >> 5 files changed, 70 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/Create.c b/Create.c
> >> index cd5485b..9663dc4 100644
> >> --- a/Create.c
> >> +++ b/Create.c
> >> @@ -752,7 +752,8 @@ int Create(struct supertype *st, char *mddev,
> >> #endif
> >> }
> >>
> >> - if (s->bitmap_file && strcmp(s->bitmap_file, "internal")==0) {
> >> + if (s->bitmap_file && (strcmp(s->bitmap_file, "internal")==0
> >> + || strcmp(s->bitmap_file, "clustered")==0)) {
> >> if ((vers%100) < 2) {
> >> pr_err("internal bitmaps not supported by this kernel.\n");
> >> goto abort_locked;
> >> diff --git a/bitmap.h b/bitmap.h
> >> index c8725a3..adbf0b4 100644
> >> --- a/bitmap.h
> >> +++ b/bitmap.h
> >> @@ -154,8 +154,11 @@ typedef struct bitmap_super_s {
> >> __u32 chunksize; /* 52 the bitmap chunk size in bytes */
> >> __u32 daemon_sleep; /* 56 seconds between disk flushes */
> >> __u32 write_behind; /* 60 number of outstanding write-behind writes */
> >> -
> >> - __u8 pad[256 - 64]; /* set to zero */
> >> + __u32 sectors_reserved; /* 64 number of 512-byte sectors that are
> >> + * reserved for the bitmap. */
> >> + __u32 nodes; /* 68 the maximum number of nodes in cluster. */
> >> + __u8 cluster_name[64]; /* 72 cluster name to which this md belongs */
> >> + __u8 pad[256 - 136]; /* set to zero */
> >> } bitmap_super_t;
> >>
> >> /* notes:
> >> diff --git a/mdadm.8.in b/mdadm.8.in
> >> index a0e8288..c015cbf 100644
> >> --- a/mdadm.8.in
> >> +++ b/mdadm.8.in
> >> @@ -700,7 +700,12 @@ and so is replicated on all devices. If the word
> >> .B "none"
> >> is given with
> >> .B \-\-grow
> >> -mode, then any bitmap that is present is removed.
> >> +mode, then any bitmap that is present is removed. If the word
> >> +.B "clustered"
> >> +is given, the array is created for a clustered environment. One bitmap
> >> +is created for each node as defined by the
> >> +.B \-\-nodes
> >> +parameter and are stored internally.
> >>
> >> To help catch typing errors, the filename must contain at least one
> >> slash ('/') if it is a real file (not 'internal' or 'none').
> >> diff --git a/mdadm.c b/mdadm.c
> >> index e4f8568..6963a09 100644
> >> --- a/mdadm.c
> >> +++ b/mdadm.c
> >> @@ -1111,6 +1111,15 @@ int main(int argc, char *argv[])
> >> s.bitmap_file = optarg;
> >> continue;
> >> }
> >> + if (strcmp(optarg, "clustered")== 0) {
> >> + s.bitmap_file = optarg;
> >> + /* Set the default number of cluster nodes
> >> + * to 4 if not already set by user
> >> + */
> >> + if (c.nodes < 1)
> >> + c.nodes = 4;
> >> + continue;
> >> + }
> >> /* probable typo */
> >> pr_err("bitmap file must contain a '/', or be 'internal', or 'none'\n"
> >> " not '%s'\n", optarg);
> >> @@ -1404,7 +1413,13 @@ int main(int argc, char *argv[])
> >> if (c.delay == 0)
> >> c.delay = DEFAULT_BITMAP_DELAY;
> >>
> >> - if (!strncmp(s.bitmap_file, "internal", 9) ||
> >> + if (!strncmp(s.bitmap_file, "clustered", 9)) {
> >> + if (s.level != 1) {
> >> + pr_err("--bitmap=clustered is currently supported with RAID mirror only\n");
> >> + rv = 1;
> >> + break;
> >> + }
> >> + } else if (!strncmp(s.bitmap_file, "internal", 9) ||
> >> !strncmp(s.bitmap_file,"none", 4)) {
> >> if (c.nodes) {
> >> pr_err("--nodes argument is incompatible with --bitmap=%s.\n",
> >> diff --git a/super1.c b/super1.c
> >> index f0508fe..ac1b011 100644
> >> --- a/super1.c
> >> +++ b/super1.c
> >> @@ -2144,6 +2144,10 @@ add_internal_bitmap1(struct supertype *st,
> >> bms->daemon_sleep = __cpu_to_le32(delay);
> >> bms->sync_size = __cpu_to_le64(size);
> >> bms->write_behind = __cpu_to_le32(write_behind);
> >> + bms->nodes = __cpu_to_le32(st->nodes);
> >> + if (st->cluster_name)
> >> + strncpy((char *)bms->cluster_name,
> >> + st->cluster_name, strlen(st->cluster_name));
> >>
> >> *chunkp = chunk;
> >> return 1;
> >> @@ -2177,6 +2181,7 @@ static int write_bitmap1(struct supertype *st, int fd)
> >> void *buf;
> >> int towrite, n;
> >> struct align_fd afd;
> >> + unsigned int i;
> >>
> >> init_afd(&afd, fd);
> >>
> >> @@ -2185,27 +2190,45 @@ static int write_bitmap1(struct supertype *st, int fd)
> >> if (posix_memalign(&buf, 4096, 4096))
> >> return -ENOMEM;
> >>
> >> - memset(buf, 0xff, 4096);
> >> - memcpy(buf, (char *)bms, sizeof(bitmap_super_t));
> >> -
> >> - towrite = __le64_to_cpu(bms->sync_size) / (__le32_to_cpu(bms->chunksize)>>9);
> >> - towrite = (towrite+7) >> 3; /* bits to bytes */
> >> - towrite += sizeof(bitmap_super_t);
> >> - towrite = ROUND_UP(towrite, 512);
> >> - while (towrite > 0) {
> >> - n = towrite;
> >> - if (n > 4096)
> >> - n = 4096;
> >> - n = awrite(&afd, buf, n);
> >> - if (n > 0)
> >> - towrite -= n;
> >> + /* We use bms->nodes as opposed to st->nodes to
> >> + * be compatible with write-after-reads such as
> >> + * the GROW operation.
> >> + */
> >> + for (i = 0; i < __le32_to_cpu(bms->nodes); i++) {
> >> + /* Only the first bitmap should resync
> >> + * the whole device
> >> + */
> >> + if (i)
> >> + memset(buf, 0x00, 4096);
> >> else
> >> + memset(buf, 0xff, 4096);
> >
> > Why is the first bitmap initialised to 0x00 and the others to 0xff?
> > If there is a good reason it should be documented either in a comment in the
> > code or in the changelog entry.
>
>
> Rather, it is the reverse. The first one is initialized to 0xff and the
> rest are set to 0x00.
>
> The reason is only the first node to assemble the device should perform
> the resync (if --assume-clean is not provided). The comment is right
> above the code. Perhaps I should be more elaborate with the comment.
>
>
Hmmm... Perhaps I should read code with my eyes open!
Not sure I agree though. Why should the first node be special?
What if node '0' doesn't get activated?
I guess it always well because of the way numbers are assigned, but I'm not
feeling very comfortable...
Thinking a bit more ... why do we set any bits to '1'?
Why not just set BITMAP_STALE, and let the kernel figure things out.
For the single-node case, BITMAP_STALE is the same as setting all the bits to
one.
For the cluster case, we can get BITMAP_STALE to do whatever we want. and we
should make sure we handle it correctly anyway.
So maybe mdadm should set BITMAP_STALE, and leave all the bits as 0.
Thoughts?
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 03/10] Create n bitmaps for clustered mode
From: Guoqing Jiang @ 2015-04-30 2:44 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, rgoldwyn
In-Reply-To: <20150429114115.2265643f@notabene.brown>
NeilBrown wrote:
> On Fri, 24 Apr 2015 15:30:34 +0800 gqjiang@suse.com wrote:
>
>
>> From: Guoqing Jiang <gqjiang@suse.com>
>>
>> For a clustered MD, create bitmaps equal to number of nodes so
>> each node has an independent bitmap.
>>
>> Only the first bitmap is has the bits set so that the first node
>> that assembles the device also performs the sync.
>>
>> The bitmaps are aligned to 4k boundaries.
>>
>> On-disk format:
>>
>> 0 4k 8k 12k
>> -------------------------------------------------------------------
>> | idle | md super | bm super [0] + bits |
>> | bm bits[0, contd] | bm super[1] + bits | bm bits[1, contd] |
>> | bm super[2] + bits | bm bits [2, contd] | bm super[3] + bits |
>> | bm bits [3, contd] | | |
>>
>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>> ---
>> Create.c | 3 ++-
>> bitmap.h | 7 +++++--
>> mdadm.8.in | 7 ++++++-
>> mdadm.c | 17 ++++++++++++++++-
>> super1.c | 59 +++++++++++++++++++++++++++++++++++++++++------------------
>> 5 files changed, 70 insertions(+), 23 deletions(-)
>>
>> diff --git a/Create.c b/Create.c
>> index cd5485b..9663dc4 100644
>> --- a/Create.c
>> +++ b/Create.c
>> @@ -752,7 +752,8 @@ int Create(struct supertype *st, char *mddev,
>> #endif
>> }
>>
>> - if (s->bitmap_file && strcmp(s->bitmap_file, "internal")==0) {
>> + if (s->bitmap_file && (strcmp(s->bitmap_file, "internal")==0
>> + || strcmp(s->bitmap_file, "clustered")==0)) {
>> if ((vers%100) < 2) {
>> pr_err("internal bitmaps not supported by this kernel.\n");
>> goto abort_locked;
>> diff --git a/bitmap.h b/bitmap.h
>> index c8725a3..adbf0b4 100644
>> --- a/bitmap.h
>> +++ b/bitmap.h
>> @@ -154,8 +154,11 @@ typedef struct bitmap_super_s {
>> __u32 chunksize; /* 52 the bitmap chunk size in bytes */
>> __u32 daemon_sleep; /* 56 seconds between disk flushes */
>> __u32 write_behind; /* 60 number of outstanding write-behind writes */
>> -
>> - __u8 pad[256 - 64]; /* set to zero */
>> + __u32 sectors_reserved; /* 64 number of 512-byte sectors that are
>> + * reserved for the bitmap. */
>> + __u32 nodes; /* 68 the maximum number of nodes in cluster. */
>> + __u8 cluster_name[64]; /* 72 cluster name to which this md belongs */
>> + __u8 pad[256 - 136]; /* set to zero */
>> } bitmap_super_t;
>>
>
> I missed this the first time, but these fields that you have added need to be
> added to sb_le_to_cpu().
>
>
I guess only nodes need to be added as follows:
sb->nodes = __le32_to_cpu(sb->nodes);
Does the cluster_name need to be addressed too? Like.
for (i = 0; i < 64; i++)
sb->cluster_name[i] = (unsigned
char)__le16_to_cpu(sb->cluster_name[i]);
Thanks,
Guoqing
> NeilBrown
>
^ permalink raw reply
* Re: [PATCH 01/10] Add nodes option while creating md
From: Guoqing Jiang @ 2015-04-30 2:33 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, rgoldwyn
In-Reply-To: <20150429113008.253c02f7@notabene.brown>
NeilBrown wrote:
> On Fri, 24 Apr 2015 15:30:32 +0800 gqjiang@suse.com wrote:
>
>
>> From: Guoqing Jiang <gqjiang@suse.com>
>>
>> Specifies the maximum number of nodes in the cluster that may use
>> this device simultaneously. This is equivalent to the number of
>> bitmaps created in the internal superblock (patches to follow).
>>
>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>>
>
> This doesn't really make much sense coming first in the series. It sets up a
> value that is never used.
>
> I would much rather 03/10 came first - which would just create 4 bitmaps.
> Then have this patch to allow that '4' to be changed.
>
>
>
Ok, I will re-arrange them, make 03 first, and keep the others with
original sequence.
>> ---
>> Create.c | 1 +
>> ReadMe.c | 1 +
>> mdadm.8.in | 5 +++++
>> mdadm.c | 20 +++++++++++++++++++-
>> mdadm.h | 3 +++
>> 5 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/Create.c b/Create.c
>> index ef28da0..b73f6cb 100644
>> --- a/Create.c
>> +++ b/Create.c
>> @@ -531,6 +531,7 @@ int Create(struct supertype *st, char *mddev,
>> st->ss->name);
>> warn = 1;
>> }
>> + st->nodes = c->nodes;
>>
>> if (warn) {
>> if (c->runstop!= 1) {
>> diff --git a/ReadMe.c b/ReadMe.c
>> index 87a4916..30c569d 100644
>> --- a/ReadMe.c
>> +++ b/ReadMe.c
>> @@ -140,6 +140,7 @@ struct option long_options[] = {
>> {"homehost", 1, 0, HomeHost},
>> {"symlinks", 1, 0, Symlinks},
>> {"data-offset",1, 0, DataOffset},
>> + {"nodes",1, 0, Nodes},
>>
>> /* For assemble */
>> {"uuid", 1, 0, 'u'},
>> diff --git a/mdadm.8.in b/mdadm.8.in
>> index a630310..bd8d59e 100644
>> --- a/mdadm.8.in
>> +++ b/mdadm.8.in
>> @@ -966,6 +966,11 @@ However for RAID0, it is not possible to add spares. So to increase
>> the number of devices in a RAID0, it is necessary to set the new
>> number of devices, and to add the new devices, in the same command.
>>
>> +.TP
>> +.BR \-\-nodes
>> +Specify the maximum number of nodes in the cluster that will use this
>> +device simultaneously. If not specified, this defaults to 4.
>> +
>>
>
> I think this should mention that it only makes sense with --bitmap=cluster
> (which of course isn't available at this point....)
>
>
>
Will add it.
>> .SH For assemble:
>>
>> .TP
>> diff --git a/mdadm.c b/mdadm.c
>> index 3e8c49b..bce6a76 100644
>> --- a/mdadm.c
>> +++ b/mdadm.c
>> @@ -588,7 +588,14 @@ int main(int argc, char *argv[])
>> }
>> ident.raid_disks = s.raiddisks;
>> continue;
>> -
>> + case O(CREATE, Nodes):
>> + c.nodes = parse_num(optarg);
>> + if (c.nodes <= 0) {
>> + pr_err("invalid number for the number of "
>> + "cluster nodes: %s\n", optarg);
>> + exit(2);
>> + }
>> + continue;
>> case O(CREATE,'x'): /* number of spare (eXtra) disks */
>> if (s.sparedisks) {
>> pr_err("spare-devices set twice: %d and %s\n",
>> @@ -1377,6 +1384,17 @@ int main(int argc, char *argv[])
>> case CREATE:
>> if (c.delay == 0)
>> c.delay = DEFAULT_BITMAP_DELAY;
>> +
>> + if (!strncmp(s.bitmap_file, "internal", 9) ||
>> + !strncmp(s.bitmap_file,"none", 4)) {
>>
>
> I'm sorry but I absolutely *hate* this construct.
> The '!' at the front makes it seem like you are testing that the string does
> *not* have that value, but it is exactly the reverse.
> And why "strncmp" rather than "strcmp" ??
>
> Please use
>
> if (strcmp(s.bitmap_file, "internal") == 0 ||
> strcmp(s.bitmap_file, "none) == 0) {
>
> except.... what about if bitmap_file in /path/to/somewhere. Presumably you
> want to exclude that case too.
> May be
> if (strcmp(s.bitmap_file, "cluster") != 0) {
> ??
>
Thanks, which is better for understanding.
Regards,
Guoqing
^ permalink raw reply
* Re: BUG?: RAID6 reshape hung in reshape_request
From: David Wahler @ 2015-04-30 0:33 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20150429100339.5b0cf4f3@notabene.brown>
On Tue, Apr 28, 2015 at 7:03 PM, NeilBrown <neilb@suse.de> wrote:
> On Mon, 27 Apr 2015 12:20:50 -0500 David Wahler <dwahler@gmail.com> wrote:
>>
>> I don't urgently need this array up and running, so I'm happy to leave
>> it in its current state for the next few days in case there's anything
>> else I can do to help track this down.
>
> Thanks for the various status data.
>
> I'm fairly easily able to reproduce the problem. I clearly never thought
> about 'reshape' when I was writing the bad_block handling.
>
> You can allow the reshape to complete by the following hack:
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 77dfd720aaa0..e6c68a450d4c 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -4306,7 +4306,7 @@ static void handle_stripe(struct stripe_head *sh)
> */
> if (s.failed > conf->max_degraded) {
> sh->check_state = 0;
> - sh->reconstruct_state = 0;
> +// sh->reconstruct_state = 0;
> if (s.to_read+s.to_write+s.written)
> handle_failed_stripe(conf, sh, &s, disks, &s.return_bi);
> if (s.syncing + s.replacing)
>
> It may not necessarily do exactly the right thing, but it won't be too bad.
Yep, worked like a charm.
Running fsck afterwards found a dozen or so corrupted inodes. I'm not
sure whether that's because of the initial read failure that caused
the blocks to be marked as bad, or if it was aggravated by me
repeatedly interrupting the reshape.
Thanks again for the assistance.
-- David
^ permalink raw reply
* [PATCH stable] block: discard bdi_unregister() in favour of bdi_destroy()
From: NeilBrown @ 2015-04-30 0:32 UTC (permalink / raw)
To: Jens Axboe
Cc: Peter Zijlstra, Christoph Hellwig, Mike Snitzer, Azat Khuzhin,
Kernel.org-Linux-RAID, Guoqing Jiang, Tejun Heo, Jan Kara, lkml,
device-mapper development
In-Reply-To: <20150429160258.GK17717@twins.programming.kicks-ass.net>
[-- Attachment #1: Type: text/plain, Size: 4448 bytes --]
bdi_unregister() now contains very little functionality.
It contains a "WARN_ON" if bdi->dev is NULL. This warning is of no
real consequence as bdi->dev isn't needed by anything else in the function,
and it triggers if
blk_cleanup_queue() -> bdi_destroy()
is called before bdi_unregister, which a subsequent patch will make happen.
So this isn't wanted.
It also calls bdi_set_min_ratio(). This needs to be called after
writes through the bdi have all been flushed, and before the bdi is destroyed.
Calling it early is better than calling it late as it frees up a global
resource.
Calling it immediately after bdi_wb_shutdown() in bdi_destroy()
perfectly fits these requirements.
So bdi_unregister can be discarded with the important content moved to
bdi_destroy, as can the
writeback_bdi_unregister
event which is already not used.
This is tagged for 'stable' as it is a pre-requisite for a subsequent
patch which moves calls to blk_cleanup_queue() before calls to
del_gendisk(). The commit identified as 'Fixes' removed a lot of
other functionality from bdi_unregister(), and made a change which
necessitated moving the blk_cleanup_queue() calls.
Reported-by: Mike Snitzer <snitzer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: stable@vger.kernel.org (v4.0)
Fixes: c4db59d31e39ea067c32163ac961e9c80198fd37
Signed-off-by: NeilBrown <neilb@suse.de>
---
Hi again Jens,
would you be able to queue this patch *before* the other one:
block: destroy bdi before blockdev is unregistered.
If it has to come after I'll need to re-write the text a bit.
If you could give me the commit hash to reference I'll do that.
diff --git a/block/genhd.c b/block/genhd.c
index e351fc521053..1d4435478e8a 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -657,7 +657,6 @@ void del_gendisk(struct gendisk *disk)
disk->flags &= ~GENHD_FL_UP;
sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi");
- bdi_unregister(&disk->queue->backing_dev_info);
blk_unregister_queue(disk);
blk_unregister_region(disk_devt(disk), disk->minors);
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index aff923ae8c4b..d87d8eced064 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -116,7 +116,6 @@ __printf(3, 4)
int bdi_register(struct backing_dev_info *bdi, struct device *parent,
const char *fmt, ...);
int bdi_register_dev(struct backing_dev_info *bdi, dev_t dev);
-void bdi_unregister(struct backing_dev_info *bdi);
int __must_check bdi_setup_and_register(struct backing_dev_info *, char *);
void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages,
enum wb_reason reason);
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 880dd7437172..c178d13d6f4c 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -250,7 +250,6 @@ DEFINE_EVENT(writeback_class, name, \
DEFINE_WRITEBACK_EVENT(writeback_nowork);
DEFINE_WRITEBACK_EVENT(writeback_wake_background);
DEFINE_WRITEBACK_EVENT(writeback_bdi_register);
-DEFINE_WRITEBACK_EVENT(writeback_bdi_unregister);
DECLARE_EVENT_CLASS(wbc_class,
TP_PROTO(struct writeback_control *wbc, struct backing_dev_info *bdi),
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 6dc4580df2af..000e7b3b9896 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -359,23 +359,6 @@ static void bdi_wb_shutdown(struct backing_dev_info *bdi)
flush_delayed_work(&bdi->wb.dwork);
}
-/*
- * Called when the device behind @bdi has been removed or ejected.
- *
- * We can't really do much here except for reducing the dirty ratio at
- * the moment. In the future we should be able to set a flag so that
- * the filesystem can handle errors at mark_inode_dirty time instead
- * of only at writeback time.
- */
-void bdi_unregister(struct backing_dev_info *bdi)
-{
- if (WARN_ON_ONCE(!bdi->dev))
- return;
-
- bdi_set_min_ratio(bdi, 0);
-}
-EXPORT_SYMBOL(bdi_unregister);
-
static void bdi_wb_init(struct bdi_writeback *wb, struct backing_dev_info *bdi)
{
memset(wb, 0, sizeof(*wb));
@@ -443,6 +426,7 @@ void bdi_destroy(struct backing_dev_info *bdi)
int i;
bdi_wb_shutdown(bdi);
+ bdi_set_min_ratio(bdi, 0);
WARN_ON(!list_empty(&bdi->work_list));
WARN_ON(delayed_work_pending(&bdi->wb.dwork));
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related
* Re: [PATCH -stable] block: destroy bdi before blockdev is unregistered.
From: NeilBrown @ 2015-04-30 0:06 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Christoph Hellwig, Mike Snitzer, Jens Axboe, Azat Khuzhin,
Kernel.org-Linux-RAID, Guoqing Jiang, Tejun Heo, Jan Kara, lkml,
device-mapper development
In-Reply-To: <20150429160258.GK17717@twins.programming.kicks-ass.net>
[-- Attachment #1: Type: text/plain, Size: 2458 bytes --]
On Wed, 29 Apr 2015 18:02:58 +0200 Peter Zijlstra <peterz@infradead.org>
wrote:
> On Wed, Apr 29, 2015 at 03:35:12PM +0200, Christoph Hellwig wrote:
> > On Wed, Apr 29, 2015 at 07:25:30AM +1000, NeilBrown wrote:
> > > As bdi_set_min_ratio doesn't touch bdi->dev, there seems to be no need for
> > > the test, or the warning.
> > >
> > > I wonder if it would make sense to move the bdi_set_min_ratio() call to
> > > bdi_destroy, and discard bdi_unregister??
> > > There is a comment which suggests bdi_unregister might be of use later, but
> > > it might be best to have a clean slate in which to add whatever might be
> > > needed??
> >
> > This seems fine to me from the block dev point of view. I don't really
> > understand the bdi_min_ratio logic, but Peter might have a better idea.
>
> Ah, that was a bit of digging, I've not looked at that in ages :-)
>
> So if you look at bdi_dirty_limit()'s comment:
>
> * The bdi's share of dirty limit will be adapting to its throughput and
> * bounded by the bdi->min_ratio and/or bdi->max_ratio parameters, if set.
>
> So the min_ratio is a minimum guaranteed fraction of the total
> throughput.
>
> Now the problem before commit ccb6108f5b0b ("mm/backing-dev.c: reset bdi
> min_ratio in bdi_unregister()") was that since bdi_set_min_ratio()
> keeps a global sum of bdi->min_ratio, you need to subtract from said
> global sum when taking the BDI away. Otherwise we loose/leak a fraction
> of the total throughput available (to the other BDIs).
>
> Which is what that bdi_set_min_ratio(bdi, 0) in unregister does. It
> resets the min_ratio for the bdi being taken out and frees up the min
> allocated bandwidth for the others.
>
> So I think moving that do destroy would be fine; assuming the delay
> between unregister and destroy is typically 'short'. Because without
> that you can 'leak' this min ratio for extended periods which means the
> bandwidth is unavailable for other BDIs.
>
> Does that make sense?
Your assessment is almost exactly what I had come up with, so it definitely
makes sense :-)
'destroy' does come very shortly after 'unregister' (and immediately before
'blk_put_queue' which actually frees the struct). However the driving force
for this patch was a desire to move blk_cleanup_queue(), which calls
'destroy', earlier. So the net result is that bdi_set_min_ratio will be
called slightly sooner.
Thanks,
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: Help needed recovering from raid failure
From: NeilBrown @ 2015-04-29 23:27 UTC (permalink / raw)
To: Peter van Es; +Cc: linux-raid
In-Reply-To: <C17D8E8D-492C-4BDD-904C-75CCA70B2CD9@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5055 bytes --]
On Wed, 29 Apr 2015 20:17:09 +0200 Peter van Es <vanes.peter@gmail.com> wrote:
> Dear Neil,
>
> first of all, I really appreciate you trying to help me. This is the first time I’m deploying software raid, so really appreciate the guidance.
>
>
> > On 29 Apr 2015, at 00:26, NeilBrown <neilb@suse.de> wrote:
> >
> > This isn't really reporting anything new.
> > There is probably a daily cron job which reports all degraded arrays. This
> > message is reported by that job.
>
> I understand...
>
> >
> >
> > Why do you think the array is off-line? The above message doesn't suggest
> > that.
> >
>
> My Ubuntu server was accessible through ssh but did not serve webpages, files etc. When I went to the console,
> it told me it had taken the array offline because of degraded /dev/sdd2 and /dev/sdc2
> Those two drives were out of the array.
>
> >
> >>
> >> Needless to say, I can't boot the system anymore as the boot drive is /dev/md0, and GRUB can't
> >> get at it. I do need to recover data (I know, but there's stuf on there I have no backup for--yet).
> >
> > You boot off a RAID5? Does grub support that? I didn't know.
> > But md0 hasn't failed, has it?
> >
> > Confused.
>
> Well, it took a little time but yes, I managed to define a raid 5 array that the system was able to boot from.
>
> > There is something VERY sick here. I suggest that you tread very carefully.
> >
> > All your '1' partitions should be about 2GB and the '2' parititions about 2TB
> >
> > But the --examine output suggests sda2 and sdb2 are 2TB, while sdd2 and sde2
> > are 2GB.
> >
> > That really really shouldn't happen. Maybe check your partition table
> > (fdisk).
> > I really cannot see how this would happen.
>
> But this question, and the previous question you asked, tell me a little of what I may have done…
>
> I think confused /dev/md0 and /dev/md1 (now called /dev/md126 and /dev/md127 when running of the USB stick).
>
> /dev/md0 is a swap array (around 6GB, comprised of 4 x 2 GB in raid 5)
> /dev/md1 is the boot and data array (around 5 TB, comprised of 4 x ~2 TB in raid 5)
>
> I must have confused them and tried to add the /dev/sdc2 and /dev/sdd2 drive to the /dev/md0 array (mdadm —add /dev/md0 /dev/sdc2)
Oops!
> instead of to the /dev/md1 array. They were then added as spare drives, their superblocks were overwritten, but since
> a) no swap space was used, and
> b) they were added as spares
>
> The data should not have been overwritten.
Hopefully not.
>
> >
> > Can you
> > mdadm -Ss
> >
> > to stop all the arrays, then
> >
> > fdisk -l /dev/sd?
> >
> > then
> >
> > mdadm -Esvv
> >
>
> Neil, here they are: again, I appreciate you taking the time and guiding me through this!
>
> Is there any way to resurrect the super blocks and try to force assemble the array, skipping the failing drive /dev/sdd2 (the /dev/sdd2 drive created some errors I observed in the log, /dev/sdc2 must have had a one off issue to be taken out….). I have two new drives (arrived today), and a new SSD drive. I would want to get the new array assembled using /dev/sdc2 perhaps forcing it back to the array geometry and “hoping for the best” and then install a new /dev/sdd2 to be recovered. Then I’ll create a boot and swap drive off the SSD which means that any array failures should not prevent the system from booting…
As you have destroyed some metadata, it is no longer possible to 'assemble'
the array. We need to re-create it.
sda2 and sdb2 appear to be the first two drives of the array. sdd2 failed
first, so sdce is a better choice to use. It is probably reasonable to
assume that it was the fourth drive in the array. If that assumption proves
false then it might be the third.
Before doing this, double check that the names have changed, so check that
mdadm --examine /dev/sda2
shows
> Array UUID : 1f28f7bb:7b3ecd41:ca0fa5d1:ccd008df
> Device Role : Active device 0
(among other info) and that
mdadm --exmaine /dev/sdb2
show the same Array UUID and
> Device Role : Active device 1
Then run
mdadm -C /dev/md1 -l5 -n4 --data-offset=262144s --metadata=1.2 --assume-clean \
/dev/sda2 /dev/sdb2 missing /dev/sde2
Then
fsck -n -f /dev/md1
If the works, mount /dev/md1 and have a look around and confirm everything
looks OK.
If fsck complains, we might have sde2 in the wrong position. Or maybe sde
and sdd changed names.
run
mdadm -Ss
then rerun the -C command with a different list of devices. e.g.
/dev/sda2 /dev/sdb2 /dev/sde2 missing
Always have one 'missing' device or you will be very likely to get
out-of-sync data.
Once you have data that look OK, copy out any really really important stuff
then, if you think the 4th drive is reliable enough, or if you have replaced
it, add '2' partition of the fourth drive to the array and let it rebuild.
Then you should be back to a safe working array.
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3 v2] wait: introduce wait_event_exclusive_cmd
From: NeilBrown @ 2015-04-29 22:17 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Yuanhan Liu, linux-raid, linux-kernel, Ingo Molnar
In-Reply-To: <20150429093109.GS5029@twins.programming.kicks-ass.net>
[-- Attachment #1: Type: text/plain, Size: 2351 bytes --]
On Wed, 29 Apr 2015 11:31:09 +0200 Peter Zijlstra <peterz@infradead.org>
wrote:
> On Wed, Apr 29, 2015 at 10:48:53AM +0800, Yuanhan Liu wrote:
> > It's just a variant of wait_event_cmd(), with exclusive flag being set.
> >
> > For cases like RAID5, which puts many processes to sleep until 1/4
> > resources are free, a wake_up wakes up all processes to run, but
> > there is one process being able to get the resource as it's protected
> > by a spin lock. That ends up introducing heavy lock contentions, and
> > hurts performance badly.
> >
> > Here introduce wait_event_exclusive_cmd to relieve the lock contention
> > naturally by letting wake_up just wake up one process.
> >
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>
> Thanks!
>
> Assuming you want to route this through Neil's MD tree:
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Thanks a lot Peter!
I've queued the three patches in my md tree for 4.2.
Thanks yliu!
NeilBrown
>
> > ---
> > include/linux/wait.h | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/include/linux/wait.h b/include/linux/wait.h
> > index 2db8334..db78c72 100644
> > --- a/include/linux/wait.h
> > +++ b/include/linux/wait.h
> > @@ -358,6 +358,19 @@ do { \
> > __ret; \
> > })
> >
> > +#define __wait_event_exclusive_cmd(wq, condition, cmd1, cmd2) \
> > + (void)___wait_event(wq, condition, TASK_UNINTERRUPTIBLE, 1, 0, \
> > + cmd1; schedule(); cmd2)
> > +/*
> > + * Just like wait_event_cmd(), except it sets exclusive flag
> > + */
> > +#define wait_event_exclusive_cmd(wq, condition, cmd1, cmd2) \
> > +do { \
> > + if (condition) \
> > + break; \
> > + __wait_event_exclusive_cmd(wq, condition, cmd1, cmd2); \
> > +} while (0)
> > +
> > #define __wait_event_cmd(wq, condition, cmd1, cmd2) \
> > (void)___wait_event(wq, condition, TASK_UNINTERRUPTIBLE, 0, 0, \
> > cmd1; schedule(); cmd2)
> > --
> > 1.9.0
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: Help needed recovering from raid failure
From: Peter van Es @ 2015-04-29 18:17 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
Dear Neil,
first of all, I really appreciate you trying to help me. This is the first time I’m deploying software raid, so really appreciate the guidance.
> On 29 Apr 2015, at 00:26, NeilBrown <neilb@suse.de> wrote:
>
> This isn't really reporting anything new.
> There is probably a daily cron job which reports all degraded arrays. This
> message is reported by that job.
I understand...
>
>
> Why do you think the array is off-line? The above message doesn't suggest
> that.
>
My Ubuntu server was accessible through ssh but did not serve webpages, files etc. When I went to the console,
it told me it had taken the array offline because of degraded /dev/sdd2 and /dev/sdc2
Those two drives were out of the array.
>
>>
>> Needless to say, I can't boot the system anymore as the boot drive is /dev/md0, and GRUB can't
>> get at it. I do need to recover data (I know, but there's stuf on there I have no backup for--yet).
>
> You boot off a RAID5? Does grub support that? I didn't know.
> But md0 hasn't failed, has it?
>
> Confused.
Well, it took a little time but yes, I managed to define a raid 5 array that the system was able to boot from.
> There is something VERY sick here. I suggest that you tread very carefully.
>
> All your '1' partitions should be about 2GB and the '2' parititions about 2TB
>
> But the --examine output suggests sda2 and sdb2 are 2TB, while sdd2 and sde2
> are 2GB.
>
> That really really shouldn't happen. Maybe check your partition table
> (fdisk).
> I really cannot see how this would happen.
But this question, and the previous question you asked, tell me a little of what I may have done…
I think confused /dev/md0 and /dev/md1 (now called /dev/md126 and /dev/md127 when running of the USB stick).
/dev/md0 is a swap array (around 6GB, comprised of 4 x 2 GB in raid 5)
/dev/md1 is the boot and data array (around 5 TB, comprised of 4 x ~2 TB in raid 5)
I must have confused them and tried to add the /dev/sdc2 and /dev/sdd2 drive to the /dev/md0 array (mdadm —add /dev/md0 /dev/sdc2)
instead of to the /dev/md1 array. They were then added as spare drives, their superblocks were overwritten, but since
a) no swap space was used, and
b) they were added as spares
The data should not have been overwritten.
>
> Can you
> mdadm -Ss
>
> to stop all the arrays, then
>
> fdisk -l /dev/sd?
>
> then
>
> mdadm -Esvv
>
Neil, here they are: again, I appreciate you taking the time and guiding me through this!
Is there any way to resurrect the super blocks and try to force assemble the array, skipping the failing drive /dev/sdd2 (the /dev/sdd2 drive created some errors I observed in the log, /dev/sdc2 must have had a one off issue to be taken out….). I have two new drives (arrived today), and a new SSD drive. I would want to get the new array assembled using /dev/sdc2 perhaps forcing it back to the array geometry and “hoping for the best” and then install a new /dev/sdd2 to be recovered. Then I’ll create a boot and swap drive off the SSD which means that any array failures should not prevent the system from booting…
Requested outputs are below
Thanks,
Peter
fdisk output: (USB devices deleted)
Disk /dev/sda: 2000.4 GB, 2000398934016 bytes
255 heads, 63 sectors/track, 243201 cylinders, total 3907029168 sectors
Units = sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disk identifier: 0x000f24ee
Device Boot Start End Blocks Id System
/dev/sda1 2048 3905535 1951744 fd Linux raid autodetect
/dev/sda2 * 3905536 3907028991 1951561728 fd Linux raid autodetect
Disk /dev/sdb: 2000.4 GB, 2000398934016 bytes
255 heads, 63 sectors/track, 243201 cylinders, total 3907029168 sectors
Units = sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disk identifier: 0x00029d5c
Device Boot Start End Blocks Id System
/dev/sdb1 2048 3905535 1951744 fd Linux raid autodetect
/dev/sdb2 * 3905536 3907028991 1951561728 fd Linux raid autodetect
Disk /dev/sdd: 2000.4 GB, 2000398934016 bytes
255 heads, 63 sectors/track, 243201 cylinders, total 3907029168 sectors
Units = sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disk identifier: 0x000727bf
Device Boot Start End Blocks Id System
/dev/sdd1 2048 3905535 1951744 fd Linux raid autodetect
/dev/sdd2 * 3905536 3907028991 1951561728 fd Linux raid autodetect
Disk /dev/sde: 2000.4 GB, 2000398934016 bytes
255 heads, 63 sectors/track, 243201 cylinders, total 3907029168 sectors
Units = sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disk identifier: 0x0009fe7f
Device Boot Start End Blocks Id System
/dev/sde1 2048 3905535 1951744 fd Linux raid autodetect
/dev/sde2 * 3905536 3907028991 1951561728 fd Linux raid autodetect
mdadm -Esvv output (USB devices deleted)
/dev/sde2:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : dbe238a3:c7a528c1:a1b78589:276ecfcf
Name : ubuntu:0 (local to host ubuntu)
Creation Time : Wed Apr 1 22:27:42 2015
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3903121408 (1861.15 GiB 1998.40 GB)
Array Size : 5850624 (5.58 GiB 5.99 GB)
Used Dev Size : 3900416 (1904.82 MiB 1997.01 MB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : cdae3287:91168194:942ba99d:1a85c466
Update Time : Wed Apr 29 17:46:25 2015
Checksum : b8b84dad - correct
Events : 30
Layout : left-symmetric
Chunk Size : 512K
Device Role : spare
Array State : AAAA ('A' == active, '.' == missing)
/dev/sde1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : dbe238a3:c7a528c1:a1b78589:276ecfcf
Name : ubuntu:0 (local to host ubuntu)
Creation Time : Wed Apr 1 22:27:42 2015
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3901440 (1905.32 MiB 1997.54 MB)
Array Size : 5850624 (5.58 GiB 5.99 GB)
Used Dev Size : 3900416 (1904.82 MiB 1997.01 MB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : b051f523:4887e729:cd63bed1:8c2a7575
Update Time : Wed Apr 29 17:46:25 2015
Checksum : 453ddeef - correct
Events : 30
Layout : left-symmetric
Chunk Size : 512K
Device Role : Active device 3
Array State : AAAA ('A' == active, '.' == missing)
/dev/sde:
MBR Magic : aa55
Partition[0] : 3903488 sectors at 2048 (type fd)
Partition[1] : 3903123456 sectors at 3905536 (type fd)
/dev/sdd2:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : dbe238a3:c7a528c1:a1b78589:276ecfcf
Name : ubuntu:0 (local to host ubuntu)
Creation Time : Wed Apr 1 22:27:42 2015
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3903121408 (1861.15 GiB 1998.40 GB)
Array Size : 5850624 (5.58 GiB 5.99 GB)
Used Dev Size : 3900416 (1904.82 MiB 1997.01 MB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : 0f3f2b91:09cbb344:e52c4c4b:722d65c4
Update Time : Wed Apr 29 17:46:25 2015
Checksum : 7e273c0f - correct
Events : 30
Layout : left-symmetric
Chunk Size : 512K
Device Role : spare
Array State : AAAA ('A' == active, '.' == missing)
/dev/sdd1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : dbe238a3:c7a528c1:a1b78589:276ecfcf
Name : ubuntu:0 (local to host ubuntu)
Creation Time : Wed Apr 1 22:27:42 2015
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3901440 (1905.32 MiB 1997.54 MB)
Array Size : 5850624 (5.58 GiB 5.99 GB)
Used Dev Size : 3900416 (1904.82 MiB 1997.01 MB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : b6668730:3b1380bf:556700d9:30df829c
Update Time : Wed Apr 29 17:46:25 2015
Checksum : 15b83814 - correct
Events : 30
Layout : left-symmetric
Chunk Size : 512K
Device Role : Active device 2
Array State : AAAA ('A' == active, '.' == missing)
/dev/sdd:
MBR Magic : aa55
Partition[0] : 3903488 sectors at 2048 (type fd)
Partition[1] : 3903123456 sectors at 3905536 (type fd)
/dev/sdb2:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 1f28f7bb:7b3ecd41:ca0fa5d1:ccd008df
Name : ubuntu:1 (local to host ubuntu)
Creation Time : Wed Apr 1 22:27:58 2015
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3902861312 (1861.03 GiB 1998.26 GB)
Array Size : 5854290432 (5583.09 GiB 5994.79 GB)
Used Dev Size : 3902860288 (1861.03 GiB 1998.26 GB)
Data Offset : 262144 sectors
Super Offset : 8 sectors
State : clean
Device UUID : f1e79609:79b7ac23:55197f70:e8fbfd58
Update Time : Sun Apr 26 05:59:13 2015
Checksum : 696f4e76 - correct
Events : 18014
Layout : left-symmetric
Chunk Size : 512K
Device Role : Active device 1
Array State : AA.. ('A' == active, '.' == missing)
/dev/sdb1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : dbe238a3:c7a528c1:a1b78589:276ecfcf
Name : ubuntu:0 (local to host ubuntu)
Creation Time : Wed Apr 1 22:27:42 2015
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3901440 (1905.32 MiB 1997.54 MB)
Array Size : 5850624 (5.58 GiB 5.99 GB)
Used Dev Size : 3900416 (1904.82 MiB 1997.01 MB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : f52239b1:0fb87e7e:71e29ea4:bf67184a
Update Time : Wed Apr 29 17:46:25 2015
Checksum : ce9c9cd0 - correct
Events : 30
Layout : left-symmetric
Chunk Size : 512K
Device Role : Active device 1
Array State : AAAA ('A' == active, '.' == missing)
/dev/sdb:
MBR Magic : aa55
Partition[0] : 3903488 sectors at 2048 (type fd)
Partition[1] : 3903123456 sectors at 3905536 (type fd)
/dev/sda2:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 1f28f7bb:7b3ecd41:ca0fa5d1:ccd008df
Name : ubuntu:1 (local to host ubuntu)
Creation Time : Wed Apr 1 22:27:58 2015
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3902861312 (1861.03 GiB 1998.26 GB)
Array Size : 5854290432 (5583.09 GiB 5994.79 GB)
Used Dev Size : 3902860288 (1861.03 GiB 1998.26 GB)
Data Offset : 262144 sectors
Super Offset : 8 sectors
State : clean
Device UUID : 713e556d:ca104217:785db68a:d820a57b
Update Time : Sun Apr 26 05:59:13 2015
Checksum : fda151f9 - correct
Events : 18014
Layout : left-symmetric
Chunk Size : 512K
Device Role : Active device 0
Array State : AA.. ('A' == active, '.' == missing)
/dev/sda1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : dbe238a3:c7a528c1:a1b78589:276ecfcf
Name : ubuntu:0 (local to host ubuntu)
Creation Time : Wed Apr 1 22:27:42 2015
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3901440 (1905.32 MiB 1997.54 MB)
Array Size : 5850624 (5.58 GiB 5.99 GB)
Used Dev Size : 3900416 (1904.82 MiB 1997.01 MB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : c483532d:06f93351:cfdf5a92:e83855b5
Update Time : Wed Apr 29 17:46:25 2015
Checksum : 76650d1c - correct
Events : 30
Layout : left-symmetric
Chunk Size : 512K
Device Role : Active device 0
Array State : AAAA ('A' == active, '.' == missing)
/dev/sda:
MBR Magic : aa55
Partition[0] : 3903488 sectors at 2048 (type fd)
Partition[1] : 3903123456 sectors at 3905536 (type fd)--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox