Linux RAID subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH 49/54] md/raid10: Replace printk() calls by the usage of higher level interfaces
From: Joe Perches @ 2016-10-06 17:39 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak, LKML,
	kernel-janitors, Julia Lawall
In-Reply-To: <321cc4aa-679a-a2fe-be0b-96285b19e489@users.sourceforge.net>

On Thu, 2016-10-06 at 19:20 +0200, SF Markus Elfring wrote:
> How should a multiline log message be achieved as it was constructed
> in the function "raid10_error" (for example)?
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/md/raid10.c?id=c802e87fbe2d4dd58982d01b3c39bc5a781223aa#n1589

As two individual calls to pr_alert

^ permalink raw reply

* Re: [PATCH 37/54] md/raid5: Replace a seq_printf() call by seq_puts() in raid5_status()
From: Joe Perches @ 2016-10-06 17:36 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak, LKML,
	kernel-janitors, Julia Lawall
In-Reply-To: <d896b697-6735-e50d-00b0-2b3d9f371c99@users.sourceforge.net>

On Thu, 2016-10-06 at 19:09 +0200, SF Markus Elfring wrote:
> > > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > []
> > > @@ -7044,7 +7044,7 @@ static void raid5_status(struct seq_file *seq, struct mddev *mddev)
> > >  			   rdev && test_bit(In_sync, &rdev->flags) ? "U" : "_");
> > >  	}
> > >  	rcu_read_unlock();
> > > -	seq_printf (seq, "]");
> > > +	seq_puts(seq, "]");
> > seq_putc
> How do you think about the possibility that the script "checkpatch.pl" can also point
> such a source code transformation out directly?

Why don't _you_ try to implement that in checkpatch instead?


^ permalink raw reply

* Re: Prefetch in /lib/raid6/avx2.c
From: Doug Dumitru @ 2016-10-06 17:32 UTC (permalink / raw)
  To: Markus Stockhausen
  Cc: Shaohua Li, linux-raid, gayatri.kammela@intel.com,
	ravi.v.shankar@intel.com, hpa@zytor.com, yu-cheng.yu@intel.com,
	yuanhan.liu@intel.com
In-Reply-To: <12EF8D94C6F8734FB2FF37B9FBEDD1739B85E450@EXCHANGE.collogia.de>

On Thu, Oct 6, 2016 at 12:27 AM, Markus Stockhausen
<stockhausen@collogia.de> wrote:
>> Von: linux-raid-owner@vger.kernel.org [linux-raid-owner@vger.kernel.org]&quot; im Auftrag von &quot;Shaohua Li [shli@kernel.org]
>> Gesendet: Donnerstag, 6. Oktober 2016 01:17
>> An: Doug Dumitru
>> Cc: linux-raid; gayatri.kammela@intel.com; ravi.v.shankar@intel.com; hpa@zytor.com; yu-cheng.yu@intel.com; yuanhan.liu@intel.com
>> Betreff: Re: Prefetch in /lib/raid6/avx2.c
>>
>> On Sun, Oct 02, 2016 at 03:40:09PM -0700, Doug Dumitru wrote:
>> > I have been doing some high bandwidth testing of raid-6, and the
>> > pretetch in raid6_avx24_gen_syndrome appears to be less than optimal.
>> >
>> > This is my patch (against 4.4.0-38 [Ubuntu 16.04LTS)
>> >
>> > --- cut here ---
>> > --- lib/raid6/avx2.c0   2016-10-01 21:42:25.280347868 -0700
>> > +++ lib/raid6/avx2.c    2016-10-02 15:35:48.168480760 -0700
>> > @@ -189,10 +189,8 @@
>> >
>> >                 for (z = z0; z >= 0; z--) {
>> >
>> > -                       asm volatile("prefetchnta %0" : : "m" (dptr[z][d]));
>> > -                       asm volatile("prefetchnta %0" : : "m" (dptr[z][d+32]));
>> > -                       asm volatile("prefetchnta %0" : : "m" (dptr[z][d+64]));
>> > -                       asm volatile("prefetchnta %0" : : "m" (dptr[z][d+96]));
>> > +                       asm volatile("prefetchnta %0" : : "m" (dptr[z][d+128]));
>> > +                       asm volatile("prefetchnta %0" : : "m" (dptr[z][d+192]));
>
> From the first look that looks strange.
>
> 1) It will add 2 prefetches for the last blocks beyond the data. Feels like bad coding.

This is correct, but the original code adds four prefetches beyond the
buffer.  My understanding is that an extra prefetch is often less
expensive than the test to avoid it.

I would also mention that the original code probably can have the
[d+32] and [d+96] removed without issue.  The comments imply 32 byte
cache lines, which sounds overly generic, especially for AVX2 specific
code.

> 2) The prefetch for the next block is already in the next loop (d+128)

The loop is 256 bytes (4 x AVX2 registers), so the prefetch is only
for the data that is to be immediately used in the next 20 or so
instructions.

I tried other iterations, including prefetching a disk ahead [z-1][d]
and [z-1][d+128] (disks are traversed backwards), but this was slower
in testing.  I also tried a lot of manual unrolling to tweak the extra
prefetches out, but still this simple case tested better.

I was actually surprised by how much it helped.  Again, my test is
very synthetic (it does use the raid6 code end-to-end, but with a lot
of experimental patches).  Also, my array has 24 disks so the pretetch
is actually 44 cache lines early (which seems like a lot, but then
again, it does fit easily in L1).

>
> Maybe the prefetcher takes longer than expected. And thus the next loop
> will benefit from the "relocated" hint.
>
>> >
>> >                         asm volatile("vpcmpgtb %ymm4,%ymm1,%ymm5");
>> >                         asm volatile("vpcmpgtb %ymm6,%ymm1,%ymm7");
>> > --- cut here ---
>> >
>> > In perf, the cpu cycles goes from 5.3% to 3.0% for
>> > raid6_avx24_gen_syndrome in my test and throughput increases from
>> > about 8.2GB/sec to almost 10GB/sec.  It is a very "synthetic" test,
>> > but the avx2 code does seem to be a factor.
>> >
>> > I suspect other SSE and AVX "unroll variants" have similar issues, but
>> > I have not tested those.
>> >
>> > My test system is an E5-1650 v3 (single socket) with DDR4.  This might
>> > help dual sockets even more.
>>
>> CC some intel folks to see if they have ideas
>> --
>> 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



-- 
Doug Dumitru
EasyCo LLC

^ permalink raw reply

* Re: [PATCH 49/54] md/raid10: Replace printk() calls by the usage of higher level interfaces
From: SF Markus Elfring @ 2016-10-06 17:20 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak, LKML,
	kernel-janitors, Julia Lawall
In-Reply-To: <1475771616.1914.9.camel@perches.com>

>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index 5119846..0f2cb20 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -18,6 +18,8 @@
>>   * Software Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>>   */
>>  
>> +#define MY_LOG_PREFIX KBUILD_MODNAME ": "
>> +#define pr_fmt(fmt) MY_LOG_PREFIX fmt
> 
> Please just use
> 
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> like the more than thousand other uses in the kernel.

Thanks for your suggestion.

I got the impression that the omission of a macro like "MY_LOG_PREFIX"
would not really work for the suggested source code transformation so far.

How should a multiline log message be achieved as it was constructed
in the function "raid10_error" (for example)?
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/md/raid10.c?id=c802e87fbe2d4dd58982d01b3c39bc5a781223aa#n1589

Regards,
Markus

^ permalink raw reply

* Re: [PATCH 37/54] md/raid5: Replace a seq_printf() call by seq_puts() in raid5_status()
From: SF Markus Elfring @ 2016-10-06 17:09 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak, LKML,
	kernel-janitors, Julia Lawall
In-Reply-To: <1475771699.1914.10.camel@perches.com>

>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> []
>> @@ -7044,7 +7044,7 @@ static void raid5_status(struct seq_file *seq, struct mddev *mddev)
>>  			   rdev && test_bit(In_sync, &rdev->flags) ? "U" : "_");
>>  	}
>>  	rcu_read_unlock();
>> -	seq_printf (seq, "]");
>> +	seq_puts(seq, "]");
> 
> seq_putc

Thanks for your update suggestion.

How do you think about the possibility that the script "checkpatch.pl" can also point
such a source code transformation out directly?
Would an additional check for the length of the passed string be useful in similar
use cases?

Regards,
Markus

^ permalink raw reply

* Re: [PATCH 37/54] md/raid5: Replace a seq_printf() call by seq_puts() in raid5_status()
From: Joe Perches @ 2016-10-06 16:34 UTC (permalink / raw)
  To: SF Markus Elfring, linux-raid, Christoph Hellwig, Guoqing Jiang,
	Jens Axboe, Mike Christie, Neil Brown, Shaohua Li,
	Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <92c52f1d-d151-cea6-e9ac-31378e6862d0@users.sourceforge.net>

On Thu, 2016-10-06 at 11:37 +0200, SF Markus Elfring wrote:
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
[]
> @@ -7044,7 +7044,7 @@ static void raid5_status(struct seq_file *seq, struct mddev *mddev)
>  			   rdev && test_bit(In_sync, &rdev->flags) ? "U" : "_");
>  	}
>  	rcu_read_unlock();
> -	seq_printf (seq, "]");
> +	seq_puts(seq, "]");

seq_putc

^ permalink raw reply

* Re: [PATCH 49/54] md/raid10: Replace printk() calls by the usage of higher level interfaces
From: Joe Perches @ 2016-10-06 16:33 UTC (permalink / raw)
  To: SF Markus Elfring, linux-raid, Christoph Hellwig, Guoqing Jiang,
	Jens Axboe, Mike Christie, Neil Brown, Shaohua Li,
	Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <f9f16d1c-c50d-d4be-a6ba-2cb7081eb9b5@users.sourceforge.net>

