* [PATCH 10/10] dm snapshot: Delete five unwanted spaces behind "list_for_each_entry"
From: SF Markus Elfring @ 2016-09-29 9:18 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <080668d9-1e1e-e208-f9ea-ff718e8070e5@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 29 Sep 2016 10:14:32 +0200
The script "checkpatch.pl" can point information out like the following.
WARNING: space prohibited between function name and open parenthesis '('
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-snap.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 7593986..1310652 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -366,7 +366,7 @@ static struct origin *__lookup_origin(struct block_device *origin)
struct origin *o;
ol = &_origins[origin_hash(origin)];
- list_for_each_entry (o, ol, hash_list)
+ list_for_each_entry(o, ol, hash_list)
if (bdev_equal(o->bdev, origin))
return o;
@@ -385,7 +385,7 @@ static struct dm_origin *__lookup_dm_origin(struct block_device *origin)
struct dm_origin *o;
ol = &_dm_origins[origin_hash(origin)];
- list_for_each_entry (o, ol, hash_list)
+ list_for_each_entry(o, ol, hash_list)
if (bdev_equal(o->dev->bdev, origin))
return o;
@@ -625,7 +625,7 @@ static void dm_exception_table_exit(struct dm_exception_table *et,
for (i = 0; i < size; i++) {
slot = et->table + i;
- list_for_each_entry_safe (ex, next, slot, hash_list)
+ list_for_each_entry_safe(ex, next, slot, hash_list)
kmem_cache_free(mem, ex);
}
@@ -653,7 +653,7 @@ static struct dm_exception *dm_lookup_exception(struct dm_exception_table *et,
struct dm_exception *e;
slot = &et->table[exception_hash(et, chunk)];
- list_for_each_entry (e, slot, hash_list)
+ list_for_each_entry(e, slot, hash_list)
if (chunk >= e->old_chunk &&
chunk <= e->old_chunk + dm_consecutive_chunk_count(e))
return e;
@@ -2068,7 +2068,7 @@ static int __origin_write(struct list_head *snapshots, sector_t sector,
chunk_t chunk;
/* Do all the snapshots on this origin */
- list_for_each_entry (snap, snapshots, list) {
+ list_for_each_entry(snap, snapshots, list) {
/*
* Don't make new exceptions in a merging snapshot
* because it has effectively been deleted
--
2.10.0
^ permalink raw reply related
* [PATCH 09/10] dm snapshot: Combine substrings for seven error messages
From: SF Markus Elfring @ 2016-09-29 9:17 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <080668d9-1e1e-e208-f9ea-ff718e8070e5@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 29 Sep 2016 09:46:32 +0200
The script "checkpatch.pl" can point information out like the following.
WARNING: quoted string split across lines
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-snap.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 4966584..7593986 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -468,8 +468,7 @@ static int __validate_exception_handover(struct dm_snapshot *snap)
if ((__find_snapshots_sharing_cow(snap, &snap_src, &snap_dest,
&snap_merge) == 2) ||
snap_dest) {
- snap->ti->error = "Snapshot cow pairing for exception "
- "table handover failed";
+ snap->ti->error = "Snapshot cow pairing for exception table handover failed";
return -EINVAL;
}
@@ -496,8 +495,7 @@ static int __validate_exception_handover(struct dm_snapshot *snap)
if (!snap_src->store->type->prepare_merge ||
!snap_src->store->type->commit_merge) {
- snap->ti->error = "Snapshot exception store does not "
- "support snapshot-merge.";
+ snap->ti->error = "Snapshot exception store does not support snapshot-merge.";
return -EINVAL;
}
@@ -858,8 +856,7 @@ static int __remove_single_exception_chunk(struct dm_snapshot *s,
e = dm_lookup_exception(&s->complete, old_chunk);
if (!e) {
- DMERR("Corruption detected: exception for block %llu is "
- "on disk but not in memory",
+ DMERR("Corruption detected: exception for block %llu is on disk but not in memory",
(unsigned long long)old_chunk);
return -EINVAL;
}
@@ -886,8 +883,7 @@ static int __remove_single_exception_chunk(struct dm_snapshot *s,
e->new_chunk++;
} else if (old_chunk != e->old_chunk +
dm_consecutive_chunk_count(e)) {
- DMERR("Attempt to merge block %llu from the "
- "middle of a chunk range [%llu - %llu]",
+ DMERR("Attempt to merge block %llu from the middle of a chunk range [%llu - %llu]",
(unsigned long long)old_chunk,
(unsigned long long)e->old_chunk,
(unsigned long long)
@@ -980,8 +976,7 @@ static void snapshot_merge_next_chunks(struct dm_snapshot *s)
&new_chunk);
if (linear_chunks <= 0) {
if (linear_chunks < 0) {
- DMERR("Read error in exception store: "
- "shutting down merge");
+ DMERR("Read error in exception store: shutting down merge");
down_write(&s->lock);
s->merge_failed = 1;
up_write(&s->lock);
@@ -1877,12 +1872,10 @@ static int snapshot_preresume(struct dm_target *ti)
if (snap_src && snap_dest) {
down_read(&snap_src->lock);
if (s == snap_src) {
- DMERR("Unable to resume snapshot source until "
- "handover completes.");
+ DMERR("Unable to resume snapshot source until handover completes.");
r = -EINVAL;
} else if (!dm_suspended(snap_src->ti)) {
- DMERR("Unable to perform snapshot handover until "
- "source is suspended.");
+ DMERR("Unable to perform snapshot handover until source is suspended.");
r = -EINVAL;
}
up_read(&snap_src->lock);
--
2.10.0
^ permalink raw reply related
* [PATCH 08/10] dm snapshot: Delete an unnecessary variable initialisation in remove_single_exception_chunk()
From: SF Markus Elfring @ 2016-09-29 9:16 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <080668d9-1e1e-e208-f9ea-ff718e8070e5@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 29 Sep 2016 09:06:37 +0200
The local variable "b" will be set to an appropriate pointer a bit later.
Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-snap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 1d90131..4966584 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -904,7 +904,7 @@ static void flush_bios(struct bio *bio);
static int remove_single_exception_chunk(struct dm_snapshot *s)
{
- struct bio *b = NULL;
+ struct bio *b;
int r;
chunk_t old_chunk = s->first_merging_chunk + s->num_merging_chunks - 1;
--
2.10.0
^ permalink raw reply related
* [PATCH 07/10] dm snapshot: Delete an unnecessary variable initialisation in merge_callback()
From: SF Markus Elfring @ 2016-09-29 9:15 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <080668d9-1e1e-e208-f9ea-ff718e8070e5@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 29 Sep 2016 09:00:06 +0200
The local variable "b" will be set to an appropriate pointer a bit later.
Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-snap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 06b1b69..1d90131 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1046,7 +1046,7 @@ static void error_bios(struct bio *bio);
static void merge_callback(int read_err, unsigned long write_err, void *context)
{
struct dm_snapshot *s = context;
- struct bio *b = NULL;
+ struct bio *b;
if (read_err || write_err) {
if (read_err)
--
2.10.0
^ permalink raw reply related
* [PATCH 06/10] dm snapshot: Delete an unnecessary variable initialisation in snapshot_ctr()
From: SF Markus Elfring @ 2016-09-29 9:14 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <080668d9-1e1e-e208-f9ea-ff718e8070e5@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 29 Sep 2016 08:38:29 +0200
The local variable "r" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-snap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 7bbd3c4..06b1b69 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1102,7 +1102,7 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv)
{
struct dm_snapshot *s;
int i;
- int r = -EINVAL;
+ int r;
char *origin_path, *cow_path;
dev_t origin_dev, cow_dev;
unsigned args_used, num_flush_bios = 1;
--
2.10.0
^ permalink raw reply related
* [PATCH 05/10] dm snapshot: Delete unnecessary variable initialisations in pending_complete()
From: SF Markus Elfring @ 2016-09-29 9:13 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <080668d9-1e1e-e208-f9ea-ff718e8070e5@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 29 Sep 2016 08:25:47 +0200
Three local variables will be set to an appropriate pointer a bit later.
Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-snap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index a6b797f..7bbd3c4 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1450,9 +1450,9 @@ static void pending_complete(void *context, int success)
struct dm_snap_pending_exception *pe = context;
struct dm_exception *e;
struct dm_snapshot *s = pe->snap;
- struct bio *origin_bios = NULL;
- struct bio *snapshot_bios = NULL;
- struct bio *full_bio = NULL;
+ struct bio *origin_bios;
+ struct bio *snapshot_bios;
+ struct bio *full_bio;
int error = 0;
if (!success) {
--
2.10.0
^ permalink raw reply related
* [PATCH 04/10] dm snapshot: Rename a jump label in pending_complete()
From: SF Markus Elfring @ 2016-09-29 9:12 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <080668d9-1e1e-e208-f9ea-ff718e8070e5@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 29 Sep 2016 08:18:51 +0200
Adjust jump labels according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-snap.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 82b7604..a6b797f 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1460,7 +1460,7 @@ static void pending_complete(void *context, int success)
down_write(&s->lock);
__invalidate_snapshot(s, -EIO);
error = 1;
- goto out;
+ goto remove_exception;
}
e = alloc_completed_exception(GFP_NOIO);
@@ -1468,7 +1468,7 @@ static void pending_complete(void *context, int success)
down_write(&s->lock);
__invalidate_snapshot(s, -ENOMEM);
error = 1;
- goto out;
+ goto remove_exception;
}
*e = pe->e;
@@ -1476,7 +1476,7 @@ static void pending_complete(void *context, int success)
if (!s->valid) {
free_completed_exception(e);
error = 1;
- goto out;
+ goto remove_exception;
}
/* Check for conflicting reads */
@@ -1487,8 +1487,7 @@ static void pending_complete(void *context, int success)
* in-flight exception from the list.
*/
dm_insert_exception(&s->complete, e);
-
-out:
+remove_exception:
dm_remove_exception(&pe->e);
snapshot_bios = bio_list_get(&pe->snapshot_bios);
origin_bios = bio_list_get(&pe->origin_bios);
--
2.10.0
^ permalink raw reply related
* [PATCH 03/10] dm snapshot: Delete an unnecessary variable initialisation in snapshot_map()
From: SF Markus Elfring @ 2016-09-29 9:11 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <080668d9-1e1e-e208-f9ea-ff718e8070e5@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 29 Sep 2016 08:00:29 +0200
The local variable "pe" will be set to an appropriate pointer a bit later.
Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-snap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 7d81390..82b7604 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1675,7 +1675,7 @@ static int snapshot_map(struct dm_target *ti, struct bio *bio)
struct dm_snapshot *s = ti->private;
int r = DM_MAPIO_REMAPPED;
chunk_t chunk;
- struct dm_snap_pending_exception *pe = NULL;
+ struct dm_snap_pending_exception *pe;
init_tracked_chunk(bio);
--
2.10.0
^ permalink raw reply related
* [PATCH 02/10] dm snapshot: Delete two error messages for a failed memory allocation
From: SF Markus Elfring @ 2016-09-29 9:10 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall, Wolfram Sang
In-Reply-To: <080668d9-1e1e-e208-f9ea-ff718e8070e5@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 28 Sep 2016 22:33:09 +0200
Omit extra messages for a memory allocation failure in this function.
Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-snap.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index f262f7e..7d81390 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -329,10 +329,8 @@ static int init_origin_hash(void)
_origins = kmalloc_array(ORIGIN_HASH_SIZE,
sizeof(*_origins),
GFP_KERNEL);
- if (!_origins) {
- DMERR("unable to allocate memory for _origins");
+ if (!_origins)
return -ENOMEM;
- }
for (i = 0; i < ORIGIN_HASH_SIZE; i++)
INIT_LIST_HEAD(_origins + i);
@@ -340,7 +338,6 @@ static int init_origin_hash(void)
sizeof(*_dm_origins),
GFP_KERNEL);
if (!_dm_origins) {
- DMERR("unable to allocate memory for _dm_origins");
kfree(_origins);
return -ENOMEM;
}
--
2.10.0
^ permalink raw reply related
* [PATCH 01/10] dm snapshot: Use kmalloc_array() in init_origin_hash()
From: SF Markus Elfring @ 2016-09-29 9:07 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <080668d9-1e1e-e208-f9ea-ff718e8070e5@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 28 Sep 2016 22:20:08 +0200
* Multiplications for the size determination of memory allocations
indicated that array data structures should be processed.
Thus use the corresponding function "kmalloc_array".
This issue was detected by using the Coccinelle software.
* Replace the specification of data structures by pointer dereferences
to make the corresponding size determination a bit safer according to
the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-snap.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index c65feea..f262f7e 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -326,8 +326,9 @@ static int init_origin_hash(void)
{
int i;
- _origins = kmalloc(ORIGIN_HASH_SIZE * sizeof(struct list_head),
- GFP_KERNEL);
+ _origins = kmalloc_array(ORIGIN_HASH_SIZE,
+ sizeof(*_origins),
+ GFP_KERNEL);
if (!_origins) {
DMERR("unable to allocate memory for _origins");
return -ENOMEM;
@@ -335,8 +336,9 @@ static int init_origin_hash(void)
for (i = 0; i < ORIGIN_HASH_SIZE; i++)
INIT_LIST_HEAD(_origins + i);
- _dm_origins = kmalloc(ORIGIN_HASH_SIZE * sizeof(struct list_head),
- GFP_KERNEL);
+ _dm_origins = kmalloc_array(ORIGIN_HASH_SIZE,
+ sizeof(*_dm_origins),
+ GFP_KERNEL);
if (!_dm_origins) {
DMERR("unable to allocate memory for _dm_origins");
kfree(_origins);
--
2.10.0
^ permalink raw reply related
* [PATCH 00/10] dm snapshot: Fine-tuning for several function implementations
From: SF Markus Elfring @ 2016-09-29 9:02 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <566ABCD9.1060404@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 29 Sep 2016 10:35:43 +0200
Some update suggestions were taken into account
from static source code analysis.
Markus Elfring (10):
Use kmalloc_array() in init_origin_hash()
Delete two error messages for a failed memory allocation
Delete an unnecessary variable initialisation in snapshot_map()
Rename a jump label in pending_complete()
Delete unnecessary variable initialisations in pending_complete()
Delete an unnecessary variable initialisation in snapshot_ctr()
Delete an unnecessary variable initialisation in merge_callback()
Delete an unnecessary variable initialisation in remove_single_exception_chunk()
Combine substrings for seven error messages
Delete five unwanted spaces behind "list_for_each_entry"
drivers/md/dm-snap.c | 69 +++++++++++++++++++++++-----------------------------
1 file changed, 30 insertions(+), 39 deletions(-)
--
2.10.0
^ permalink raw reply
* Re: [PATCH 08/16] md/bitmap: Rename a jump label in location_store()
From: Guoqing Jiang @ 2016-09-29 3:16 UTC (permalink / raw)
To: Jes Sorensen, SF Markus Elfring
Cc: linux-raid, Shaohua Li, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <wrfjy42b94uu.fsf@redhat.com>
On 09/28/2016 03:55 PM, Jes Sorensen wrote:
> SF Markus Elfring <elfring@users.sourceforge.net> writes:
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Tue, 27 Sep 2016 15:46:22 +0200
>>
>> Adjust jump labels according to the current Linux coding style convention.
>>
>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>> ---
>> drivers/md/bitmap.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
> Sorry but this patch is just plain ridiculous. It does not improve the
> code in any shape or form.
>
> 'out' as a label is perfectly legitimate and just as good as 'unlock'.
Agree, I also curious which document recorded the coding style convention.
Thanks,
Guoqing
^ permalink raw reply
* Re: moving spares into group and checking spares
From: NeilBrown @ 2016-09-29 1:21 UTC (permalink / raw)
To: scar, linux-raid
In-Reply-To: <nramjg$n07$1@blaine.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]
On Wed, Sep 14 2016, scar wrote:
> i currently have four RAID-5 md arrays which i concatenated into one
> logical volume (lvm2), essentially creating a RAID-50. each md array
> was created with one spare disk.
>
> instead, i would like to move the four spare disks into one group that
> each of the four arrays can have access to when needed. i was wondering
> how to safely accomplish this, preferably without unmounting/disrupting
> the filesystem.
You don't need to move the devices. Just use the spare-group= setting
in mdadm.conf to tell mdadm that all arrays should be considered part of
the same spare-group. Then "mdadm --monitor" will freely move spares
between arrays as needed.
>
> secondly, i have the checkarray script scheduled via cron to
> periodically check each of the four arrays. i noticed in the output of
> checkarray that it doesn't list the spare disk(s). so i'm guessing they
> are not being checked? i was wondering, then, how i could also check
> the spare disks to make sure they are healthy and ready to be used if
> needed?
mdadm doesn't provide any automatic support for this.
You would need to e.g.
- remove a spare from an array
- test it in some way: e.g. use dd to read every block.
- if satisfied, add it back to the array.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]
^ permalink raw reply
* Re: moving spares into group and checking spares
From: NeilBrown @ 2016-09-29 1:17 UTC (permalink / raw)
To: scar, linux-raid
In-Reply-To: <nsf29k$f6s$1@blaine.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2099 bytes --]
On Wed, Sep 28 2016, scar wrote:
> Chris Murphy wrote on 09/14/2016 08:51 PM:
>> On Wed, Sep 14, 2016 at 9:42 PM, scar <scar@drigon.com> wrote:
>>
>>> 00 80 00
>>> Sep 12 08:05:34 hind kernel: [1350051.023847] end_request: critical medium
>>> error, dev sdk, sector 598457896
>>> Sep 12 08:05:35 hind kernel: [1350051.385810] md/raid:md0: read error
>>> corrected (8 sectors at 598455848 on sdk1)
>>> Sep 12 08:05:35 hind kernel: [1350051.385826] md/raid:md0: read error
>>> corrected (8 sectors at 598455856 on sdk1)
>>> Sep 12 08:05:35 hind kernel: [1350051.385834] md/raid:md0: read error
>>> corrected (8 sectors at 598455864 on sdk1)
>>> Sep 12 08:05:35 hind kernel: [1350051.385840] md/raid:md0: read error
>>> corrected (8 sectors at 598455872 on sdk1)
>>> Sep 12 08:05:35 hind kernel: [1350051.385847] md/raid:md0: read error
>>> corrected (8 sectors at 598455880 on sdk1)
>>> Sep 12 08:05:35 hind kernel: [1350051.385853] md/raid:md0: read error
>>> corrected (8 sectors at 598455888 on sdk1)
>>> Sep 12 08:05:35 hind kernel: [1350051.385859] md/raid:md0: read error
>>> corrected (8 sectors at 598455896 on sdk1)
>>> Sep 12 08:05:35 hind kernel: [1350051.385865] md/raid:md0: read error
>>> corrected (8 sectors at 598455904 on sdk1)
>>> Sep 12 08:05:35 hind kernel: [1350051.385873] md/raid:md0: read error
>>> corrected (8 sectors at 598455912 on sdk1)
>>> Sep 12 08:05:35 hind kernel: [1350051.385880] md/raid:md0: read error
>>> corrected (8 sectors at 598455920 on sdk1)
>>> Sep 12 13:39:43 hind kernel: [1370087.022160] md: md0: data-check done.
>>>
>>>
>>> so it seems to be working? although the sector reported by libata is
>>> different than what md corrected
>>
>> Looks like it replaced the entire chunk that includes the bad sector.
>> Is the chunk size 32KiB?
>>
>
> No it is 512K, and the sector reported by libata is almost 2000 sectors
> different than what mdadm reported
The difference is probably the Data Offset.
libata reports a sector in the device.
md reports a sector in the data section of the device.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]
^ permalink raw reply
* Re: [PATCH 08/16] md/bitmap: Rename a jump label in location_store()
From: Jes Sorensen @ 2016-09-28 19:55 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-raid, Shaohua Li, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <d4de2fd9-93a6-9731-13a4-9b02a6c7f971@users.sourceforge.net>
SF Markus Elfring <elfring@users.sourceforge.net> writes:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 27 Sep 2016 15:46:22 +0200
>
> Adjust jump labels according to the current Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/md/bitmap.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
Sorry but this patch is just plain ridiculous. It does not improve the
code in any shape or form.
'out' as a label is perfectly legitimate and just as good as 'unlock'.
Jes
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index 41d99fd..22fa09a 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -2187,11 +2187,11 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
> if (mddev->pers) {
> if (!mddev->pers->quiesce) {
> rv = -EBUSY;
> - goto out;
> + goto unlock;
> }
> if (mddev->recovery || mddev->sync_thread) {
> rv = -EBUSY;
> - goto out;
> + goto unlock;
> }
> }
>
> @@ -2200,7 +2200,7 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
> /* bitmap already configured. Only option is to clear it */
> if (strncmp(buf, "none", 4) != 0) {
> rv = -EBUSY;
> - goto out;
> + goto unlock;
> }
> if (mddev->pers) {
> mddev->pers->quiesce(mddev, 1);
> @@ -2221,23 +2221,23 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
> else if (strncmp(buf, "file:", 5) == 0) {
> /* Not supported yet */
> rv = -EINVAL;
> - goto out;
> + goto unlock;
> } else {
> if (buf[0] == '+')
> rv = kstrtoll(buf+1, 10, &offset);
> else
> rv = kstrtoll(buf, 10, &offset);
> if (rv)
> - goto out;
> + goto unlock;
> if (offset == 0) {
> rv = -EINVAL;
> - goto out;
> + goto unlock;
> }
> if (mddev->bitmap_info.external == 0 &&
> mddev->major_version == 0 &&
> offset != mddev->bitmap_info.default_offset) {
> rv = -EINVAL;
> - goto out;
> + goto unlock;
> }
> mddev->bitmap_info.offset = offset;
> if (mddev->pers) {
> @@ -2255,7 +2255,7 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
> mddev->pers->quiesce(mddev, 0);
> if (rv) {
> bitmap_destroy(mddev);
> - goto out;
> + goto unlock;
> }
> }
> }
> @@ -2268,7 +2268,7 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
> md_wakeup_thread(mddev->thread);
> }
> rv = 0;
> -out:
> +unlock:
> mddev_unlock(mddev);
> if (rv)
> return rv;
^ permalink raw reply
* Re: [PATCH 01/16] md/bitmap: Use kmalloc_array() in bitmap_storage_alloc()
From: Jes Sorensen @ 2016-09-28 19:51 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-raid, Shaohua Li, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <c8f8d7c1-c62b-d889-9800-2da8358c1f50@users.sourceforge.net>
SF Markus Elfring <elfring@users.sourceforge.net> writes:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 27 Sep 2016 13:01:07 +0200
>
> * A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "kmalloc_array".
>
> This issue was detected by using the Coccinelle software.
>
> * Replace the specification of a data type by a pointer dereference
> to make the corresponding size determination a bit safer according to
> the Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/md/bitmap.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index 13041ee..8cfb02c 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -760,9 +760,9 @@ static int bitmap_storage_alloc(struct bitmap_storage *store,
>
> num_pages = DIV_ROUND_UP(bytes, PAGE_SIZE);
> offset = slot_number * num_pages;
> -
> - store->filemap = kmalloc(sizeof(struct page *)
> - * num_pages, GFP_KERNEL);
> + store->filemap = kmalloc_array(num_pages,
> + sizeof(*store->filemap),
> + GFP_KERNEL);
If you have to make cosmetic changes for the sake of it, then at least
use the 80 characters per line that you have available.
This one makes the code uglier.
Jes
^ permalink raw reply
* Re: WARNING: mismatch_cnt is not 0 on <array device>
From: Benjammin2068 @ 2016-09-28 15:55 UTC (permalink / raw)
To: Adam Goryachev, Linux-RAID
In-Reply-To: <be044941-8097-7d6f-5216-294fe2cac4d9@websitemanagers.com.au>
On 09/27/2016 06:09 PM, Adam Goryachev wrote:
> Just out of interest, but I'm not sure how useful your munin monitoring will be... AFAIK, the mismatch_cnt value is only updated when you run a check, which would probably take some number of hours to complete. I would guess that you are unlikely to run more than one check a week or month.... and as soon as there is any change (unless you know the explanation) then you should be looking to resolve that.
>
> Unless of course I'm wrong about when the count is updated?
You would be correct - only during checks - but it's an easy way for me to just keep track of it..
And a check takes about 3hrs on this array. (and it happens every weekend)
-Ben
^ permalink raw reply
* [PATCH 10/10] md/dm-crypt: Delete unnecessary variable initialisations in crypt_iv_essiv_ctr()
From: SF Markus Elfring @ 2016-09-28 15:47 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <7c232017-e291-fd2d-5516-26e5150d90df@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 28 Sep 2016 16:44:32 +0200
Three local variables will be set to an appropriate pointer a bit later.
Thus omit the explicit initialisation which became unnecessary with
a previous update step.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-crypt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 53a9155..d27716e 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -361,9 +361,9 @@ static void crypt_iv_essiv_dtr(struct crypt_config *cc)
static int crypt_iv_essiv_ctr(struct crypt_config *cc, struct dm_target *ti,
const char *opts)
{
- struct crypto_cipher *essiv_tfm = NULL;
- struct crypto_ahash *hash_tfm = NULL;
- u8 *salt = NULL;
+ struct crypto_cipher *essiv_tfm;
+ struct crypto_ahash *hash_tfm;
+ u8 *salt;
int err;
if (!opts) {
--
2.10.0
^ permalink raw reply related
* [PATCH 09/10] md/dm-crypt: Two checks and one function call less in crypt_iv_essiv_ctr() after error detection
From: SF Markus Elfring @ 2016-09-28 15:46 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <7c232017-e291-fd2d-5516-26e5150d90df@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 28 Sep 2016 16:38:37 +0200
The kfree() function was called in one case by the crypt_iv_essiv_ctr()
function during error handling even if the passed variable "salt"
contained a null pointer.
* Adjust a jump target according to the Linux coding style convention.
* Delete this function call and a condition check which became unnecessary
with this refactoring.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-crypt.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 47f6265..53a9155 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -382,7 +382,7 @@ static int crypt_iv_essiv_ctr(struct crypt_config *cc, struct dm_target *ti,
if (!salt) {
ti->error = "Error kmallocing salt storage in ESSIV";
err = -ENOMEM;
- goto bad;
+ goto free_hash;
}
cc->iv_gen_private.essiv.salt = salt;
@@ -397,11 +397,8 @@ static int crypt_iv_essiv_ctr(struct crypt_config *cc, struct dm_target *ti,
cc->iv_private = essiv_tfm;
return 0;
-
-bad:
- if (hash_tfm && !IS_ERR(hash_tfm))
- crypto_free_ahash(hash_tfm);
- kfree(salt);
+free_hash:
+ crypto_free_ahash(hash_tfm);
return err;
}
--
2.10.0
^ permalink raw reply related
* [PATCH 08/10] md/dm-crypt: Return directly after a failed crypto_alloc_ahash() in crypt_iv_essiv_ctr()
From: SF Markus Elfring @ 2016-09-28 15:45 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <7c232017-e291-fd2d-5516-26e5150d90df@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 28 Sep 2016 15:55:47 +0200
Return directly after a call of the function "crypto_alloc_ahash"
failed here.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-crypt.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index c457b5e..47f6265 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -375,8 +375,7 @@ static int crypt_iv_essiv_ctr(struct crypt_config *cc, struct dm_target *ti,
hash_tfm = crypto_alloc_ahash(opts, 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(hash_tfm)) {
ti->error = "Error initializing ESSIV hash";
- err = PTR_ERR(hash_tfm);
- goto bad;
+ return PTR_ERR(hash_tfm);
}
salt = kzalloc(crypto_ahash_digestsize(hash_tfm), GFP_KERNEL);
--
2.10.0
^ permalink raw reply related
* [PATCH 07/10] md/dm-crypt: Rename a jump label in crypt_iv_tcw_whitening()
From: SF Markus Elfring @ 2016-09-28 15:44 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <7c232017-e291-fd2d-5516-26e5150d90df@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 28 Sep 2016 15:32:15 +0200
Adjust jump labels according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-crypt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 3bc54c1..c457b5e 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -698,13 +698,13 @@ static int crypt_iv_tcw_whitening(struct crypt_config *cc,
for (i = 0; i < 4; i++) {
r = crypto_shash_init(desc);
if (r)
- goto out;
+ goto zero_memory;
r = crypto_shash_update(desc, &buf[i * 4], 4);
if (r)
- goto out;
+ goto zero_memory;
r = crypto_shash_final(desc, &buf[i * 4]);
if (r)
- goto out;
+ goto zero_memory;
}
crypto_xor(&buf[0], &buf[12], 4);
crypto_xor(&buf[4], &buf[8], 4);
@@ -712,7 +712,7 @@ static int crypt_iv_tcw_whitening(struct crypt_config *cc,
/* apply whitening (8 bytes) to whole sector */
for (i = 0; i < ((1 << SECTOR_SHIFT) / 8); i++)
crypto_xor(data + i * 8, buf, 8);
-out:
+zero_memory:
memzero_explicit(buf, sizeof(buf));
return r;
}
--
2.10.0
^ permalink raw reply related
* [PATCH 06/10] md/dm-crypt: Delete an unnecessary variable initialisation in crypt_set_key()
From: SF Markus Elfring @ 2016-09-28 15:43 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <7c232017-e291-fd2d-5516-26e5150d90df@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 28 Sep 2016 15:24:13 +0200
The local variable "r" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-crypt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 7e0fd82..3bc54c1 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1491,7 +1491,7 @@ static int crypt_setkey_allcpus(struct crypt_config *cc)
static int crypt_set_key(struct crypt_config *cc, char *key)
{
- int r = -EINVAL;
+ int r;
int key_string_len = strlen(key);
/* The key size may not be changed. */
--
2.10.0
^ permalink raw reply related
* [PATCH 05/10] md/dm-crypt: Rename a jump label in crypt_set_key()
From: SF Markus Elfring @ 2016-09-28 15:42 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <7c232017-e291-fd2d-5516-26e5150d90df@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 28 Sep 2016 15:21:18 +0200
Adjust jump labels according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-crypt.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 7778e9b..7e0fd82 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1496,20 +1496,19 @@ static int crypt_set_key(struct crypt_config *cc, char *key)
/* The key size may not be changed. */
if (cc->key_size != (key_string_len >> 1))
- goto out;
+ goto set_memory;
/* Hyphen (which gives a key_size of zero) means there is no key. */
if (!cc->key_size && strcmp(key, "-"))
- goto out;
+ goto set_memory;
if (cc->key_size && crypt_decode_key(cc->key, key, cc->key_size) < 0)
- goto out;
+ goto set_memory;
set_bit(DM_CRYPT_KEY_VALID, &cc->flags);
r = crypt_setkey_allcpus(cc);
-
-out:
+set_memory:
/* Hex key string not needed after here, so wipe it. */
memset(key, '0', key_string_len);
--
2.10.0
^ permalink raw reply related
* [PATCH 04/10] md/dm-crypt: Delete an unnecessary variable initialisation in crypt_message()
From: SF Markus Elfring @ 2016-09-28 15:41 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <7c232017-e291-fd2d-5516-26e5150d90df@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 28 Sep 2016 15:06:05 +0200
The local variable "ret" will be set to an appropriate value in if branches.
Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-crypt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 08e3de2..7778e9b 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2013,7 +2013,7 @@ static void crypt_resume(struct dm_target *ti)
static int crypt_message(struct dm_target *ti, unsigned argc, char **argv)
{
struct crypt_config *cc = ti->private;
- int ret = -EINVAL;
+ int ret;
if (argc < 2)
goto show_warning;
--
2.10.0
^ permalink raw reply related
* [PATCH 03/10] md/dm-crypt: Rename a jump label in crypt_message()
From: SF Markus Elfring @ 2016-09-28 15:40 UTC (permalink / raw)
To: dm-devel, linux-raid, Alasdair Kergon, Mike Snitzer, Shaohua Li
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <7c232017-e291-fd2d-5516-26e5150d90df@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 28 Sep 2016 14:54:39 +0200
Adjust a jump label according to the current Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/md/dm-crypt.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 88a3b62..08e3de2 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2016,7 +2016,7 @@ static int crypt_message(struct dm_target *ti, unsigned argc, char **argv)
int ret = -EINVAL;
if (argc < 2)
- goto error;
+ goto show_warning;
if (!strcasecmp(argv[0], "key")) {
if (!test_bit(DM_CRYPT_SUSPENDED, &cc->flags)) {
@@ -2040,8 +2040,7 @@ static int crypt_message(struct dm_target *ti, unsigned argc, char **argv)
return crypt_wipe_key(cc);
}
}
-
-error:
+show_warning:
DMWARN("unrecognised message received.");
return -EINVAL;
}
--
2.10.0
^ permalink raw reply related
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