All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] btrfs: Fix -Wunused-but-set-variable warnings
@ 2019-05-31 19:53 Andrey Abramov
  2019-05-31 20:05 ` Joe Perches
  2019-06-03 12:21 ` David Sterba
  0 siblings, 2 replies; 5+ messages in thread
From: Andrey Abramov @ 2019-05-31 19:53 UTC (permalink / raw)
  To: clm, josef, dsterba; +Cc: linux-btrfs, linux-kernel, st5pub

Fix -Wunused-but-set-variable warnings in raid56.c and sysfs.c files

Signed-off-by: Andrey Abramov <st5pub@yandex.ru>
---
 fs/btrfs/raid56.c | 32 +++++++++++---------------------
 fs/btrfs/sysfs.c  |  5 +----
 2 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index f3d0576dd327..4ab29eacfdf3 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -1182,22 +1182,17 @@ static noinline void finish_rmw(struct btrfs_raid_bio *rbio)
 	int nr_data = rbio->nr_data;
 	int stripe;
 	int pagenr;
-	int p_stripe = -1;
-	int q_stripe = -1;
+	int is_q_stripe = 0;
 	struct bio_list bio_list;
 	struct bio *bio;
 	int ret;
 
 	bio_list_init(&bio_list);
 
-	if (rbio->real_stripes - rbio->nr_data == 1) {
-		p_stripe = rbio->real_stripes - 1;
-	} else if (rbio->real_stripes - rbio->nr_data == 2) {
-		p_stripe = rbio->real_stripes - 2;
-		q_stripe = rbio->real_stripes - 1;
-	} else {
+	if (rbio->real_stripes - rbio->nr_data == 2)
+		is_q_stripe = 1;
+	else if (rbio->real_stripes - rbio->nr_data != 1)
 		BUG();
-	}
 
 	/* at this point we either have a full stripe,
 	 * or we've read the full stripe from the drive.
@@ -1241,7 +1236,7 @@ static noinline void finish_rmw(struct btrfs_raid_bio *rbio)
 		SetPageUptodate(p);
 		pointers[stripe++] = kmap(p);
 
-		if (q_stripe != -1) {
+		if (is_q_stripe) {
 
 			/*
 			 * raid6, add the qstripe and call the
@@ -2340,8 +2335,7 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
 	int nr_data = rbio->nr_data;
 	int stripe;
 	int pagenr;
-	int p_stripe = -1;
-	int q_stripe = -1;
+	int is_q_stripe = 0;
 	struct page *p_page = NULL;
 	struct page *q_page = NULL;
 	struct bio_list bio_list;
@@ -2351,14 +2345,10 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
 
 	bio_list_init(&bio_list);
 
-	if (rbio->real_stripes - rbio->nr_data == 1) {
-		p_stripe = rbio->real_stripes - 1;
-	} else if (rbio->real_stripes - rbio->nr_data == 2) {
-		p_stripe = rbio->real_stripes - 2;
-		q_stripe = rbio->real_stripes - 1;
-	} else {
+	if (rbio->real_stripes - rbio->nr_data == 2)
+		is_q_stripe = 1;
+	else if (rbio->real_stripes - rbio->nr_data != 1)
 		BUG();
-	}
 
 	if (bbio->num_tgtdevs && bbio->tgtdev_map[rbio->scrubp]) {
 		is_replace = 1;
@@ -2380,7 +2370,7 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
 		goto cleanup;
 	SetPageUptodate(p_page);
 
-	if (q_stripe != -1) {
+	if (is_q_stripe) {
 		q_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
 		if (!q_page) {
 			__free_page(p_page);
@@ -2403,7 +2393,7 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
 		/* then add the parity stripe */
 		pointers[stripe++] = kmap(p_page);
 