On Thu, 2016-10-06 at 11:49 +0200, SF Markus Elfring wrote:
[]
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 5119846..0f2cb20 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -18,6 +18,8 @@
>   * Software Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>   */
>  
> +#define MY_LOG_PREFIX KBUILD_MODNAME ": "
> +#define pr_fmt(fmt) MY_LOG_PREFIX fmt

Please just use

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

like the more than thousand other uses in the kernel.

^ permalink raw reply

* [PATCH v2 13/15] md-cluster: Less function calls in join() after error detection
From: SF Markus Elfring @ 2016-10-06 15:16 UTC (permalink / raw)
  To: linux-raid, Guoqing Jiang, Shaohua Li; +Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <c0a9d85c-39d4-901e-a93f-d419d9f7159b@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 6 Oct 2016 16:48:51 +0200

A few resource release functions were called in some cases
by the join() function during error handling
even if the passed data structure member contained a null pointer.

* Adjust jump targets according to the Linux coding style convention.

* Delete a repeated check which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---

v2: Julia Lawall wrote on 2016-10-01 at 19:07:
> Please check lines 919 and 920.
> 
> julia
> 
> ---------- Forwarded message ----------
> Date: Sun, 2 Oct 2016 01:03:19 +0800
> From: kbuild test robot <fengguang.wu@intel.com>
> To: kbuild@01.org
> Cc: Julia Lawall <julia.lawall@lip6.fr>
> Subject:
>     [linux-review:SF-Markus-Elfring/md-cluster-Fine-tuning-for-ten-function-impl
>     ementations/20161001-230311 13/15] drivers/md/md-cluster.c:920:23-28: ERROR:
>      reference preceded by free on line 919
> 
> CC: kbuild-all@01.org
> TO: Markus Elfring <elfring@users.sourceforge.net>
> CC: 0day robot <fengguang.wu@intel.com>
> 
> tree:   https://github.com/0day-ci/linux SF-Markus-Elfring/md-cluster-Fine-tuning-for-ten-function-implementations/20161001-230311
> head:   7b2084c76eb1c60f2ae5c2778470124b5b68e9fb
> commit: 6955234a0083a739f595e753e89ba7b5b3962f50 [13/15] md-cluster: Less function calls in join() after error detection
> :::::: branch date: 2 hours ago
> :::::: commit date: 2 hours ago
> 
>>> drivers/md/md-cluster.c:920:23-28: ERROR: reference preceded by free on line 919
> 
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 6955234a0083a739f595e753e89ba7b5b3962f50
> vim +920 drivers/md/md-cluster.c
> 
> 6955234a Markus Elfring    2016-10-01  913  	lockres_free(cinfo->message_lockres);
> 6955234a Markus Elfring    2016-10-01  914  unregister_recv:
> 6955234a Markus Elfring    2016-10-01  915  	md_unregister_thread(&cinfo->recv_thread);
> 6955234a Markus Elfring    2016-10-01  916  release_lockspace:
> c4ce867f Goldwyn Rodrigues 2014-03-29  917  	dlm_release_lockspace(cinfo->lockspace, 2);
> 6955234a Markus Elfring    2016-10-01  918  free_cluster_info:
> c4ce867f Goldwyn Rodrigues 2014-03-29 @919  	kfree(cinfo);
> 6955234a Markus Elfring    2016-10-01 @920  	md_unregister_thread(&cinfo->recovery_thread);
> 6955234a Markus Elfring    2016-10-01  921  	mddev->cluster_info = NULL;
> c4ce867f Goldwyn Rodrigues 2014-03-29  922  	return ret;
> edb39c9d Goldwyn Rodrigues 2014-03-29  923  }
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation


This automatic notification pointed out that I should have switched the order
of calls for the functions "kfree" and "md_unregister_thread".
I hope that my second approach could be integrated into another
source code repository.


 drivers/md/md-cluster.c | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index e1ebcc4..e60511a 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -837,39 +837,39 @@ static int join(struct mddev *mddev, int nodes)
 				DLM_LSFL_FS, LVB_SIZE,
 				&md_ls_ops, mddev, &ops_rv, &cinfo->lockspace);
 	if (ret)
-		goto err;
+		goto unregister_recovery_thread;
 	wait_for_completion(&cinfo->completion);
 	if (nodes < cinfo->slot_number) {
 		pr_err("md-cluster: Slot allotted(%d) is greater than available slots(%d).",
 			cinfo->slot_number, nodes);
 		ret = -ERANGE;
-		goto err;
+		goto release_lockspace;
 	}
 	/* Initiate the communication resources */
 	ret = -ENOMEM;
 	cinfo->recv_thread = md_register_thread(recv_daemon, mddev, "cluster_recv");
 	if (!cinfo->recv_thread)
-		goto err;
+		goto release_lockspace;
 	cinfo->message_lockres = lockres_init(mddev, "message", NULL, 1);
 	if (!cinfo->message_lockres)
-		goto err;
+		goto unregister_recv;
 	cinfo->token_lockres = lockres_init(mddev, "token", NULL, 0);
 	if (!cinfo->token_lockres)
-		goto err;
+		goto free_message;
 	cinfo->no_new_dev_lockres = lockres_init(mddev, "no-new-dev", NULL, 0);
 	if (!cinfo->no_new_dev_lockres)
-		goto err;
+		goto free_token;
 
 	ret = dlm_lock_sync(cinfo->token_lockres, DLM_LOCK_EX);
 	if (ret) {
 		ret = -EAGAIN;
 		pr_err("md-cluster: can't join cluster to avoid lock issue\n");
-		goto err;
+		goto free_no_new_dev;
 	}
 	cinfo->ack_lockres = lockres_init(mddev, "ack", ack_bast, 0);
 	if (!cinfo->ack_lockres) {
 		ret = -ENOMEM;
-		goto err;
+		goto free_no_new_dev;
 	}
 	/* get sync CR lock on ACK. */
 	if (dlm_lock_sync(cinfo->ack_lockres, DLM_LOCK_CR))
@@ -886,34 +886,39 @@ static int join(struct mddev *mddev, int nodes)
 	cinfo->bitmap_lockres = lockres_init(mddev, str, NULL, 1);
 	if (!cinfo->bitmap_lockres) {
 		ret = -ENOMEM;
-		goto err;
+		goto free_ack;
 	}
 	if (dlm_lock_sync(cinfo->bitmap_lockres, DLM_LOCK_PW)) {
 		pr_err("Failed to get bitmap lock\n");
 		ret = -EINVAL;
-		goto err;
+		goto free_bitmap;
 	}
 
 	cinfo->resync_lockres = lockres_init(mddev, "resync", NULL, 0);
 	if (!cinfo->resync_lockres) {
 		ret = -ENOMEM;
-		goto err;
+		goto free_bitmap;
 	}
 
 	return 0;
-err:
-	md_unregister_thread(&cinfo->recovery_thread);
-	md_unregister_thread(&cinfo->recv_thread);
-	lockres_free(cinfo->message_lockres);
-	lockres_free(cinfo->token_lockres);
+free_bitmap:
+	lockres_free(cinfo->bitmap_lockres);
+free_ack:
 	lockres_free(cinfo->ack_lockres);
+free_no_new_dev:
 	lockres_free(cinfo->no_new_dev_lockres);
-	lockres_free(cinfo->resync_lockres);
-	lockres_free(cinfo->bitmap_lockres);
-	if (cinfo->lockspace)
-		dlm_release_lockspace(cinfo->lockspace, 2);
-	mddev->cluster_info = NULL;
+free_token:
+	lockres_free(cinfo->token_lockres);
+free_message:
+	lockres_free(cinfo->message_lockres);
+unregister_recv:
+	md_unregister_thread(&cinfo->recv_thread);
+release_lockspace:
+	dlm_release_lockspace(cinfo->lockspace, 2);
+unregister_recovery_thread:
+	md_unregister_thread(&cinfo->recovery_thread);
 	kfree(cinfo);
+	mddev->cluster_info = NULL;
 	return ret;
 }
 
-- 
2.10.1


^ permalink raw reply related

* Re: MD-RAID: Fine-tuning for several function implementations
From: SF Markus Elfring @ 2016-10-06 10:07 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-raid, Guoqing Jiang, Jens Axboe, Mike Christie, Neil Brown,
	Shaohua Li, Tomasz Majchrzak, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <20161006084949.GA3689@lst.de>

> This is not fine tuning but a waste of everyones time.

Does any of the published 54 update steps contain changes
which you might find also worthwhile for the affected source files
so that the shown software situation can be improved further?

Regards,
Markus

^ permalink raw reply

* [PATCH 54/54] md/raid10: Add some spaces for better code readability
From: SF Markus Elfring @ 2016-10-06  9:54 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 22:22:14 +0200

Use space characters at some source code places according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 71 +++++++++++++++++++++++++++--------------------------
 1 file changed, 36 insertions(+), 35 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 09c0e2f..786992c 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -224,7 +224,7 @@ static void r10buf_pool_free(void *__r10_bio, void *data)
 	struct r10bio *r10bio = __r10_bio;
 	int j;
 
