| Message ID | 20190531195349.31129-1-st5pub@yandex.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Fix -Wunused-but-set-variable warnings | expand |
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) {
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?
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.
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;
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)
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(-)