-		if (q_stripe != -1) {
+		if (is_q_stripe) {
 
 			/*
 			 * raid6, add the qstripe and call the
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 2f078b77fe14..514b75dec4a9 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -887,13 +887,10 @@ void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
 {
 	struct btrfs_fs_devices *fs_devs;
 	struct kobject *fsid_kobj;
-	u64 features;
-	int ret;
 
 	if (!fs_info)
 		return;
 
-	features = get_features(fs_info, set);
 	ASSERT(bit & supported_feature_masks[set]);
 
 	fs_devs = fs_info->fs_devices;
@@ -907,7 +904,7 @@ void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
 	 * to use sysfs_update_group but some refactoring is needed first.
 	 */
 	sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
-	ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
+	sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
 }
 
 static int btrfs_init_debugfs(void)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs: Fix -Wunused-but-set-variable warnings
  2019-05-31 19:53 [PATCH] btrfs: Fix -Wunused-but-set-variable warnings Andrey Abramov
@ 2019-05-31 20:05 ` Joe Perches
  2019-05-31 20:31   ` Andrey Abramov
  2019-06-03 12:21 ` David Sterba
  1 sibling, 1 reply; 5+ messages in thread
From: Joe Perches @ 2019-05-31 20:05 UTC (permalink / raw)
  To: Andrey Abramov, clm, josef, dsterba; +Cc: linux-btrfs, linux-kernel

On Fri, 2019-05-31 at 22:53 +0300, Andrey Abramov wrote:
> Fix -Wunused-but-set-variable warnings in raid56.c and sysfs.c files

trivia:

> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index f3d0576dd327..4ab29eacfdf3 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -1182,22 +1182,17 @@ static noinline void finish_rmw(struct btrfs_raid_bio *rbio)
>  	int nr_data = rbio->nr_data;
>  	int stripe;
>  	int pagenr;
> -	int p_stripe = -1;
> -	int q_stripe = -1;
> +	int is_q_stripe = 0;

These uses seem boolean, so perhaps just use bool?

> +	if (rbio->real_stripes - rbio->nr_data == 2)
> +		is_q_stripe = 1;
[]
> @@ -1241,7 +1236,7 @@ static noinline void finish_rmw(struct btrfs_raid_bio *rbio)
[]
> +		if (is_q_stripe) {
> @@ -2340,8 +2335,7 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
[]
> +	int is_q_stripe = 0;
> @@ -2351,14 +2345,10 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
> +	if (rbio->real_stripes - rbio->nr_data == 2)
> +		is_q_stripe = 1;
[]
> @@ -2380,7 +2370,7 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
[]
> +	if (is_q_stripe) {



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs: Fix -Wunused-but-set-variable warnings
  2019-05-31 20:05 ` Joe Perches
@ 2019-05-31 20:31   ` Andrey Abramov
  2019-05-31 20:38     ` Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Andrey Abramov @ 2019-05-31 20:31 UTC (permalink / raw)
  To: Joe Perches, clm, josef, dsterba; +Cc: linux-btrfs, linux-kernel



31.05.2019, 23:05, "Joe Perches" <joe@perches.com>:
> On Fri, 2019-05-31 at 22:53 +0300, Andrey Abramov wrote:
>>  Fix -Wunused-but-set-variable warnings in raid56.c and sysfs.c files
> These uses seem boolean, so perhaps just use bool?
I used int because you use ints (as bools) everywhere (for example there is only one bool (as a function argument) in the entire raid56.c file with 3000 lines of code), so with int code looks more consistent.
Are you sure that I should use bool?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs: Fix -Wunused-but-set-variable warnings
  2019-05-31 20:31   ` Andrey Abramov
@ 2019-05-31 20:38     ` Joe Perches
  0 siblings, 0 replies; 5+ messages in thread
From: Joe Perches @ 2019-05-31 20:38 UTC (permalink / raw)
  To: Andrey Abramov, clm, josef, dsterba; +Cc: linux-btrfs, linux-kernel

On Fri, 2019-05-31 at 23:31 +0300, Andrey Abramov wrote:
> 31.05.2019, 23:05, "Joe Perches" <joe@perches.com>:
> > On Fri, 2019-05-31 at 22:53 +0300, Andrey Abramov wrote:
> > >  Fix -Wunused-but-set-variable warnings in raid56.c and sysfs.c files
> > These uses seem boolean, so perhaps just use bool?
> I used int because you use ints (as bools) everywhere (for example
> there is only one bool (as a function argument) in the entire raid56.c
> file with 3000 lines of code), so with int code looks more consistent.
> Are you sure that I should use bool?

That's up to you and the btrfs maintainers.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs: Fix -Wunused-but-set-variable warnings
  2019-05-31 19:53 [PATCH] btrfs: Fix -Wunused-but-set-variable warnings Andrey Abramov
  2019-05-31 20:05 ` Joe Perches
@ 2019-06-03 12:21 ` David Sterba
  1 sibling, 0 replies; 5+ messages in thread
From: David Sterba @ 2019-06-03 12:21 UTC (permalink / raw)
  To: Andrey Abramov; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel

On Fri, May 31, 2019 at 10:53:49PM +0300, Andrey Abramov wrote:
> Fix -Wunused-but-set-variable warnings in raid56.c and sysfs.c files

Please ignore the warnings for now. The RAID56 needs more cleanups than
that an the sysfs part needs to be reworked. The stale code comes from
e410e34fad913dd568ec28d2a9949694324c14db that reverted
14e46e04958df740c6c6a94849f176159a333f13.

> Signed-off-by: Andrey Abramov <st5pub@yandex.ru>
> ---
>  fs/btrfs/raid56.c | 32 +++++++++++---------------------
>  fs/btrfs/sysfs.c  |  5 +----
>  2 files changed, 12 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index f3d0576dd327..4ab29eacfdf3 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -1182,22 +1182,17 @@ static noinline void finish_rmw(struct btrfs_raid_bio *rbio)
>  	int nr_data = rbio->nr_data;
>  	int stripe;
>  	int pagenr;
> -	int p_stripe = -1;
> -	int q_stripe = -1;
> +	int is_q_stripe = 0;
>  	struct bio_list bio_list;
>  	struct bio *bio;
>  	int ret;
>  
>  	bio_list_init(&bio_list);
>  
> -	if (rbio->real_stripes - rbio->nr_data == 1) {
> -		p_stripe = rbio->real_stripes - 1;
> -	} else if (rbio->real_stripes - rbio->nr_data == 2) {
> -		p_stripe = rbio->real_stripes - 2;
> -		q_stripe = rbio->real_stripes - 1;
> -	} else {
> +	if (rbio->real_stripes - rbio->nr_data == 2)
> +		is_q_stripe = 1;
> +	else if (rbio->real_stripes - rbio->nr_data != 1)
>  		BUG();
> -	}

The original code is better structured, enumerates the expected cases
and leaves a catch-all branch.

>  
>  	/* at this point we either have a full stripe,
>  	 * or we've read the full stripe from the drive.
> @@ -1241,7 +1236,7 @@ static noinline void finish_rmw(struct btrfs_raid_bio *rbio)
>  		SetPageUptodate(p);
>  		pointers[stripe++] = kmap(p);
>  
> -		if (q_stripe != -1) {
> +		if (is_q_stripe) {
>  
>  			/*
>  			 * raid6, add the qstripe and call the
> @@ -2340,8 +2335,7 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
>  	int nr_data = rbio->nr_data;
>  	int stripe;
>  	int pagenr;
> -	int p_stripe = -1;

To get rid of the warning, perhaps just this initialization could be
removed, the rest of the code untouched.

> -	int q_stripe = -1;
> +	int is_q_stripe = 0;
>  	struct page *p_page = NULL;
>  	struct page *q_page = NULL;
>  	struct bio_list bio_list;

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-06-03 12:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-31 19:53 [PATCH] btrfs: Fix -Wunused-but-set-variable warnings Andrey Abramov
2019-05-31 20:05 ` Joe Perches
2019-05-31 20:31   ` Andrey Abramov
2019-05-31 20:38     ` Joe Perches
2019-06-03 12:21 ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.