-	for (j=0; j < conf->copies; j++) {
+	for (j = 0; j < conf->copies; j++) {
 		struct bio *bio = r10bio->devs[j].bio;
 		if (bio) {
 			for (i = 0; i < RESYNC_PAGES; i++) {
@@ -553,7 +553,7 @@ static void raid10_end_write_request(struct bio *bio)
 
 static void __raid10_find_phys(struct geom *geo, struct r10bio *r10bio)
 {
-	int n,f;
+	int n, f;
 	sector_t sector;
 	sector_t chunk;
 	sector_t stripe;
@@ -1937,7 +1937,7 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
 	atomic_set(&r10_bio->remaining, 1);
 
 	/* find the first device with a block */
-	for (i=0; i<conf->copies; i++)
+	for (i = 0; i < conf->copies; i++)
 		if (!r10_bio->devs[i].bio->bi_error)
 			break;
 
@@ -1951,7 +1951,7 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
 
 	vcnt = (r10_bio->sectors + (PAGE_SIZE >> 9) - 1) >> (PAGE_SHIFT - 9);
 	/* now find blocks with errors */
-	for (i=0 ; i < conf->copies ; i++) {
+	for (i = 0; i < conf->copies; i++) {
 		int  j, d;
 
 		tbio = r10_bio->devs[i].bio;
@@ -2236,7 +2236,7 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
 {
 	int sect = 0; /* Offset from r10_bio->sector */
 	int sectors = r10_bio->sectors;
-	struct md_rdev*rdev;
+	struct md_rdev *rdev;
 	int max_read_errors = atomic_read(&mddev->max_corr_read_errors);
 	int d = r10_bio->devs[r10_bio->read_slot].devnum;
 
@@ -2265,7 +2265,7 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
 		return;
 	}
 
-	while(sectors) {
+	while (sectors) {
 		int s = sectors;
 		int sl = r10_bio->read_slot;
 		int success = 0;
@@ -2331,7 +2331,7 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
 		while (sl != r10_bio->read_slot) {
 			char b[BDEVNAME_SIZE];
 
-			if (sl==0)
+			if (sl == 0)
 				sl = conf->copies;
 			sl--;
 			d = r10_bio->devs[sl].devnum;
@@ -2368,7 +2368,7 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
 		while (sl != r10_bio->read_slot) {
 			char b[BDEVNAME_SIZE];
 
-			if (sl==0)
+			if (sl == 0)
 				sl = conf->copies;
 			sl--;
 			d = r10_bio->devs[sl].devnum;
@@ -2996,7 +2996,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			raise_barrier(conf, rb2 != NULL);
 			atomic_set(&r10_bio->remaining, 0);
 
-			r10_bio->master_bio = (struct bio*)rb2;
+			r10_bio->master_bio = (struct bio *)rb2;
 			if (rb2)
 				atomic_inc(&rb2->remaining);
 			r10_bio->mddev = mddev;
@@ -3022,7 +3022,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 						      &sync_blocks, still_degraded);
 
 			any_working = 0;
-			for (j=0; j<conf->copies;j++) {
+			for (j = 0; j < conf->copies; j++) {
 				int k;
 				int d = r10_bio->devs[j].devnum;
 				sector_t from_addr, to_addr;
@@ -3063,7 +3063,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 				atomic_inc(&rdev->nr_pending);
 				/* and we write to 'i' (if not in_sync) */
 
-				for (k=0; k<conf->copies; k++)
+				for (k = 0; k < conf->copies; k++)
 					if (r10_bio->devs[k].devnum == i)
 						break;
 				BUG_ON(k == conf->copies);
@@ -3165,7 +3165,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 		if (!biolist) {
 			while (r10_bio) {
 				struct r10bio *rb2 = r10_bio;
-				r10_bio = (struct r10bio*) rb2->master_bio;
+
+				r10_bio = (struct r10bio *) rb2->master_bio;
 				rb2->master_bio = NULL;
 				put_buf(rb2);
 			}
@@ -3268,7 +3269,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 		}
 
 		if (count < 2) {
-			for (i=0; i<conf->copies; i++) {
+			for (i = 0; i < conf->copies; i++) {
 				int d = r10_bio->devs[i].devnum;
 				if (r10_bio->devs[i].bio->bi_end_io)
 					rdev_dec_pending(conf->mirrors[d].rdev,
@@ -3295,7 +3296,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			len = (max_sector - sector_nr) << 9;
 		if (len == 0)
 			break;
-		for (bio= biolist ; bio ; bio=bio->bi_next) {
+		for (bio = biolist; bio; bio = bio->bi_next) {
 			struct bio *bio2;
 			page = bio->bi_io_vec[bio->bi_vcnt].bv_page;
 			if (bio_add_page(bio, page, len, 0))
@@ -3765,7 +3766,7 @@ static void raid10_quiesce(struct mddev *mddev, int state)
 {
 	struct r10conf *conf = mddev->private;
 
-	switch(state) {
+	switch (state) {
 	case 1:
 		raise_barrier(conf, 0);
 		break;
@@ -4658,26 +4659,26 @@ static void raid10_finish_reshape(struct mddev *mddev)
 }
 
 static struct md_personality raid10_personality = {
-	.name		= "raid10",
-	.level		= 10,
-	.owner		= THIS_MODULE,
-	.make_request	= raid10_make_request,
-	.run		= raid10_run,
-	.free		= raid10_free,
-	.status		= raid10_status,
-	.error_handler	= raid10_error,
-	.hot_add_disk	= raid10_add_disk,
-	.hot_remove_disk= raid10_remove_disk,
-	.spare_active	= raid10_spare_active,
-	.sync_request	= raid10_sync_request,
-	.quiesce	= raid10_quiesce,
-	.size		= raid10_size,
-	.resize		= raid10_resize,
-	.takeover	= raid10_takeover,
-	.check_reshape	= raid10_check_reshape,
-	.start_reshape	= raid10_start_reshape,
-	.finish_reshape	= raid10_finish_reshape,
-	.congested	= raid10_congested,
+	.name            = "raid10",
+	.level           = 10,
+	.owner           = THIS_MODULE,
+	.make_request    = raid10_make_request,
+	.run             = raid10_run,
+	.free            = raid10_free,
+	.status          = raid10_status,
+	.error_handler   = raid10_error,
+	.hot_add_disk    = raid10_add_disk,
+	.hot_remove_disk = raid10_remove_disk,
+	.spare_active    = raid10_spare_active,
+	.sync_request    = raid10_sync_request,
+	.quiesce         = raid10_quiesce,
+	.size            = raid10_size,
+	.resize          = raid10_resize,
+	.takeover        = raid10_takeover,
+	.check_reshape   = raid10_check_reshape,
+	.start_reshape   = raid10_start_reshape,
+	.finish_reshape  = raid10_finish_reshape,
+	.congested       = raid10_congested,
 };
 
 static int __init raid_init(void)
-- 
2.10.1


^ permalink raw reply related

* [PATCH 53/54] md/raid10: Delete two unwanted spaces behind asterisks
From: SF Markus Elfring @ 2016-10-06  9:53 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 22:02:18 +0200

The script "checkpatch.pl" pointed information out like the following.

ERROR: "foo * bar" should be "foo *bar"

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 554b6d2..09c0e2f 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -107,7 +107,7 @@ static void reshape_request_write(struct mddev *mddev, struct r10bio *r10_bio);
 static void end_reshape_write(struct bio *bio);
 static void end_reshape(struct r10conf *conf);
 
-static void * r10bio_pool_alloc(gfp_t gfp_flags, void *data)
+static void *r10bio_pool_alloc(gfp_t gfp_flags, void *data)
 {
 	struct r10conf *conf = data;
 	int size = offsetof(struct r10bio, devs[conf->copies]);
@@ -137,7 +137,7 @@ static void r10bio_pool_free(void *r10_bio, void *data)
  * one for write (we recover only one drive per r10buf)
  *
  */
-static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
+static void *r10buf_pool_alloc(gfp_t gfp_flags, void *data)
 {
 	struct r10conf *conf = data;
 	struct page *page;
-- 
2.10.1

^ permalink raw reply related

* [PATCH 52/54] md/raid10: Replace a seq_printf() call by seq_puts() in raid10_status()
From: SF Markus Elfring @ 2016-10-06  9:52 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 21:41:09 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: Prefer seq_puts to seq_printf

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 82d79f5..554b6d2 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1502,7 +1502,7 @@ static void raid10_status(struct seq_file *seq, struct mddev *mddev)
 		seq_printf(seq, "%s", rdev && test_bit(In_sync, &rdev->flags) ? "U" : "_");
 	}
 	rcu_read_unlock();
-	seq_printf(seq, "]");
+	seq_puts(seq, "]");
 }
 
 /* check if there are enough drives for
-- 
2.10.1

^ permalink raw reply related

* [PATCH 51/54] md/raid10: Adjust 22 checks for null pointers
From: SF Markus Elfring @ 2016-10-06  9:51 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 21:36:43 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script "checkpatch.pl" pointed information out like the following.

Comparison to NULL could be written …

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 2181f53..82d79f5 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -734,10 +734,10 @@ static struct md_rdev *read_balance(struct r10conf *conf,
 			continue;
 		disk = r10_bio->devs[slot].devnum;
 		rdev = rcu_dereference(conf->mirrors[disk].replacement);
-		if (rdev == NULL || test_bit(Faulty, &rdev->flags) ||
+		if (!rdev || test_bit(Faulty, &rdev->flags) ||
 		    r10_bio->devs[slot].addr + sectors > rdev->recovery_offset)
 			rdev = rcu_dereference(conf->mirrors[disk].rdev);
-		if (rdev == NULL ||
+		if (!rdev ||
 		    test_bit(Faulty, &rdev->flags))
 			continue;
 		if (!test_bit(In_sync, &rdev->flags) &&
@@ -1386,7 +1386,8 @@ static void __make_request(struct mddev *mddev, struct bio *bio)
 
 		if (r10_bio->devs[i].repl_bio) {
 			struct md_rdev *rdev = conf->mirrors[d].replacement;
-			if (rdev == NULL) {
+
+			if (!rdev) {
 				/* Replacement just got moved to main 'rdev' */
 				smp_mb();
 				rdev = conf->mirrors[d].rdev;
@@ -1701,7 +1702,7 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
 		first = last = rdev->raid_disk;
 
 	if (rdev->saved_raid_disk >= first &&
-	    conf->mirrors[rdev->saved_raid_disk].rdev == NULL)
+	    !conf->mirrors[rdev->saved_raid_disk].rdev)
 		mirror = rdev->saved_raid_disk;
 	else
 		mirror = first;
@@ -1711,7 +1712,7 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
 			continue;
 		if (p->rdev) {
 			if (!test_bit(WantReplacement, &p->rdev->flags) ||
-			    p->replacement != NULL)
+			    p->replacement)
 				continue;
 			clear_bit(In_sync, &rdev->flags);
 			set_bit(Replacement, &rdev->flags);
@@ -1849,7 +1850,7 @@ static void end_sync_request(struct r10bio *r10_bio)
 	struct mddev *mddev = r10_bio->mddev;
 
 	while (atomic_dec_and_test(&r10_bio->remaining)) {
-		if (r10_bio->master_bio == NULL) {
+		if (!r10_bio->master_bio) {
 			/* the primary of several recovery bios */
 			sector_t s = r10_bio->sectors;
 			if (test_bit(R10BIO_MadeGood, &r10_bio->state) ||
@@ -2515,7 +2516,7 @@ static void handle_read_error(struct mddev *mddev, struct r10bio *r10_bio)
 
 read_more:
 	rdev = read_balance(conf, r10_bio, &max_sectors);
-	if (rdev == NULL) {
+	if (!rdev) {
 		pr_alert("%s: %s: unrecoverable I/O read error for block %llu\n",
 			 mdname(mddev), b,
 			 (unsigned long long)r10_bio->sector);
@@ -2587,7 +2588,7 @@ static void handle_write_completed(struct r10conf *conf, struct r10bio *r10_bio)
 		for (m = 0; m < conf->copies; m++) {
 			int dev = r10_bio->devs[m].devnum;
 			rdev = conf->mirrors[dev].rdev;
-			if (r10_bio->devs[m].bio == NULL)
+			if (!r10_bio->devs[m].bio)
 				continue;
 			if (!r10_bio->devs[m].bio->bi_error) {
 				rdev_clear_badblocks(
@@ -2602,7 +2603,7 @@ static void handle_write_completed(struct r10conf *conf, struct r10bio *r10_bio)
 					md_error(conf->mddev, rdev);
 			}
 			rdev = conf->mirrors[dev].replacement;
-			if (r10_bio->devs[m].repl_bio == NULL)
+			if (!r10_bio->devs[m].repl_bio)
 				continue;
 
 			if (!r10_bio->devs[m].repl_bio->bi_error) {
@@ -2631,7 +2632,7 @@ static void handle_write_completed(struct r10conf *conf, struct r10bio *r10_bio)
 					r10_bio->devs[m].addr,
 					r10_bio->sectors, 0);
 				rdev_dec_pending(rdev, conf->mddev);
-			} else if (bio != NULL && bio->bi_error) {
+			} else if (bio && bio->bi_error) {
 				fail = true;
 				if (!narrow_write_error(r10_bio, m)) {
 					md_error(conf->mddev, rdev);
@@ -2816,7 +2817,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 	 * Allow skipping a full rebuild for incremental assembly
 	 * of a clean array, like RAID1 does.
 	 */
-	if (mddev->bitmap == NULL &&
+	if (!mddev->bitmap &&
 	    mddev->recovery_cp == MaxSector &&
 	    mddev->reshape_position == MaxSector &&
 	    !test_bit(MD_RECOVERY_SYNC, &mddev->recovery) &&
@@ -2945,10 +2946,10 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			mrdev = rcu_dereference(mirror->rdev);
 			mreplace = rcu_dereference(mirror->replacement);
 
-			if ((mrdev == NULL ||
+			if ((!mrdev ||
 			     test_bit(Faulty, &mrdev->flags) ||
 			     test_bit(In_sync, &mrdev->flags)) &&
-			    (mreplace == NULL ||
+			    (!mreplace ||
 			     test_bit(Faulty, &mreplace->flags))) {
 				rcu_read_unlock();
 				continue;
@@ -2976,7 +2977,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			if (sync_blocks < max_sync)
 				max_sync = sync_blocks;
 			if (!must_sync &&
-			    mreplace == NULL &&
+			    !mreplace &&
 			    !conf->fullsync) {
 				/* yep, skip the sync_blocks here, but don't assume
 				 * that there will never be anything to do here
@@ -3011,7 +3012,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			for (j = 0; j < conf->geo.raid_disks; j++) {
 				struct md_rdev *rdev = rcu_dereference(
 					conf->mirrors[j].rdev);
-				if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
+				if (!rdev || test_bit(Faulty, &rdev->flags)) {
 					still_degraded = 1;
 					break;
 				}
@@ -3099,7 +3100,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 				 * this comment keeps human reviewers
 				 * happy.
 				 */
-				if (mreplace == NULL || bio == NULL ||
+				if (!mreplace || !bio ||
 				    test_bit(Faulty, &mreplace->flags))
 					break;
 				bio_reset(bio);
@@ -3161,7 +3162,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			if (mreplace)
 				rdev_dec_pending(mreplace, mddev);
 		}
-		if (biolist == NULL) {
+		if (!biolist) {
 			while (r10_bio) {
 				struct r10bio *rb2 = r10_bio;
 				r10_bio = (struct r10bio*) rb2->master_bio;
@@ -3214,7 +3215,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			bio->bi_error = -EIO;
 			rcu_read_lock();
 			rdev = rcu_dereference(conf->mirrors[d].rdev);
-			if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
+			if (!rdev || test_bit(Faulty, &rdev->flags)) {
 				rcu_read_unlock();
 				continue;
 			}
@@ -3243,7 +3244,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			count++;
 
 			rdev = rcu_dereference(conf->mirrors[d].replacement);
-			if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
+			if (!rdev || test_bit(Faulty, &rdev->flags)) {
 				rcu_read_unlock();
 				continue;
 			}
@@ -3565,7 +3566,7 @@ static int raid10_run(struct mddev *mddev)
 	int first = 1;
 	bool discard_supported = false;
 
-	if (mddev->private == NULL) {
+	if (!mddev->private) {
 		conf = setup_conf(mddev);
 		if (IS_ERR(conf))
 			return PTR_ERR(conf);
@@ -4542,7 +4543,7 @@ static int handle_reshape_read_error(struct mddev *mddev,
 			int d = r10b->devs[slot].devnum;
 			struct md_rdev *rdev = rcu_dereference(conf->mirrors[d].rdev);
 			sector_t addr;
-			if (rdev == NULL ||
+			if (!rdev ||
 			    test_bit(Faulty, &rdev->flags) ||
 			    !test_bit(In_sync, &rdev->flags))
 				goto failed;
-- 
2.10.1

^ permalink raw reply related

* [PATCH 50/54] md/raid10: Delete indentation for one jump label
From: SF Markus Elfring @ 2016-10-06  9:50 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 21:07:35 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: labels should not be indented

Thus fix the affected source code place.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0f2cb20..2181f53 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4559,7 +4559,7 @@ static int handle_reshape_read_error(struct mddev *mddev,
 			rcu_read_lock();
 			if (success)
 				break;
-		failed:
+failed:
 			slot++;
 			if (slot >= conf->copies)
 				slot = 0;
-- 
2.10.1


^ permalink raw reply related

* [PATCH 49/54] md/raid10: Replace printk() calls by the usage of higher level interfaces
From: SF Markus Elfring @ 2016-10-06  9:49 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 21:00:05 +0200

1. Add a definition for the macros "MY_LOG_PREFIX" and "pr_fmt"
   so that their information can be used for consistent message output.

2. Prefer usage of some higher level macros over calling "printk" directly
   in this software module.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 168 +++++++++++++++++++++++-----------------------------
 1 file changed, 74 insertions(+), 94 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 5119846..0f2cb20 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -18,6 +18,8 @@
  * Software Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#define MY_LOG_PREFIX KBUILD_MODNAME ": "
+#define pr_fmt(fmt) MY_LOG_PREFIX fmt
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/blkdev.h>
@@ -404,8 +406,8 @@ static void raid10_end_read_request(struct bio *bio)
 		 * oops, read error - keep the refcount on the rdev
 		 */
 		char b[BDEVNAME_SIZE];
-		printk_ratelimited(KERN_ERR
-				   "md/raid10:%s: %s: rescheduling sector %llu\n",
+
+		pr_err_ratelimited("%s: %s: rescheduling sector %llu\n",
 				   mdname(conf->mddev),
 				   bdevname(rdev->bdev, b),
 				   (unsigned long long)r10_bio->sector);
@@ -1586,11 +1588,10 @@ static void raid10_error(struct mddev *mddev, struct md_rdev *rdev)
 	set_mask_bits(&mddev->flags, 0,
 		      BIT(MD_CHANGE_DEVS) | BIT(MD_CHANGE_PENDING));
 	spin_unlock_irqrestore(&conf->device_lock, flags);
-	printk(KERN_ALERT
-	       "md/raid10:%s: Disk failure on %s, disabling device.\n"
-	       "md/raid10:%s: Operation continuing on %d devices.\n",
-	       mdname(mddev), bdevname(rdev->bdev, b),
-	       mdname(mddev), conf->geo.raid_disks - mddev->degraded);
+	pr_alert("%s: Disk failure on %s, disabling device.\n"
+		 MY_LOG_PREFIX "%s: Operation continuing on %d devices.\n",
+		 mdname(mddev), bdevname(rdev->bdev, b),
+		 mdname(mddev), conf->geo.raid_disks - mddev->degraded);
 }
 
 static void print_conf(struct r10conf *conf)
@@ -1598,13 +1599,14 @@ static void print_conf(struct r10conf *conf)
 	int i;
 	struct md_rdev *rdev;
 
-	printk(KERN_DEBUG "RAID10 conf printout:\n");
+	pr_debug("conf printout:\n");
 	if (!conf) {
-		printk(KERN_DEBUG "(!conf)\n");
+		pr_debug("(!conf)\n");
 		return;
 	}
-	printk(KERN_DEBUG " --- wd:%d rd:%d\n", conf->geo.raid_disks - conf->mddev->degraded,
-		conf->geo.raid_disks);
+	pr_debug("--- wd:%d rd:%d\n",
+		 conf->geo.raid_disks - conf->mddev->degraded,
+		 conf->geo.raid_disks);
 
 	/* This is only called with ->reconfix_mutex held, so
 	 * rcu protection of rdev is not needed */
@@ -1612,10 +1614,10 @@ static void print_conf(struct r10conf *conf)
 		char b[BDEVNAME_SIZE];
 		rdev = conf->mirrors[i].rdev;
 		if (rdev)
-			printk(KERN_DEBUG " disk %d, wo:%d, o:%d, dev:%s\n",
-				i, !test_bit(In_sync, &rdev->flags),
-			        !test_bit(Faulty, &rdev->flags),
-				bdevname(rdev->bdev,b));
+			pr_debug("disk %d, wo:%d, o:%d, dev:%s\n",
+				 i, !test_bit(In_sync, &rdev->flags),
+				 !test_bit(Faulty, &rdev->flags),
+				 bdevname(rdev->bdev, b));
 	}
 }
 
@@ -2106,11 +2108,8 @@ static void fix_recovery_read_error(struct r10bio *r10_bio)
 				ok = rdev_set_badblocks(rdev2, addr, s, 0);
 				if (!ok) {
 					/* just abort the recovery */
-					printk(KERN_NOTICE
-					       "md/raid10:%s: recovery aborted"
-					       " due to read error\n",
-					       mdname(mddev));
-
+					pr_notice("%s: recovery aborted due to read error\n",
+						  mdname(mddev));
 					conf->mirrors[dw].recovery_disabled
 						= mddev->recovery_disabled;
 					set_bit(MD_RECOVERY_INTR,
@@ -2256,14 +2255,10 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
 		char b[BDEVNAME_SIZE];
 		bdevname(rdev->bdev, b);
 
-		printk(KERN_NOTICE
-		       "md/raid10:%s: %s: Raid device exceeded "
-		       "read_error threshold [cur %d:max %d]\n",
-		       mdname(mddev), b,
-		       atomic_read(&rdev->read_errors), max_read_errors);
-		printk(KERN_NOTICE
-		       "md/raid10:%s: %s: Failing raid device\n",
-		       mdname(mddev), b);
+		pr_notice("%s: %s: Raid device exceeded read_error threshold [cur %d:max %d]\n",
+			  mdname(mddev), b,
+			  atomic_read(&rdev->read_errors), max_read_errors);
+		pr_notice("%s: %s: Failing raid device\n", mdname(mddev), b);
 		md_error(mddev, rdev);
 		r10_bio->devs[r10_bio->read_slot].bio = IO_BLOCKED;
 		return;
@@ -2353,20 +2348,17 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
 					     s, conf->tmppage, WRITE)
 			    == 0) {
 				/* Well, this device is dead */
-				printk(KERN_NOTICE
-				       "md/raid10:%s: read correction "
-				       "write failed"
-				       " (%d sectors at %llu on %s)\n",
-				       mdname(mddev), s,
-				       (unsigned long long)(
-					       sect +
-					       choose_data_offset(r10_bio,
-								  rdev)),
-				       bdevname(rdev->bdev, b));
-				printk(KERN_NOTICE "md/raid10:%s: %s: failing "
-				       "drive\n",
-				       mdname(mddev),
-				       bdevname(rdev->bdev, b));
+				pr_notice("%s: read correction write failed ("
+					  "%d sectors at %llu on %s)\n",
+					  mdname(mddev),
+					  s,
+					  (unsigned long long)
+					  (sect
+					  + choose_data_offset(r10_bio, rdev)),
+					  bdevname(rdev->bdev, b));
+				pr_notice("%s: %s: failing drive\n",
+					  mdname(mddev),
+					  bdevname(rdev->bdev, b));
 			}
 			rdev_dec_pending(rdev, mddev);
 			rcu_read_lock();
@@ -2394,29 +2386,27 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
 						 READ)) {
 			case 0:
 				/* Well, this device is dead */
-				printk(KERN_NOTICE
-				       "md/raid10:%s: unable to read back "
-				       "corrected sectors"
-				       " (%d sectors at %llu on %s)\n",
-				       mdname(mddev), s,
-				       (unsigned long long)(
-					       sect +
-					       choose_data_offset(r10_bio, rdev)),
-				       bdevname(rdev->bdev, b));
-				printk(KERN_NOTICE "md/raid10:%s: %s: failing "
-				       "drive\n",
-				       mdname(mddev),
-				       bdevname(rdev->bdev, b));
+				pr_notice("%s: unable to read back corrected sectors ("
+					  "%d sectors at %llu on %s)\n",
+					  mdname(mddev),
+					  s,
+					  (unsigned long long)
+					  (sect
+					  + choose_data_offset(r10_bio, rdev)),
+					  bdevname(rdev->bdev, b));
+				pr_notice("%s: %s: failing drive\n",
+					  mdname(mddev),
+					  bdevname(rdev->bdev, b));
 				break;
 			case 1:
-				printk(KERN_INFO
-				       "md/raid10:%s: read error corrected"
-				       " (%d sectors at %llu on %s)\n",
-				       mdname(mddev), s,
-				       (unsigned long long)(
-					       sect +
-					       choose_data_offset(r10_bio, rdev)),
-				       bdevname(rdev->bdev, b));
+				pr_info("%s: read error corrected ("
+					"%d sectors at %llu on %s)\n",
+					mdname(mddev),
+					s,
+					(unsigned long long)
+					(sect
+					+ choose_data_offset(r10_bio, rdev)),
+					bdevname(rdev->bdev, b));
 				atomic_add(s, &rdev->corrected_errors);
 			}
 
@@ -2526,23 +2516,19 @@ static void handle_read_error(struct mddev *mddev, struct r10bio *r10_bio)
 read_more:
 	rdev = read_balance(conf, r10_bio, &max_sectors);
 	if (rdev == NULL) {
-		printk(KERN_ALERT "md/raid10:%s: %s: unrecoverable I/O"
-		       " read error for block %llu\n",
-		       mdname(mddev), b,
-		       (unsigned long long)r10_bio->sector);
+		pr_alert("%s: %s: unrecoverable I/O read error for block %llu\n",
+			 mdname(mddev), b,
+			 (unsigned long long)r10_bio->sector);
 		raid_end_bio_io(r10_bio);
 		return;
 	}
 
 	do_sync = (r10_bio->master_bio->bi_opf & REQ_SYNC);
 	slot = r10_bio->read_slot;
-	printk_ratelimited(
-		KERN_ERR
-		"md/raid10:%s: %s: redirecting "
-		"sector %llu to another mirror\n",
-		mdname(mddev),
-		bdevname(rdev->bdev, b),
-		(unsigned long long)r10_bio->sector);
+	pr_err_ratelimited("%s: %s: redirecting sector %llu to another mirror\n",
+			   mdname(mddev),
+			   bdevname(rdev->bdev, b),
+			   (unsigned long long)r10_bio->sector);
 	bio = bio_clone_mddev(r10_bio->master_bio,
 			      GFP_NOIO, mddev);
 	bio_trim(bio, r10_bio->sector - bio->bi_iter.bi_sector, max_sectors);
@@ -3157,9 +3143,8 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 				if (!any_working)  {
 					if (!test_and_set_bit(MD_RECOVERY_INTR,
 							      &mddev->recovery))
-						printk(KERN_INFO "md/raid10:%s: insufficient "
-						       "working devices for recovery.\n",
-						       mdname(mddev));
+						pr_info("%s: insufficient working devices for recovery.\n",
+							mdname(mddev));
 					mirror->recovery_disabled
 						= mddev->recovery_disabled;
 				}
@@ -3486,14 +3471,13 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	copies = setup_geo(&geo, mddev, geo_new);
 
 	if (copies == -2) {
-		printk(KERN_ERR "md/raid10:%s: chunk size must be "
-		       "at least PAGE_SIZE(%ld) and be a power of 2.\n",
+		pr_err("%s: chunk size must be at least PAGE_SIZE(%ld) and be a power of 2.\n",
 		       mdname(mddev), PAGE_SIZE);
 		return ERR_PTR(-EINVAL);
 	}
 
 	if (copies < 2 || copies > mddev->raid_disks) {
-		printk(KERN_ERR "md/raid10:%s: unsupported raid10 layout: 0x%8x\n",
+		pr_err("%s: unsupported raid10 layout: 0x%8x\n",
 		       mdname(mddev), mddev->new_layout);
 		return ERR_PTR(-EINVAL);
 	}
@@ -3657,8 +3641,7 @@ static int raid10_run(struct mddev *mddev)
 	}
 	/* need to check that every block has at least one working mirror */
 	if (!enough(conf, -1)) {
-		printk(KERN_ERR "md/raid10:%s: not enough operational mirrors.\n",
-		       mdname(mddev));
+		pr_err("%s: not enough operational mirrors.\n", mdname(mddev));
 		goto out_free_conf;
 	}
 
@@ -3699,12 +3682,11 @@ static int raid10_run(struct mddev *mddev)
 	}
 
 	if (mddev->recovery_cp != MaxSector)
-		printk(KERN_NOTICE "md/raid10:%s: not clean"
-		       " -- starting background reconstruction\n",
-		       mdname(mddev));
-	printk(KERN_INFO
-		"md/raid10:%s: active with %d out of %d devices\n",
-		mdname(mddev), conf->geo.raid_disks - mddev->degraded,
+		pr_notice("%s: not clean - starting background reconstruction\n",
+			  mdname(mddev));
+	pr_info("%s: active with %d out of %d devices\n",
+		mdname(mddev),
+		conf->geo.raid_disks - mddev->degraded,
 		conf->geo.raid_disks);
 	/*
 	 * Ok, everything is just fine now
@@ -3740,7 +3722,7 @@ static int raid10_run(struct mddev *mddev)
 
 		if (max(before_length, after_length) > min_offset_diff) {
 			/* This cannot work */
-			printk("md/raid10: offset difference not enough to continue reshape\n");
+			pr_notice("offset difference not enough to continue reshape\n");
 			goto out_free_conf;
 		}
 		conf->offset_diff = min_offset_diff;
@@ -3847,8 +3829,7 @@ static void *raid10_takeover_raid0(struct mddev *mddev, sector_t size, int devs)
 	struct r10conf *conf;
 
 	if (mddev->degraded > 0) {
-		printk(KERN_ERR "md/raid10:%s: Error: degraded raid0!\n",
-		       mdname(mddev));
+		pr_err("%s: Error: degraded raid0!\n", mdname(mddev));
 		return ERR_PTR(-EINVAL);
 	}
 	sector_div(size, devs);
@@ -3888,8 +3869,7 @@ static void *raid10_takeover(struct mddev *mddev)
 		/* for raid0 takeover only one zone is supported */
 		raid0_conf = mddev->private;
 		if (raid0_conf->nr_strip_zones > 1) {
-			printk(KERN_ERR "md/raid10:%s: cannot takeover raid 0"
-			       " with more than one zone.\n",
+			pr_err("%s: cannot takeover raid 0 with more than one zone.\n",
 			       mdname(mddev));
 			return ERR_PTR(-EINVAL);
 		}
@@ -4078,7 +4058,7 @@ static int raid10_start_reshape(struct mddev *mddev)
 		sector_t size = raid10_size(mddev, 0, 0);
 		if (size < mddev->array_sectors) {
 			spin_unlock_irq(&conf->device_lock);
-			printk(KERN_ERR "md/raid10:%s: array size must be reduce before number of disks\n",
+			pr_err("%s: array size must be reduced before number of disks\n",
 			       mdname(mddev));
 			return -EINVAL;
 		}
-- 
2.10.1

^ permalink raw reply related

* [PATCH 48/54] md/raid10: Move a brace for a designated initialiser
From: SF Markus Elfring @ 2016-10-06  9:48 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 20:03:32 +0200

The script "checkpatch.pl" pointed information out like the following.

ERROR: that open brace { should be on the previous line

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 62cd159..5119846 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4676,8 +4676,7 @@ static void raid10_finish_reshape(struct mddev *mddev)
 	mddev->reshape_backwards = 0;
 }
 
-static struct md_personality raid10_personality =
-{
+static struct md_personality raid10_personality = {
 	.name		= "raid10",
 	.level		= 10,
 	.owner		= THIS_MODULE,
-- 
2.10.1


^ permalink raw reply related

* [PATCH 47/54] md/raid10: Improve another size determination in raid10_start_reshape()
From: SF Markus Elfring @ 2016-10-06  9:47 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 18:48:17 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 9b8d11f..62cd159 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4065,7 +4065,7 @@ static int raid10_start_reshape(struct mddev *mddev)
 	spin_lock_irq(&conf->device_lock);
 	if (conf->mirrors_new) {
 		memcpy(conf->mirrors_new, conf->mirrors,
-		       sizeof(struct raid10_info)*conf->prev.raid_disks);
+		       sizeof(*conf->mirrors_new) * conf->prev.raid_disks);
 		smp_mb();
 		kfree(conf->mirrors_old);
 		conf->mirrors_old = conf->mirrors;
-- 
2.10.1

^ permalink raw reply related

* [PATCH 46/54] md/raid10: Less function calls in setup_conf() after error detection
From: SF Markus Elfring @ 2016-10-06  9:46 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 18:32:30 +0200

Resource release functions were called in up to three cases
by the setup_conf() function during error handling even if
the passed data structure members contained a null pointer.

Adjust jump targets according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 7e512d4..9b8d11f 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3508,13 +3508,13 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 				GFP_KERNEL);
 	if (!conf->mirrors) {
 		err = -ENOMEM;
-		goto out;
+		goto free_conf;
 	}
 
 	conf->tmppage = alloc_page(GFP_KERNEL);
 	if (!conf->tmppage) {
 		err = -ENOMEM;
-		goto out;
+		goto kfree_mirrors;
 	}
 
 	conf->geo = geo;
@@ -3523,7 +3523,7 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 					   r10bio_pool_free, conf);
 	if (!conf->r10bio_pool) {
 		err = -ENOMEM;
-		goto out;
+		goto put_page;
 	}
 
 	calc_sectors(conf, mddev->dev_sectors);
@@ -3533,7 +3533,7 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	} else {
 		if (setup_geo(&conf->prev, mddev, geo_old) != conf->copies) {
 			err = -EINVAL;
-			goto out;
+			goto destroy_pool;
 		}
 		conf->reshape_progress = mddev->reshape_position;
 		if (conf->prev.far_offset)
@@ -3554,16 +3554,18 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	conf->thread = md_register_thread(raid10d, mddev, "raid10");
 	if (!conf->thread) {
 		err = -ENOMEM;
-		goto out;
+		goto destroy_pool;
 	}
 
 	conf->mddev = mddev;
 	return conf;
-
- out:
+destroy_pool:
 	mempool_destroy(conf->r10bio_pool);
+put_page:
 	safe_put_page(conf->tmppage);
+kfree_mirrors:
 	kfree(conf->mirrors);
+free_conf:
 	kfree(conf);
 	return ERR_PTR(err);
 }
-- 
2.10.1

^ permalink raw reply related

* [PATCH 45/54] md/raid10: Move assignments for the variable "err" in setup_conf()
From: SF Markus Elfring @ 2016-10-06  9:45 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 18:26:45 +0200

One local variable was set to an error code before a concrete
error situation was detected. Thus move the corresponding assignments
into if branches to indicate a memory allocation failure there.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index abe75c2..7e512d4 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3498,7 +3498,6 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 		return ERR_PTR(-EINVAL);
 	}
 
-	err = -ENOMEM;
 	conf = kzalloc(sizeof(*conf), GFP_KERNEL);
 	if (!conf)
 		return ERR_PTR(-ENOMEM);
@@ -3507,19 +3506,25 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	conf->mirrors = kcalloc(mddev->raid_disks + max(0, -mddev->delta_disks),
 				sizeof(*conf->mirrors),
 				GFP_KERNEL);
-	if (!conf->mirrors)
+	if (!conf->mirrors) {
+		err = -ENOMEM;
 		goto out;
+	}
 
 	conf->tmppage = alloc_page(GFP_KERNEL);
-	if (!conf->tmppage)
+	if (!conf->tmppage) {
+		err = -ENOMEM;
 		goto out;
+	}
 
 	conf->geo = geo;
 	conf->copies = copies;
 	conf->r10bio_pool = mempool_create(NR_RAID10_BIOS, r10bio_pool_alloc,
 					   r10bio_pool_free, conf);
-	if (!conf->r10bio_pool)
+	if (!conf->r10bio_pool) {
+		err = -ENOMEM;
 		goto out;
+	}
 
 	calc_sectors(conf, mddev->dev_sectors);
 	if (mddev->reshape_position == MaxSector) {
@@ -3547,8 +3552,10 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	atomic_set(&conf->nr_pending, 0);
 
 	conf->thread = md_register_thread(raid10d, mddev, "raid10");
-	if (!conf->thread)
+	if (!conf->thread) {
+		err = -ENOMEM;
 		goto out;
+	}
 
 	conf->mddev = mddev;
 	return conf;
-- 
2.10.1

^ permalink raw reply related

* [PATCH 44/54] md/raid10: Return directly after a failed kzalloc() in setup_conf()
From: SF Markus Elfring @ 2016-10-06  9:44 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 17:46:02 +0200

* Return directly after a call of the function "kzalloc" failed
  at the beginning.

* Delete a repeated check for the local variable "conf"
  which became unnecessary with this refactoring.

* Reorder calls for the functions "kfree" and "safe_put_page"
  at the end.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 8326e68..abe75c2 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3501,7 +3501,7 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	err = -ENOMEM;
 	conf = kzalloc(sizeof(*conf), GFP_KERNEL);
 	if (!conf)
-		goto out;
+		return ERR_PTR(-ENOMEM);
 
 	/* FIXME calc properly */
 	conf->mirrors = kcalloc(mddev->raid_disks + max(0, -mddev->delta_disks),
@@ -3554,12 +3554,10 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	return conf;
 
  out:
-	if (conf) {
-		mempool_destroy(conf->r10bio_pool);
-		kfree(conf->mirrors);
-		safe_put_page(conf->tmppage);
-		kfree(conf);
-	}
+	mempool_destroy(conf->r10bio_pool);
+	safe_put_page(conf->tmppage);
+	kfree(conf->mirrors);
+	kfree(conf);
 	return ERR_PTR(err);
 }
 
-- 
2.10.1

^ permalink raw reply related

* [PATCH 43/54] md/raid10: Return directly after detection of unsupported settings in setup_conf()
From: SF Markus Elfring @ 2016-10-06  9:43 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 17:32:49 +0200

* Return directly after unsupported system settings were detected
  at the beginning.

* Delete the explicit initialisation for the local variables "conf"
  and "err" which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 1f712f7..8326e68 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3478,8 +3478,8 @@ static int setup_geo(struct geom *geo, struct mddev *mddev, enum geo_type new)
 
 static struct r10conf *setup_conf(struct mddev *mddev)
 {
-	struct r10conf *conf = NULL;
-	int err = -EINVAL;
+	struct r10conf *conf;
+	int err;
 	struct geom geo;
 	int copies;
 
@@ -3489,13 +3489,13 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 		printk(KERN_ERR "md/raid10:%s: chunk size must be "
 		       "at least PAGE_SIZE(%ld) and be a power of 2.\n",
 		       mdname(mddev), PAGE_SIZE);
-		goto out;
+		return ERR_PTR(-EINVAL);
 	}
 
 	if (copies < 2 || copies > mddev->raid_disks) {
 		printk(KERN_ERR "md/raid10:%s: unsupported raid10 layout: 0x%8x\n",
 		       mdname(mddev), mddev->new_layout);
-		goto out;
+		return ERR_PTR(-EINVAL);
 	}
 
 	err = -ENOMEM;
-- 
2.10.1

^ permalink raw reply related

* [PATCH 42/54] md/raid10: Delete an error message for a failed memory allocation
From: SF Markus Elfring @ 2016-10-06  9:42 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall, Wolfram Sang
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 16:57:19 +0200

Omit an extra message for a memory allocation failure
(and another corresponding condition check) 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/raid10.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 04e8f78..1f712f7 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3554,9 +3554,6 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	return conf;
 
  out:
-	if (err == -ENOMEM)
-		printk(KERN_ERR "md/raid10:%s: couldn't allocate memory.\n",
-		       mdname(mddev));
 	if (conf) {
 		mempool_destroy(conf->r10bio_pool);
 		kfree(conf->mirrors);
-- 
2.10.1

^ permalink raw reply related

* [PATCH 41/54] md/raid10: Improve another size determination in setup_conf()
From: SF Markus Elfring @ 2016-10-06  9:41 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 16:51:52 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 17352a9..04e8f78 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3499,7 +3499,7 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	}
 
 	err = -ENOMEM;
-	conf = kzalloc(sizeof(struct r10conf), GFP_KERNEL);
+	conf = kzalloc(sizeof(*conf), GFP_KERNEL);
 	if (!conf)
 		goto out;
 
-- 
2.10.1

^ permalink raw reply related

* [PATCH 40/54] md/raid10: Use kcalloc() in two functions
From: SF Markus Elfring @ 2016-10-06  9:40 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 16:45:05 +0200

* Multiplications for the size determination of memory allocations
  indicated that array data structures should be processed.
  Thus use the corresponding function "kcalloc".

  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/raid10.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index be1a9fc..17352a9 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3504,8 +3504,8 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 		goto out;
 
 	/* FIXME calc properly */
-	conf->mirrors = kzalloc(sizeof(struct raid10_info)*(mddev->raid_disks +
-							    max(0,-mddev->delta_disks)),
+	conf->mirrors = kcalloc(mddev->raid_disks + max(0, -mddev->delta_disks),
+				sizeof(*conf->mirrors),
 				GFP_KERNEL);
 	if (!conf->mirrors)
 		goto out;
@@ -3936,11 +3936,10 @@ static int raid10_check_reshape(struct mddev *mddev)
 	conf->mirrors_new = NULL;
 	if (mddev->delta_disks > 0) {
 		/* allocate new 'mirrors' list */
-		conf->mirrors_new = kzalloc(
-			sizeof(struct raid10_info)
-			*(mddev->raid_disks +
-			  mddev->delta_disks),
-			GFP_KERNEL);
+		conf->mirrors_new = kcalloc(mddev->raid_disks
+					    + mddev->delta_disks,
+					    sizeof(*conf->mirrors_new),
+					    GFP_KERNEL);
 		if (!conf->mirrors_new)
 			return -ENOMEM;
 	}
-- 
2.10.1

^ permalink raw reply related

* [PATCH 39/54] md/raid5: Add some spaces for better code readability
From: SF Markus Elfring @ 2016-10-06  9:39 UTC (permalink / raw)
  To: linux-raid, Christoph Hellwig, Guoqing Jiang, Jens Axboe,
	Mike Christie, Neil Brown, Shaohua Li, Tomasz Majchrzak
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <786843ef-4b6f-eb04-7326-2f6f5b408826@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 5 Oct 2016 16:00:32 +0200

Use space characters at some source code places according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid5.c | 165 +++++++++++++++++++++++++++--------------------------
 1 file changed, 83 insertions(+), 82 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 5bcc1ff..e29c198 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -292,7 +292,7 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
 			      struct list_head *temp_inactive_list)
 {
 	BUG_ON(!list_empty(&sh->lru));
-	BUG_ON(atomic_read(&conf->active_stripes)==0);
+	BUG_ON(atomic_read(&conf->active_stripes) == 0);
 	if (test_bit(STRIPE_HANDLE, &sh->state)) {
 		if (test_bit(STRIPE_DELAYED, &sh->state) &&
 		    !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
@@ -2224,7 +2224,7 @@ static int resize_stripes(struct r5conf *conf, int newsize)
 		osh = get_free_stripe(conf, hash);
 		unlock_device_hash_lock(conf, hash);
 
-		for(i=0; i<conf->pool_size; i++) {
+		for (i = 0; i < conf->pool_size; i++) {
 			nsh->dev[i].page = osh->dev[i].page;
 			nsh->dev[i].orig_page = osh->dev[i].page;
 		}
@@ -2246,7 +2246,7 @@ static int resize_stripes(struct r5conf *conf, int newsize)
 	 */
 	ndisks = kcalloc(newsize, sizeof(*ndisks), GFP_NOIO);
 	if (ndisks) {
-		for (i=0; i<conf->raid_disks; i++)
+		for (i = 0; i < conf->raid_disks; i++)
 			ndisks[i] = conf->disks[i];
 		kfree(conf->disks);
 		conf->disks = ndisks;
@@ -2255,11 +2255,11 @@ static int resize_stripes(struct r5conf *conf, int newsize)
 
 	mutex_unlock(&conf->cache_size_mutex);
 	/* Step 4, return new stripes to service */
-	while(!list_empty(&newstripes)) {
+	while (!list_empty(&newstripes)) {
 		nsh = list_entry(newstripes.next, struct stripe_head, lru);
 		list_del_init(&nsh->lru);
 
-		for (i=conf->raid_disks; i < newsize; i++)
+		for (i = conf->raid_disks; i < newsize; i++)
 			if (!nsh->dev[i].page) {
 				struct page *p = alloc_page(GFP_NOIO);
 				nsh->dev[i].page = p;
@@ -2315,7 +2315,7 @@ static void raid5_end_read_request(struct bio *bi)
 	struct md_rdev *rdev = NULL;
 	sector_t s;
 
-	for (i=0 ; i<disks; i++)
+	for (i = 0 ; i < disks; i++)
 		if (bi == &sh->dev[i].req)
 			break;
 
@@ -2571,7 +2571,7 @@ sector_t raid5_compute_sector(struct r5conf *conf, sector_t r_sector,
 	 * Select the parity disk based on the user selected algorithm.
 	 */
 	pd_idx = qd_idx = -1;
-	switch(conf->level) {
+	switch (conf->level) {
 	case 4:
 		pd_idx = data_disks;
 		break;
@@ -2759,7 +2759,7 @@ sector_t raid5_compute_blocknr(struct stripe_head *sh, int i, int previous)
 
 	if (i == sh->pd_idx)
 		return 0;
-	switch(conf->level) {
+	switch (conf->level) {
 	case 4: break;
 	case 5:
 		switch (algorithm) {
@@ -2957,7 +2957,7 @@ static int add_stripe_bio(struct stripe_head *sh, struct bio *bi, int dd_idx,
 {
 	struct bio **bip;
 	struct r5conf *conf = sh->raid_conf;
-	int firstwrite=0;
+	int firstwrite = 0;
 
 	pr_debug("adding bi b#%llu to stripe s#%llu\n",
 		(unsigned long long)bi->bi_iter.bi_sector,
@@ -3001,7 +3001,7 @@ static int add_stripe_bio(struct stripe_head *sh, struct bio *bi, int dd_idx,
 	if (forwrite) {
 		/* check if page is covered */
 		sector_t sector = sh->dev[dd_idx].sector;
-		for (bi=sh->dev[dd_idx].towrite;
+		for (bi = sh->dev[dd_idx].towrite;
 		     sector < sh->dev[dd_idx].sector + STRIPE_SECTORS &&
 			     bi && bi->bi_iter.bi_sector <= sector;
 		     bi = r5_next_bio(bi, sh->dev[dd_idx].sector)) {
@@ -3639,7 +3639,8 @@ static void handle_stripe_dirtying(struct r5conf *conf,
 	}
 	if ((rcw < rmw || (rcw == rmw && conf->rmw_level != PARITY_PREFER_RMW)) && rcw > 0) {
 		/* want reconstruct write, but need to get some data */
-		int qread =0;
+		int qread = 0;
+
 		rcw = 0;
 		for (i = disks; i--; ) {
 			struct r5dev *dev = &sh->dev[i];
@@ -4031,7 +4032,7 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
 
 	/* Now to look around and see what can be done */
 	rcu_read_lock();
-	for (i=disks; i--; ) {
+	for (i = disks; i--; ) {
 		struct md_rdev *rdev;
 		sector_t first_bad;
 		int bad_sectors;
@@ -4716,7 +4717,7 @@ static int in_chunk_boundary(struct mddev *mddev, struct bio *bio)
  *  add bio to the retry LIFO  ( in O(1) ... we are in interrupt )
  *  later sampled by raid5d.
  */
-static void add_bio_to_retry(struct bio *bi,struct r5conf *conf)
+static void add_bio_to_retry(struct bio *bi, struct r5conf *conf)
 {
 	unsigned long flags;
 
@@ -4739,7 +4740,7 @@ static struct bio *remove_bio_from_retry(struct r5conf *conf)
 		return bi;
 	}
 	bi = conf->retry_read_aligned_list;
-	if(bi) {
+	if (bi) {
 		conf->retry_read_aligned_list = bi->bi_next;
 		bi->bi_next = NULL;
 		/*
@@ -4768,7 +4769,7 @@ static void raid5_align_endio(struct bio *bi)
 
 	bio_put(bi);
 
-	rdev = (void*)raid_bi->bi_next;
+	rdev = (void *)raid_bi->bi_next;
 	raid_bi->bi_next = NULL;
 	mddev = rdev->mddev;
 	conf = mddev->private;
@@ -4838,7 +4839,7 @@ static int raid5_read_one_chunk(struct mddev *mddev, struct bio *raid_bio)
 
 		atomic_inc(&rdev->nr_pending);
 		rcu_read_unlock();
-		raid_bio->bi_next = (void*)rdev;
+		raid_bio->bi_next = (void *)rdev;
 		align_bi->bi_bdev =  rdev->bdev;
 		bio_clear_flag(align_bi, BIO_SEG_VALID);
 
@@ -5198,7 +5199,7 @@ static void raid5_make_request(struct mddev *mddev, struct bio *bi)
 	bi->bi_phys_segments = 1;	/* over-loaded to count active stripes */
 
 	prepare_to_wait(&conf->wait_for_overlap, &w, TASK_UNINTERRUPTIBLE);
-	for (;logical_sector < last_sector; logical_sector += STRIPE_SECTORS) {
+	for (; logical_sector < last_sector; logical_sector += STRIPE_SECTORS) {
 		int previous;
 		int seq;
 
@@ -5464,10 +5465,10 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
 	if ((mddev->reshape_backwards
 	     ? (safepos > writepos && readpos < writepos)
 	     : (safepos < writepos && readpos > writepos)) ||
-	    time_after(jiffies, conf->reshape_checkpoint + 10*HZ)) {
+	    time_after(jiffies, conf->reshape_checkpoint + 10 * HZ)) {
 		/* Cannot proceed until we've updated the superblock... */
 		wait_event(conf->wait_for_overlap,
-			   atomic_read(&conf->reshape_stripes)==0
+			   atomic_read(&conf->reshape_stripes) == 0
 			   || test_bit(MD_RECOVERY_INTR, &mddev->recovery));
 		if (atomic_read(&conf->reshape_stripes) != 0)
 			return 0;
@@ -5497,7 +5498,7 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
 		/* If any of this stripe is beyond the end of the old
 		 * array, then we need to zero those blocks
 		 */
-		for (j=sh->disks; j--;) {
+		for (j = sh->disks; j--;) {
 			sector_t s;
 			if (j == sh->pd_idx)
 				continue;
@@ -6406,7 +6407,7 @@ static unsigned long raid5_cache_scan(struct shrinker *shrink,
 	unsigned long ret = SHRINK_STOP;
 
 	if (mutex_trylock(&conf->cache_size_mutex)) {
-		ret= 0;
+		ret = 0;
 		while (ret < sc->nr_to_scan &&
 		       conf->max_nr_stripes > conf->min_nr_stripes) {
 			if (drop_one_stripe(conf) == 0) {
@@ -7582,7 +7583,7 @@ static void raid5_quiesce(struct mddev *mddev, int state)
 {
 	struct r5conf *conf = mddev->private;
 
-	switch(state) {
+	switch (state) {
 	case 2: /* resume for a suspend */
 		wake_up(&conf->wait_for_overlap);
 		break;
@@ -7850,70 +7851,70 @@ static void *raid6_takeover(struct mddev *mddev)
 }
 
 static struct md_personality raid6_personality = {
-	.name		= "raid6",
-	.level		= 6,
-	.owner		= THIS_MODULE,
-	.make_request	= raid5_make_request,
-	.run		= raid5_run,
-	.free		= raid5_free,
-	.status		= raid5_status,
-	.error_handler	= raid5_error,
-	.hot_add_disk	= raid5_add_disk,
-	.hot_remove_disk= raid5_remove_disk,
-	.spare_active	= raid5_spare_active,
-	.sync_request	= raid5_sync_request,
-	.resize		= raid5_resize,
-	.size		= raid5_size,
-	.check_reshape	= raid6_check_reshape,
-	.start_reshape  = raid5_start_reshape,
-	.finish_reshape = raid5_finish_reshape,
-	.quiesce	= raid5_quiesce,
-	.takeover	= raid6_takeover,
-	.congested	= raid5_congested,
+	.name            = "raid6",
+	.level           = 6,
+	.owner           = THIS_MODULE,
+	.make_request    = raid5_make_request,
+	.run             = raid5_run,
+	.free            = raid5_free,
+	.status          = raid5_status,
+	.error_handler   = raid5_error,
+	.hot_add_disk    = raid5_add_disk,
+	.hot_remove_disk = raid5_remove_disk,
+	.spare_active    = raid5_spare_active,
+	.sync_request    = raid5_sync_request,
+	.resize          = raid5_resize,
+	.size            = raid5_size,
+	.check_reshape   = raid6_check_reshape,
+	.start_reshape   = raid5_start_reshape,
+	.finish_reshape  = raid5_finish_reshape,
+	.quiesce         = raid5_quiesce,
+	.takeover        = raid6_takeover,
+	.congested       = raid5_congested,
 };
 static struct md_personality raid5_personality = {
-	.name		= "raid5",
-	.level		= 5,
-	.owner		= THIS_MODULE,
-	.make_request	= raid5_make_request,
-	.run		= raid5_run,
-	.free		= raid5_free,
-	.status		= raid5_status,
-	.error_handler	= raid5_error,
-	.hot_add_disk	= raid5_add_disk,
-	.hot_remove_disk= raid5_remove_disk,
-	.spare_active	= raid5_spare_active,
-	.sync_request	= raid5_sync_request,
-	.resize		= raid5_resize,
-	.size		= raid5_size,
-	.check_reshape	= raid5_check_reshape,
-	.start_reshape  = raid5_start_reshape,
-	.finish_reshape = raid5_finish_reshape,
-	.quiesce	= raid5_quiesce,
-	.takeover	= raid5_takeover,
-	.congested	= raid5_congested,
+	.name            = "raid5",
+	.level           = 5,
+	.owner           = THIS_MODULE,
+	.make_request    = raid5_make_request,
+	.run             = raid5_run,
+	.free            = raid5_free,
+	.status          = raid5_status,
+	.error_handler   = raid5_error,
+	.hot_add_disk    = raid5_add_disk,
+	.hot_remove_disk = raid5_remove_disk,
+	.spare_active    = raid5_spare_active,
+	.sync_request    = raid5_sync_request,
+	.resize          = raid5_resize,
+	.size            = raid5_size,
+	.check_reshape   = raid5_check_reshape,
+	.start_reshape   = raid5_start_reshape,
+	.finish_reshape  = raid5_finish_reshape,
+	.quiesce         = raid5_quiesce,
+	.takeover        = raid5_takeover,
+	.congested       = raid5_congested,
 };
 static struct md_personality raid4_personality = {
-	.name		= "raid4",
-	.level		= 4,
-	.owner		= THIS_MODULE,
-	.make_request	= raid5_make_request,
-	.run		= raid5_run,
-	.free		= raid5_free,
-	.status		= raid5_status,
-	.error_handler	= raid5_error,
-	.hot_add_disk	= raid5_add_disk,
-	.hot_remove_disk= raid5_remove_disk,
-	.spare_active	= raid5_spare_active,
-	.sync_request	= raid5_sync_request,
-	.resize		= raid5_resize,
-	.size		= raid5_size,
-	.check_reshape	= raid5_check_reshape,
-	.start_reshape  = raid5_start_reshape,
-	.finish_reshape = raid5_finish_reshape,
-	.quiesce	= raid5_quiesce,
-	.takeover	= raid4_takeover,
-	.congested	= raid5_congested,
+	.name            = "raid4",
+	.level           = 4,
+	.owner           = THIS_MODULE,
+	.make_request    = raid5_make_request,
+	.run             = raid5_run,
+	.free            = raid5_free,
+	.status          = raid5_status,
+	.error_handler   = raid5_error,
+	.hot_add_disk    = raid5_add_disk,
+	.hot_remove_disk = raid5_remove_disk,
+	.spare_active    = raid5_spare_active,
+	.sync_request    = raid5_sync_request,
+	.resize          = raid5_resize,
+	.size            = raid5_size,
+	.check_reshape   = raid5_check_reshape,
+	.start_reshape   = raid5_start_reshape,
+	.finish_reshape  = raid5_finish_reshape,
+	.quiesce         = raid5_quiesce,
+	.takeover        = raid4_takeover,
+	.congested       = raid5_congested,
 };
 
 static int __init raid5_init(void)
-- 
2.10.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox