All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] define BTRFS_DEV_STATE
@ 2017-12-04  4:54 Anand Jain
  2017-12-04  4:54 ` [PATCH v2 1/5] btrfs: cleanup device states define BTRFS_DEV_STATE_WRITEABLE Anand Jain
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Anand Jain @ 2017-12-04  4:54 UTC (permalink / raw)
  To: linux-btrfs

As of now device properties and states are being represented as int
variable. So clean that up using bitwise operations. Also patches in
the ML such as device failed state needs this cleanup as well.

V2:
 Accepts all comments from Nikolay.
 Drops can_discard.
 Adds BTRFS_DEV_STATE_REPLACE_TGT and BTRFS_DEV_STATE_FLUSH_SENT
    patches.

Anand Jain (5):
  btrfs: cleanup device states define BTRFS_DEV_STATE_WRITEABLE
  btrfs: cleanup device states define BTRFS_DEV_STATE_IN_FS_METADATA
  btrfs: cleanup device states define BTRFS_DEV_STATE_MISSING
  btrfs: cleanup device states define BTRFS_DEV_STATE_REPLACE_TGT
  btrfs: cleanup device states define BTRFS_DEV_STATE_FLUSH_SENT

 fs/btrfs/dev-replace.c |   8 ++-
 fs/btrfs/disk-io.c     |  29 ++++++---
 fs/btrfs/extent-tree.c |   5 +-
 fs/btrfs/extent_io.c   |   3 +-
 fs/btrfs/ioctl.c       |   4 +-
 fs/btrfs/scrub.c       |  13 +++--
 fs/btrfs/super.c       |   8 ++-
 fs/btrfs/volumes.c     | 156 ++++++++++++++++++++++++++++---------------------
 fs/btrfs/volumes.h     |  11 ++--
 9 files changed, 143 insertions(+), 94 deletions(-)

-- 
2.15.0


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

* [PATCH v2 1/5] btrfs: cleanup device states define BTRFS_DEV_STATE_WRITEABLE
  2017-12-04  4:54 [PATCH v2 0/5] define BTRFS_DEV_STATE Anand Jain
@ 2017-12-04  4:54 ` Anand Jain
  2017-12-04  4:54 ` [PATCH v2 2/5] btrfs: cleanup device states define BTRFS_DEV_STATE_IN_FS_METADATA Anand Jain
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2017-12-04  4:54 UTC (permalink / raw)
  To: linux-btrfs

Currently device state is being managed by each individual int
variable such as struct btrfs_device::writeable. Instead of that
declare device state BTRFS_DEV_STATE_WRITEABLE and use the
bit operations.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: Remove a unrelated change.
    Start btrfs_device::dev_state position from bit 0.
 fs/btrfs/disk-io.c     | 12 ++++++----
 fs/btrfs/extent-tree.c |  2 +-
 fs/btrfs/extent_io.c   |  3 ++-
 fs/btrfs/ioctl.c       |  2 +-
 fs/btrfs/scrub.c       |  3 ++-
 fs/btrfs/volumes.c     | 60 +++++++++++++++++++++++++++++---------------------
 fs/btrfs/volumes.h     |  4 +++-
 7 files changed, 52 insertions(+), 34 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 10a2a579cc7f..56198cb02b35 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3403,7 +3403,8 @@ static int barrier_all_devices(struct btrfs_fs_info *info)
 			continue;
 		if (!dev->bdev)
 			continue;
-		if (!dev->in_fs_metadata || !dev->writeable)
+		if (!dev->in_fs_metadata ||
+			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
 			continue;
 
 		write_dev_flush(dev);
@@ -3418,7 +3419,8 @@ static int barrier_all_devices(struct btrfs_fs_info *info)
 			errors_wait++;
 			continue;
 		}
-		if (!dev->in_fs_metadata || !dev->writeable)
+		if (!dev->in_fs_metadata ||
+			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
 			continue;
 
 		ret = wait_dev_flush(dev);
@@ -3515,7 +3517,8 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors)
 			total_errors++;
 			continue;
 		}
-		if (!dev->in_fs_metadata || !dev->writeable)
+		if (!dev->in_fs_metadata ||
+			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
 			continue;
 
 		btrfs_set_stack_device_generation(dev_item, 0);
@@ -3554,7 +3557,8 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors)
 	list_for_each_entry_rcu(dev, head, dev_list) {
 		if (!dev->bdev)
 			continue;
-		if (!dev->in_fs_metadata || !dev->writeable)
+		if (!dev->in_fs_metadata ||
+			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
 			continue;
 
 		ret = wait_dev_supers(dev, max_mirrors);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 15c01014e5e1..2cd323d184a0 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -10877,7 +10877,7 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 	*trimmed = 0;
 
 	/* Not writeable = nothing to do. */
-	if (!device->writeable)
+	if (!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state))
 		return 0;
 
 	/* No free space = nothing to do. */
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 012d63870b99..25682c5a0dd5 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2027,7 +2027,8 @@ int repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start,
 	bio->bi_iter.bi_sector = sector;
 	dev = bbio->stripes[bbio->mirror_num - 1].dev;
 	btrfs_put_bbio(bbio);
-	if (!dev || !dev->bdev || !dev->writeable) {
+	if (!dev || !dev->bdev ||
+		!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state)) {
 		btrfs_bio_counter_dec(fs_info);
 		bio_put(bio);
 		return -EIO;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index d748ad1c3620..e59004a17166 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1503,7 +1503,7 @@ static noinline int btrfs_ioctl_resize(struct file *file,
 		goto out_free;
 	}
 
-	if (!device->writeable) {
+	if (!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state)) {
 		btrfs_info(fs_info,
 			   "resizer unable to apply on readonly device %llu",
 		       devid);
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index b2f871d80982..fa70ff9b7762 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -4117,7 +4117,8 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start,
 		return -ENODEV;
 	}
 
-	if (!is_dev_replace && !readonly && !dev->writeable) {
+	if (!is_dev_replace && !readonly &&
+		!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state)) {
 		mutex_unlock(&fs_info->fs_devices->device_list_mutex);
 		rcu_read_lock();
 		name = rcu_dereference(dev->name);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index c19a49167966..9d14d83ab8dc 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -620,10 +620,15 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
 	device->generation = btrfs_super_generation(disk_super);
 
 	if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
-		device->writeable = 0;
+		clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
 		fs_devices->seeding = 1;
 	} else {
-		device->writeable = !bdev_read_only(bdev);
+		if (bdev_read_only(bdev))
+			clear_bit(BTRFS_DEV_STATE_WRITEABLE,
+						&device->dev_state);
+		else
+			set_bit(BTRFS_DEV_STATE_WRITEABLE,
+						&device->dev_state);
 	}
 
 	q = bdev_get_queue(bdev);
@@ -635,7 +640,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
 	device->mode = flags;
 
 	fs_devices->open_devices++;
-	if (device->writeable &&
+	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
 	    device->devid != BTRFS_DEV_REPLACE_DEVID) {
 		fs_devices->rw_devices++;
 		list_add(&device->dev_alloc_list,
@@ -867,9 +872,10 @@ void btrfs_close_extra_devices(struct btrfs_fs_devices *fs_devices, int step)
 			device->bdev = NULL;
 			fs_devices->open_devices--;
 		}
-		if (device->writeable) {
+		if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state)) {
 			list_del_init(&device->dev_alloc_list);
-			device->writeable = 0;
+			clear_bit(BTRFS_DEV_STATE_WRITEABLE,
+							&device->dev_state);
 			if (!device->is_tgtdev_for_dev_replace)
 				fs_devices->rw_devices--;
 		}
@@ -912,7 +918,8 @@ static void free_device(struct rcu_head *head)
 
 static void btrfs_close_bdev(struct btrfs_device *device)
 {
-	if (device->bdev && device->writeable) {
+	if (device->bdev &&
+		test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state)) {
 		sync_blockdev(device->bdev);
 		invalidate_bdev(device->bdev);
 	}
@@ -930,7 +937,7 @@ static void btrfs_prepare_close_one_device(struct btrfs_device *device)
 	if (device->bdev)
 		fs_devices->open_devices--;
 
-	if (device->writeable &&
+	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
 	    device->devid != BTRFS_DEV_REPLACE_DEVID) {
 		list_del_init(&device->dev_alloc_list);
 		fs_devices->rw_devices--;
@@ -1893,12 +1900,13 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
 		goto out;
 	}
 
-	if (device->writeable && fs_info->fs_devices->rw_devices == 1) {
+	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
+			fs_info->fs_devices->rw_devices == 1) {
 		ret = BTRFS_ERROR_DEV_ONLY_WRITABLE;
 		goto out;
 	}
 
-	if (device->writeable) {
+	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state)) {
 		mutex_lock(&fs_info->chunk_mutex);
 		list_del_init(&device->dev_alloc_list);
 		device->fs_devices->rw_devices--;
@@ -1960,7 +1968,7 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
 	 * the devices list.  All that's left is to zero out the old
 	 * supers and free the device.
 	 */
-	if (device->writeable)
+	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state))
 		btrfs_scratch_superblocks(device->bdev, device->name->str);
 
 	btrfs_close_bdev(device);
@@ -1986,7 +1994,7 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
 	return ret;
 
 error_undo:
-	if (device->writeable) {
+	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state)) {
 		mutex_lock(&fs_info->chunk_mutex);
 		list_add(&device->dev_alloc_list,
 			 &fs_info->fs_devices->alloc_list);
@@ -2017,7 +2025,7 @@ void btrfs_rm_dev_replace_remove_srcdev(struct btrfs_fs_info *fs_info,
 	if (srcdev->missing)
 		fs_devices->missing_devices--;
 
-	if (srcdev->writeable)
+	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &srcdev->dev_state))
 		fs_devices->rw_devices--;
 
 	if (srcdev->bdev)
@@ -2029,7 +2037,7 @@ void btrfs_rm_dev_replace_free_srcdev(struct btrfs_fs_info *fs_info,
 {
 	struct btrfs_fs_devices *fs_devices = srcdev->fs_devices;
 
-	if (srcdev->writeable) {
+	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &srcdev->dev_state)) {
 		/* zero out the old super if it is writable */
 		btrfs_scratch_superblocks(srcdev->bdev, srcdev->name->str);
 	}
@@ -2385,7 +2393,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	}
 
 	q = bdev_get_queue(bdev);
-	device->writeable = 1;
+	set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
 	device->generation = trans->transid;
 	device->io_width = fs_info->sectorsize;
 	device->io_align = fs_info->sectorsize;
@@ -2594,7 +2602,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
 	rcu_assign_pointer(device->name, name);
 
 	mutex_lock(&fs_info->fs_devices->device_list_mutex);
-	device->writeable = 1;
+	set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
 	device->generation = 0;
 	device->io_width = fs_info->sectorsize;
 	device->io_align = fs_info->sectorsize;
@@ -2694,7 +2702,7 @@ int btrfs_grow_device(struct btrfs_trans_handle *trans,
 	u64 old_total;
 	u64 diff;
 
-	if (!device->writeable)
+	if (!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state))
 		return -EACCES;
 
 	new_size = round_down(new_size, fs_info->sectorsize);
@@ -3514,7 +3522,7 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
 		old_size = btrfs_device_get_total_bytes(device);
 		size_to_free = div_factor(old_size, 1);
 		size_to_free = min_t(u64, size_to_free, SZ_1M);
-		if (!device->writeable ||
+		if (!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) ||
 		    btrfs_device_get_total_bytes(device) -
 		    btrfs_device_get_bytes_used(device) > size_to_free ||
 		    device->is_tgtdev_for_dev_replace)
@@ -4397,7 +4405,7 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size)
 	mutex_lock(&fs_info->chunk_mutex);
 
 	btrfs_device_set_total_bytes(device, new_size);
-	if (device->writeable) {
+	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state)) {
 		device->fs_devices->total_rw_bytes -= diff;
 		atomic64_sub(diff, &fs_info->free_chunk_space);
 	}
@@ -4522,7 +4530,7 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size)
 	if (ret) {
 		mutex_lock(&fs_info->chunk_mutex);
 		btrfs_device_set_total_bytes(device, old_size);
-		if (device->writeable)
+		if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state))
 			device->fs_devices->total_rw_bytes += diff;
 		atomic64_add(diff, &fs_info->free_chunk_space);
 		mutex_unlock(&fs_info->chunk_mutex);
@@ -4682,7 +4690,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		u64 max_avail;
 		u64 dev_offset;
 
-		if (!device->writeable) {
+		if (!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state)) {
 			WARN(1, KERN_ERR
 			       "BTRFS: read-only device in alloc_list\n");
 			continue;
@@ -5041,8 +5049,8 @@ int btrfs_chunk_readonly(struct btrfs_fs_info *fs_info, u64 chunk_offset)
 			miss_ndevs++;
 			continue;
 		}
-
-		if (!map->stripes[i].dev->writeable) {
+		if (!test_bit(BTRFS_DEV_STATE_WRITEABLE,
+					&map->stripes[i].dev->dev_state)) {
 			readonly = 1;
 			goto end;
 		}
@@ -6211,7 +6219,8 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
 	for (dev_nr = 0; dev_nr < total_devs; dev_nr++) {
 		dev = bbio->stripes[dev_nr].dev;
 		if (!dev || !dev->bdev ||
-		    (bio_op(first_bio) == REQ_OP_WRITE && !dev->writeable)) {
+		    (bio_op(first_bio) == REQ_OP_WRITE &&
+		    !test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))) {
 			bbio_error(bbio, first_bio, logical);
 			continue;
 		}
@@ -6648,7 +6657,7 @@ static int read_one_dev(struct btrfs_fs_info *fs_info,
 	}
 
 	if (device->fs_devices != fs_info->fs_devices) {
-		BUG_ON(device->writeable);
+		BUG_ON(test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state));
 		if (device->generation !=
 		    btrfs_device_generation(leaf, dev_item))
 			return -EINVAL;
@@ -6656,7 +6665,8 @@ static int read_one_dev(struct btrfs_fs_info *fs_info,
 
 	fill_device_from_item(leaf, dev_item, device);
 	device->in_fs_metadata = 1;
-	if (device->writeable && !device->is_tgtdev_for_dev_replace) {
+	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
+					!device->is_tgtdev_for_dev_replace) {
 		device->fs_devices->total_rw_bytes += device->total_bytes;
 		atomic64_add(device->total_bytes - device->bytes_used,
 				&fs_info->free_chunk_space);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index bbb5db6be7f8..b6d1b1f90b73 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -47,6 +47,8 @@ struct btrfs_pending_bios {
 #define btrfs_device_data_ordered_init(device) do { } while (0)
 #endif
 
+#define BTRFS_DEV_STATE_WRITEABLE	(1UL << 0)
+
 struct btrfs_device {
 	struct list_head dev_list;
 	struct list_head dev_alloc_list;
@@ -69,7 +71,7 @@ struct btrfs_device {
 	/* the mode sent to blkdev_get */
 	fmode_t mode;
 
-	int writeable;
+	unsigned long dev_state;
 	int in_fs_metadata;
 	int missing;
 	int is_tgtdev_for_dev_replace;
-- 
2.15.0


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

* [PATCH v2 2/5] btrfs: cleanup device states define BTRFS_DEV_STATE_IN_FS_METADATA
  2017-12-04  4:54 [PATCH v2 0/5] define BTRFS_DEV_STATE Anand Jain
  2017-12-04  4:54 ` [PATCH v2 1/5] btrfs: cleanup device states define BTRFS_DEV_STATE_WRITEABLE Anand Jain
@ 2017-12-04  4:54 ` Anand Jain
  2017-12-04  4:54 ` [PATCH v2 3/5] btrfs: cleanup device states define BTRFS_DEV_STATE_MISSING Anand Jain
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2017-12-04  4:54 UTC (permalink / raw)
  To: linux-btrfs

Currently device state is being managed by each individual int
variable such as struct btrfs_device::in_fs_metadata. Instead of
that declare device state BTRFS_DEV_STATE_IN_FS_METADATA and use
the bit operations.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/disk-io.c | 21 ++++++++++++++-------
 fs/btrfs/scrub.c   |  3 ++-
 fs/btrfs/super.c   |  5 +++--
 fs/btrfs/volumes.c | 29 +++++++++++++++++------------
 fs/btrfs/volumes.h |  2 +-
 5 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 56198cb02b35..634e8eb51cc8 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3403,8 +3403,10 @@ static int barrier_all_devices(struct btrfs_fs_info *info)
 			continue;
 		if (!dev->bdev)
 			continue;
-		if (!dev->in_fs_metadata ||
-			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
+		if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
+						&dev->dev_state) ||
+			!test_bit(BTRFS_DEV_STATE_WRITEABLE,
+						&dev->dev_state))
 			continue;
 
 		write_dev_flush(dev);
@@ -3419,8 +3421,10 @@ static int barrier_all_devices(struct btrfs_fs_info *info)
 			errors_wait++;
 			continue;
 		}
-		if (!dev->in_fs_metadata ||
-			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
+		if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
+						&dev->dev_state) ||
+			!test_bit(BTRFS_DEV_STATE_WRITEABLE,
+						&dev->dev_state))
 			continue;
 
 		ret = wait_dev_flush(dev);
@@ -3517,7 +3521,8 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors)
 			total_errors++;
 			continue;
 		}
-		if (!dev->in_fs_metadata ||
+		if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
+						&dev->dev_state) ||
 			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
 			continue;
 
@@ -3557,8 +3562,10 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors)
 	list_for_each_entry_rcu(dev, head, dev_list) {
 		if (!dev->bdev)
 			continue;
-		if (!dev->in_fs_metadata ||
-			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
+		if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
+						&dev->dev_state) ||
+			!test_bit(BTRFS_DEV_STATE_WRITEABLE,
+						&dev->dev_state))
 			continue;
 
 		ret = wait_dev_supers(dev, max_mirrors);
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index fa70ff9b7762..c4705de2ec26 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -4129,7 +4129,8 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start,
 	}
 
 	mutex_lock(&fs_info->scrub_lock);
-	if (!dev->in_fs_metadata || dev->is_tgtdev_for_dev_replace) {
+	if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &dev->dev_state) ||
+					dev->is_tgtdev_for_dev_replace) {
 		mutex_unlock(&fs_info->scrub_lock);
 		mutex_unlock(&fs_info->fs_devices->device_list_mutex);
 		return -EIO;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 3a4dce153645..f0906fbfa731 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1972,8 +1972,9 @@ static int btrfs_calc_avail_data_space(struct btrfs_fs_info *fs_info,
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(device, &fs_devices->devices, dev_list) {
-		if (!device->in_fs_metadata || !device->bdev ||
-		    device->is_tgtdev_for_dev_replace)
+		if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
+						&device->dev_state) ||
+			!device->bdev || device->is_tgtdev_for_dev_replace)
 			continue;
 
 		if (i >= nr_devices)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 9d14d83ab8dc..7100c877748d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -636,7 +636,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
 		fs_devices->rotating = 1;
 
 	device->bdev = bdev;
-	device->in_fs_metadata = 0;
+	clear_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
 	device->mode = flags;
 
 	fs_devices->open_devices++;
@@ -843,7 +843,8 @@ void btrfs_close_extra_devices(struct btrfs_fs_devices *fs_devices, int step)
 again:
 	/* This is the initialized path, it is safe to release the devices. */
 	list_for_each_entry_safe(device, next, &fs_devices->devices, dev_list) {
-		if (device->in_fs_metadata) {
+		if (test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
+							&device->dev_state)) {
 			if (!device->is_tgtdev_for_dev_replace &&
 			    (!latest_dev ||
 			     device->generation > latest_dev->generation)) {
@@ -1588,7 +1589,7 @@ static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
 	struct extent_buffer *leaf;
 	struct btrfs_key key;
 
-	WARN_ON(!device->in_fs_metadata);
+	WARN_ON(!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state));
 	WARN_ON(device->is_tgtdev_for_dev_replace);
 	path = btrfs_alloc_path();
 	if (!path)
@@ -1928,7 +1929,7 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
 	if (ret)
 		goto error_undo;
 
-	device->in_fs_metadata = 0;
+	clear_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
 	btrfs_scrub_cancel_dev(fs_info, device);
 
 	/*
@@ -2147,7 +2148,8 @@ int btrfs_find_device_missing_or_by_path(struct btrfs_fs_info *fs_info,
 		 * is held by the caller.
 		 */
 		list_for_each_entry(tmp, devices, dev_list) {
-			if (tmp->in_fs_metadata && !tmp->bdev) {
+			if (test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
+					&tmp->dev_state) && !tmp->bdev) {
 				*device = tmp;
 				break;
 			}
@@ -2404,7 +2406,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	device->commit_total_bytes = device->total_bytes;
 	device->fs_info = fs_info;
 	device->bdev = bdev;
-	device->in_fs_metadata = 1;
+	set_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
 	device->is_tgtdev_for_dev_replace = 0;
 	device->mode = FMODE_EXCL;
 	device->dev_stats_valid = 1;
@@ -2615,7 +2617,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
 	device->commit_bytes_used = device->bytes_used;
 	device->fs_info = fs_info;
 	device->bdev = bdev;
-	device->in_fs_metadata = 1;
+	set_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
 	device->is_tgtdev_for_dev_replace = 1;
 	device->mode = FMODE_EXCL;
 	device->dev_stats_valid = 1;
@@ -2644,7 +2646,7 @@ void btrfs_init_dev_replace_tgtdev_for_resume(struct btrfs_fs_info *fs_info,
 	tgtdev->io_align = sectorsize;
 	tgtdev->sector_size = sectorsize;
 	tgtdev->fs_info = fs_info;
-	tgtdev->in_fs_metadata = 1;
+	set_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &tgtdev->dev_state);
 }
 
 static noinline int btrfs_update_device(struct btrfs_trans_handle *trans,
@@ -4696,8 +4698,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 			continue;
 		}
 
-		if (!device->in_fs_metadata ||
-		    device->is_tgtdev_for_dev_replace)
+		if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
+					&device->dev_state) ||
+					device->is_tgtdev_for_dev_replace)
 			continue;
 
 		if (device->total_bytes > device->bytes_used)
@@ -6489,7 +6492,9 @@ static int read_one_chunk(struct btrfs_fs_info *fs_info, struct btrfs_key *key,
 			}
 			btrfs_report_missing_device(fs_info, devid, uuid, false);
 		}
-		map->stripes[i].dev->in_fs_metadata = 1;
+		set_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
+				&(map->stripes[i].dev->dev_state));
+
 	}
 
 	write_lock(&map_tree->map_tree.lock);
@@ -6664,7 +6669,7 @@ static int read_one_dev(struct btrfs_fs_info *fs_info,
 	}
 
 	fill_device_from_item(leaf, dev_item, device);
-	device->in_fs_metadata = 1;
+	set_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
 	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
 					!device->is_tgtdev_for_dev_replace) {
 		device->fs_devices->total_rw_bytes += device->total_bytes;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index b6d1b1f90b73..4c369ec9efac 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -48,6 +48,7 @@ struct btrfs_pending_bios {
 #endif
 
 #define BTRFS_DEV_STATE_WRITEABLE	(1UL << 0)
+#define BTRFS_DEV_STATE_IN_FS_METADATA	(1UL << 1)
 
 struct btrfs_device {
 	struct list_head dev_list;
@@ -72,7 +73,6 @@ struct btrfs_device {
 	fmode_t mode;
 
 	unsigned long dev_state;
-	int in_fs_metadata;
 	int missing;
 	int is_tgtdev_for_dev_replace;
 	blk_status_t last_flush_error;
-- 
2.15.0


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

* [PATCH v2 3/5] btrfs: cleanup device states define BTRFS_DEV_STATE_MISSING
  2017-12-04  4:54 [PATCH v2 0/5] define BTRFS_DEV_STATE Anand Jain
  2017-12-04  4:54 ` [PATCH v2 1/5] btrfs: cleanup device states define BTRFS_DEV_STATE_WRITEABLE Anand Jain
  2017-12-04  4:54 ` [PATCH v2 2/5] btrfs: cleanup device states define BTRFS_DEV_STATE_IN_FS_METADATA Anand Jain
@ 2017-12-04  4:54 ` Anand Jain
  2017-12-04  4:54 ` [PATCH v2 4/5] btrfs: cleanup device states define BTRFS_DEV_STATE_REPLACE_TGT Anand Jain
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2017-12-04  4:54 UTC (permalink / raw)
  To: linux-btrfs

Currently device state is being managed by each individual int
variable such as struct btrfs_device::missing. Instead of that
declare btrfs_device::dev_state BTRFS_DEV_STATE_MISSING and use
the bit operations.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by : Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/dev-replace.c |  3 ++-
 fs/btrfs/disk-io.c     |  4 ++--
 fs/btrfs/scrub.c       |  7 ++++---
 fs/btrfs/super.c       |  2 +-
 fs/btrfs/volumes.c     | 34 ++++++++++++++++++++--------------
 fs/btrfs/volumes.h     |  2 +-
 6 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 4b6ceb38cb5f..559db7667f38 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -306,7 +306,8 @@ void btrfs_after_dev_replace_commit(struct btrfs_fs_info *fs_info)
 
 static inline char *dev_missing_or_rcu_str(struct btrfs_device *device)
 {
-	return device->missing ? "<missing disk>" : rcu_str_deref(device->name);
+	return test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state) ?
+				"<missing disk>" : rcu_str_deref(device->name);
 }
 
 int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info,
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 634e8eb51cc8..890e3a6a2f3e 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3399,7 +3399,7 @@ static int barrier_all_devices(struct btrfs_fs_info *info)
 	/* send down all the barriers */
 	head = &info->fs_devices->devices;
 	list_for_each_entry_rcu(dev, head, dev_list) {
-		if (dev->missing)
+		if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state))
 			continue;
 		if (!dev->bdev)
 			continue;
@@ -3415,7 +3415,7 @@ static int barrier_all_devices(struct btrfs_fs_info *info)
 
 	/* wait for all the barriers */
 	list_for_each_entry_rcu(dev, head, dev_list) {
-		if (dev->missing)
+		if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state))
 			continue;
 		if (!dev->bdev) {
 			errors_wait++;
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index c4705de2ec26..b6de017066b3 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -2535,7 +2535,7 @@ static int scrub_pages(struct scrub_ctx *sctx, u64 logical, u64 len,
 	}
 
 	WARN_ON(sblock->page_count == 0);
-	if (dev->missing) {
+	if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state)) {
 		/*
 		 * This case should only be hit for RAID 5/6 device replace. See
 		 * the comment in scrub_missing_raid56_pages() for details.
@@ -2870,7 +2870,7 @@ static int scrub_extent_for_parity(struct scrub_parity *sparity,
 	u8 csum[BTRFS_CSUM_SIZE];
 	u32 blocksize;
 
-	if (dev->missing) {
+	if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state)) {
 		scrub_parity_mark_sectors_error(sparity, logical, len);
 		return 0;
 	}
@@ -4112,7 +4112,8 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start,
 
 	mutex_lock(&fs_info->fs_devices->device_list_mutex);
 	dev = btrfs_find_device(fs_info, devid, NULL, NULL);
-	if (!dev || (dev->missing && !is_dev_replace)) {
+	if (!dev || (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state) &&
+						!is_dev_replace)) {
 		mutex_unlock(&fs_info->fs_devices->device_list_mutex);
 		return -ENODEV;
 	}
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index f0906fbfa731..6bae2e046257 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2270,7 +2270,7 @@ static int btrfs_show_devname(struct seq_file *m, struct dentry *root)
 	while (cur_devices) {
 		head = &cur_devices->devices;
 		list_for_each_entry(dev, head, dev_list) {
-			if (dev->missing)
+			if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state))
 				continue;
 			if (!dev->name)
 				continue;
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 7100c877748d..c6f7f4935dc4 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -758,9 +758,9 @@ static noinline int device_list_add(const char *path,
 			return -ENOMEM;
 		rcu_string_free(device->name);
 		rcu_assign_pointer(device->name, name);
-		if (device->missing) {
+		if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state)) {
 			fs_devices->missing_devices--;
-			device->missing = 0;
+			clear_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state);
 		}
 	}
 
@@ -944,7 +944,7 @@ static void btrfs_prepare_close_one_device(struct btrfs_device *device)
 		fs_devices->rw_devices--;
 	}
 
-	if (device->missing)
+	if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
 		fs_devices->missing_devices--;
 
 	new_device = btrfs_alloc_device(NULL, &device->devid,
@@ -1836,7 +1836,8 @@ static struct btrfs_device * btrfs_find_next_active_device(
 
 	list_for_each_entry(next_device, &fs_devs->devices, dev_list) {
 		if (next_device != device &&
-			!next_device->missing && next_device->bdev)
+			!test_bit(BTRFS_DEV_STATE_MISSING,
+				&next_device->dev_state) && next_device->bdev)
 			return next_device;
 	}
 
@@ -1949,7 +1950,7 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
 	device->fs_devices->num_devices--;
 	device->fs_devices->total_devices--;
 
-	if (device->missing)
+	if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
 		device->fs_devices->missing_devices--;
 
 	btrfs_assign_next_active_device(fs_info, device, NULL);
@@ -2023,7 +2024,7 @@ void btrfs_rm_dev_replace_remove_srcdev(struct btrfs_fs_info *fs_info,
 	list_del_rcu(&srcdev->dev_list);
 	list_del(&srcdev->dev_alloc_list);
 	fs_devices->num_devices--;
-	if (srcdev->missing)
+	if (test_bit(BTRFS_DEV_STATE_MISSING, &srcdev->dev_state))
 		fs_devices->missing_devices--;
 
 	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &srcdev->dev_state))
@@ -5048,7 +5049,8 @@ int btrfs_chunk_readonly(struct btrfs_fs_info *fs_info, u64 chunk_offset)
 
 	map = em->map_lookup;
 	for (i = 0; i < map->num_stripes; i++) {
-		if (map->stripes[i].dev->missing) {
+		if (test_bit(BTRFS_DEV_STATE_MISSING,
+					&map->stripes[i].dev->dev_state)) {
 			miss_ndevs++;
 			continue;
 		}
@@ -6076,7 +6078,8 @@ static noinline void btrfs_schedule_bio(struct btrfs_device *device,
 	int should_queue = 1;
 	struct btrfs_pending_bios *pending_bios;
 
-	if (device->missing || !device->bdev) {
+	if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state) ||
+			!device->bdev) {
 		bio_io_error(bio);
 		return;
 	}
@@ -6272,7 +6275,7 @@ static struct btrfs_device *add_missing_dev(struct btrfs_fs_devices *fs_devices,
 	device->fs_devices = fs_devices;
 	fs_devices->num_devices++;
 
-	device->missing = 1;
+	set_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state);
 	fs_devices->missing_devices++;
 
 	return device;
@@ -6635,7 +6638,8 @@ static int read_one_dev(struct btrfs_fs_info *fs_info,
 							dev_uuid, false);
 		}
 
-		if(!device->bdev && !device->missing) {
+		if (!device->bdev && !test_bit(BTRFS_DEV_STATE_MISSING,
+							&device->dev_state)) {
 			/*
 			 * this happens when a device that was properly setup
 			 * in the device info lists suddenly goes bad.
@@ -6643,12 +6647,13 @@ static int read_one_dev(struct btrfs_fs_info *fs_info,
 			 * device->missing to one here
 			 */
 			device->fs_devices->missing_devices++;
-			device->missing = 1;
+			set_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state);
 		}
 
 		/* Move the device to its own fs_devices */
 		if (device->fs_devices != fs_devices) {
-			ASSERT(device->missing);
+			ASSERT(test_bit(BTRFS_DEV_STATE_MISSING,
+							&device->dev_state));
 
 			list_move(&device->dev_list, &fs_devices->devices);
 			device->fs_devices->num_devices--;
@@ -6834,8 +6839,9 @@ bool btrfs_check_rw_degradable(struct btrfs_fs_info *fs_info)
 		for (i = 0; i < map->num_stripes; i++) {
 			struct btrfs_device *dev = map->stripes[i].dev;
 
-			if (!dev || !dev->bdev || dev->missing ||
-			    dev->last_flush_error)
+			if (!dev || !dev->bdev ||
+				test_bit(BTRFS_DEV_STATE_MISSING,
+				&dev->dev_state) || dev->last_flush_error)
 				missing++;
 		}
 		if (missing > max_tolerated) {
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 4c369ec9efac..df67fea18708 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -49,6 +49,7 @@ struct btrfs_pending_bios {
 
 #define BTRFS_DEV_STATE_WRITEABLE	(1UL << 0)
 #define BTRFS_DEV_STATE_IN_FS_METADATA	(1UL << 1)
+#define BTRFS_DEV_STATE_MISSING		(1UL << 2)
 
 struct btrfs_device {
 	struct list_head dev_list;
@@ -73,7 +74,6 @@ struct btrfs_device {
 	fmode_t mode;
 
 	unsigned long dev_state;
-	int missing;
 	int is_tgtdev_for_dev_replace;
 	blk_status_t last_flush_error;
 	int flush_bio_sent;
-- 
2.15.0


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

* [PATCH v2 4/5] btrfs: cleanup device states define BTRFS_DEV_STATE_REPLACE_TGT
  2017-12-04  4:54 [PATCH v2 0/5] define BTRFS_DEV_STATE Anand Jain
                   ` (2 preceding siblings ...)
  2017-12-04  4:54 ` [PATCH v2 3/5] btrfs: cleanup device states define BTRFS_DEV_STATE_MISSING Anand Jain
@ 2017-12-04  4:54 ` Anand Jain
  2017-12-04  4:54 ` [PATCH v2 5/5] btrfs: cleanup device states define BTRFS_DEV_STATE_FLUSH_SENT Anand Jain
  2017-12-04 20:28 ` [PATCH v2 0/5] define BTRFS_DEV_STATE David Sterba
  5 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2017-12-04  4:54 UTC (permalink / raw)
  To: linux-btrfs

Currently device state is being managed by each individual int
variable such as struct btrfs_device::is_tgtdev_for_dev_replace.
Instead of that declare btrfs_device::dev_state
BTRFS_DEV_STATE_MISSING and use the bit operations.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/dev-replace.c |  5 +++--
 fs/btrfs/extent-tree.c |  3 ++-
 fs/btrfs/ioctl.c       |  2 +-
 fs/btrfs/scrub.c       |  2 +-
 fs/btrfs/super.c       |  5 +++--
 fs/btrfs/volumes.c     | 39 ++++++++++++++++++++++-----------------
 fs/btrfs/volumes.h     |  2 +-
 7 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 559db7667f38..12fd8a203735 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -172,7 +172,8 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info)
 				dev_replace->tgtdev->commit_bytes_used =
 					dev_replace->srcdev->commit_bytes_used;
 			}
-			dev_replace->tgtdev->is_tgtdev_for_dev_replace = 1;
+			set_bit(BTRFS_DEV_STATE_REPLACE_TGT,
+					&dev_replace->tgtdev->dev_state);
 			btrfs_init_dev_replace_tgtdev_for_resume(fs_info,
 				dev_replace->tgtdev);
 		}
@@ -564,7 +565,7 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
 			  dev_missing_or_rcu_str(src_device),
 			  src_device->devid,
 			  rcu_str_deref(tgt_device->name));
-	tgt_device->is_tgtdev_for_dev_replace = 0;
+	clear_bit(BTRFS_DEV_STATE_REPLACE_TGT, &tgt_device->dev_state);
 	tgt_device->devid = src_device->devid;
 	src_device->devid = BTRFS_DEV_REPLACE_DEVID;
 	memcpy(uuid_tmp, tgt_device->uuid, sizeof(uuid_tmp));
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 2cd323d184a0..1e65d5d54a8a 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -9692,7 +9692,8 @@ int btrfs_can_relocate(struct btrfs_fs_info *fs_info, u64 bytenr)
 		 * space to fit our block group in.
 		 */
 		if (device->total_bytes > device->bytes_used + min_free &&
-		    !device->is_tgtdev_for_dev_replace) {
+				!test_bit(BTRFS_DEV_STATE_REPLACE_TGT,
+				&device->dev_state)) {
 			ret = find_free_dev_extent(trans, device, min_free,
 						   &dev_offset, NULL);
 			if (!ret)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e59004a17166..953563138020 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1528,7 +1528,7 @@ static noinline int btrfs_ioctl_resize(struct file *file,
 		}
 	}
 
-	if (device->is_tgtdev_for_dev_replace) {
+	if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) {
 		ret = -EPERM;
 		goto out_free;
 	}
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index b6de017066b3..b5a33db38874 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -4131,7 +4131,7 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start,
 
 	mutex_lock(&fs_info->scrub_lock);
 	if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &dev->dev_state) ||
-					dev->is_tgtdev_for_dev_replace) {
+		test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &dev->dev_state)) {
 		mutex_unlock(&fs_info->scrub_lock);
 		mutex_unlock(&fs_info->fs_devices->device_list_mutex);
 		return -EIO;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 6bae2e046257..b16e3fbd5895 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1973,8 +1973,9 @@ static int btrfs_calc_avail_data_space(struct btrfs_fs_info *fs_info,
 	rcu_read_lock();
 	list_for_each_entry_rcu(device, &fs_devices->devices, dev_list) {
 		if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
-						&device->dev_state) ||
-			!device->bdev || device->is_tgtdev_for_dev_replace)
+				&device->dev_state) || !device->bdev ||
+				test_bit(BTRFS_DEV_STATE_REPLACE_TGT,
+				&device->dev_state))
 			continue;
 
 		if (i >= nr_devices)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index c6f7f4935dc4..37b1aed14353 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -845,8 +845,8 @@ void btrfs_close_extra_devices(struct btrfs_fs_devices *fs_devices, int step)
 	list_for_each_entry_safe(device, next, &fs_devices->devices, dev_list) {
 		if (test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
 							&device->dev_state)) {
-			if (!device->is_tgtdev_for_dev_replace &&
-			    (!latest_dev ||
+			if (!test_bit(BTRFS_DEV_STATE_REPLACE_TGT,
+			     &device->dev_state) && (!latest_dev ||
 			     device->generation > latest_dev->generation)) {
 				latest_dev = device;
 			}
@@ -864,7 +864,8 @@ void btrfs_close_extra_devices(struct btrfs_fs_devices *fs_devices, int step)
 			 * not, which means whether this device is
 			 * used or whether it should be removed.
 			 */
-			if (step == 0 || device->is_tgtdev_for_dev_replace) {
+			if (step == 0 || test_bit(BTRFS_DEV_STATE_REPLACE_TGT,
+					&device->dev_state)) {
 				continue;
 			}
 		}
@@ -877,7 +878,8 @@ void btrfs_close_extra_devices(struct btrfs_fs_devices *fs_devices, int step)
 			list_del_init(&device->dev_alloc_list);
 			clear_bit(BTRFS_DEV_STATE_WRITEABLE,
 							&device->dev_state);
-			if (!device->is_tgtdev_for_dev_replace)
+			if (!test_bit(BTRFS_DEV_STATE_REPLACE_TGT,
+							&device->dev_state))
 				fs_devices->rw_devices--;
 		}
 		list_del_init(&device->dev_list);
@@ -1204,7 +1206,8 @@ int btrfs_account_dev_extents_size(struct btrfs_device *device, u64 start,
 
 	*length = 0;
 
-	if (start >= device->total_bytes || device->is_tgtdev_for_dev_replace)
+	if (start >= device->total_bytes ||
+		test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state))
 		return 0;
 
 	path = btrfs_alloc_path();
@@ -1382,7 +1385,8 @@ int find_free_dev_extent_start(struct btrfs_transaction *transaction,
 	max_hole_size = 0;
 
 again:
-	if (search_start >= search_end || device->is_tgtdev_for_dev_replace) {
+	if (search_start >= search_end ||
+		test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) {
 		ret = -ENOSPC;
 		goto out;
 	}
@@ -1590,7 +1594,7 @@ static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
 	struct btrfs_key key;
 
 	WARN_ON(!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state));
-	WARN_ON(device->is_tgtdev_for_dev_replace);
+	WARN_ON(test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state));
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;
@@ -1897,7 +1901,7 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
 	if (ret)
 		goto out;
 
-	if (device->is_tgtdev_for_dev_replace) {
+	if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) {
 		ret = BTRFS_ERROR_DEV_TGT_REPLACE;
 		goto out;
 	}
@@ -2408,7 +2412,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	device->fs_info = fs_info;
 	device->bdev = bdev;
 	set_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
-	device->is_tgtdev_for_dev_replace = 0;
+	clear_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state);
 	device->mode = FMODE_EXCL;
 	device->dev_stats_valid = 1;
 	set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
@@ -2619,7 +2623,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
 	device->fs_info = fs_info;
 	device->bdev = bdev;
 	set_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
-	device->is_tgtdev_for_dev_replace = 1;
+	set_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state);
 	device->mode = FMODE_EXCL;
 	device->dev_stats_valid = 1;
 	set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
@@ -2715,7 +2719,7 @@ int btrfs_grow_device(struct btrfs_trans_handle *trans,
 	diff = round_down(new_size - device->total_bytes, fs_info->sectorsize);
 
 	if (new_size <= device->total_bytes ||
-	    device->is_tgtdev_for_dev_replace) {
+	    test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) {
 		mutex_unlock(&fs_info->chunk_mutex);
 		return -EINVAL;
 	}
@@ -3528,7 +3532,7 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
 		if (!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) ||
 		    btrfs_device_get_total_bytes(device) -
 		    btrfs_device_get_bytes_used(device) > size_to_free ||
-		    device->is_tgtdev_for_dev_replace)
+		    test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state))
 			continue;
 
 		ret = btrfs_shrink_device(device, old_size - size_to_free);
@@ -4396,7 +4400,7 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size)
 	new_size = round_down(new_size, fs_info->sectorsize);
 	diff = round_down(old_size - new_size, fs_info->sectorsize);
 
-	if (device->is_tgtdev_for_dev_replace)
+	if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state))
 		return -EINVAL;
 
 	path = btrfs_alloc_path();
@@ -4700,8 +4704,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		}
 
 		if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
-					&device->dev_state) ||
-					device->is_tgtdev_for_dev_replace)
+				&device->dev_state) ||
+				test_bit(BTRFS_DEV_STATE_REPLACE_TGT,
+				&device->dev_state))
 			continue;
 
 		if (device->total_bytes > device->bytes_used)
@@ -6526,7 +6531,7 @@ static void fill_device_from_item(struct extent_buffer *leaf,
 	device->io_width = btrfs_device_io_width(leaf, dev_item);
 	device->sector_size = btrfs_device_sector_size(leaf, dev_item);
 	WARN_ON(device->devid == BTRFS_DEV_REPLACE_DEVID);
-	device->is_tgtdev_for_dev_replace = 0;
+	clear_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state);
 
 	ptr = btrfs_device_uuid(dev_item);
 	read_extent_buffer(leaf, device->uuid, ptr, BTRFS_UUID_SIZE);
@@ -6676,7 +6681,7 @@ static int read_one_dev(struct btrfs_fs_info *fs_info,
 	fill_device_from_item(leaf, dev_item, device);
 	set_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state);
 	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
-					!device->is_tgtdev_for_dev_replace) {
+	   !test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state)) {
 		device->fs_devices->total_rw_bytes += device->total_bytes;
 		atomic64_add(device->total_bytes - device->bytes_used,
 				&fs_info->free_chunk_space);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index df67fea18708..4096350c2cea 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -50,6 +50,7 @@ struct btrfs_pending_bios {
 #define BTRFS_DEV_STATE_WRITEABLE	(1UL << 0)
 #define BTRFS_DEV_STATE_IN_FS_METADATA	(1UL << 1)
 #define BTRFS_DEV_STATE_MISSING		(1UL << 2)
+#define BTRFS_DEV_STATE_REPLACE_TGT	(1UL << 3)
 
 struct btrfs_device {
 	struct list_head dev_list;
@@ -74,7 +75,6 @@ struct btrfs_device {
 	fmode_t mode;
 
 	unsigned long dev_state;
-	int is_tgtdev_for_dev_replace;
 	blk_status_t last_flush_error;
 	int flush_bio_sent;
 
-- 
2.15.0


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

* [PATCH v2 5/5] btrfs: cleanup device states define BTRFS_DEV_STATE_FLUSH_SENT
  2017-12-04  4:54 [PATCH v2 0/5] define BTRFS_DEV_STATE Anand Jain
                   ` (3 preceding siblings ...)
  2017-12-04  4:54 ` [PATCH v2 4/5] btrfs: cleanup device states define BTRFS_DEV_STATE_REPLACE_TGT Anand Jain
@ 2017-12-04  4:54 ` Anand Jain
  2017-12-04 20:28 ` [PATCH v2 0/5] define BTRFS_DEV_STATE David Sterba
  5 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2017-12-04  4:54 UTC (permalink / raw)
  To: linux-btrfs

Currently device state is being managed by each individual int
variable such as struct btrfs_device::is_tgtdev_for_dev_replace.
Instead of that declare btrfs_device::dev_state
BTRFS_DEV_STATE_FLUSH_SENT and use the bit operations.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/disk-io.c | 6 +++---
 fs/btrfs/volumes.h | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 890e3a6a2f3e..9b20c1f3563b 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3359,7 +3359,7 @@ static void write_dev_flush(struct btrfs_device *device)
 	bio->bi_private = &device->flush_wait;
 
 	btrfsic_submit_bio(bio);
-	device->flush_bio_sent = 1;
+	set_bit(BTRFS_DEV_STATE_FLUSH_SENT, &device->dev_state);
 }
 
 /*
@@ -3369,10 +3369,10 @@ static blk_status_t wait_dev_flush(struct btrfs_device *device)
 {
 	struct bio *bio = device->flush_bio;
 
-	if (!device->flush_bio_sent)
+	if (!test_bit(BTRFS_DEV_STATE_FLUSH_SENT, &device->dev_state))
 		return BLK_STS_OK;
 
-	device->flush_bio_sent = 0;
+	clear_bit(BTRFS_DEV_STATE_FLUSH_SENT, &device->dev_state);
 	wait_for_completion_io(&device->flush_wait);
 
 	return bio->bi_status;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 4096350c2cea..7acfd61611aa 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -51,6 +51,7 @@ struct btrfs_pending_bios {
 #define BTRFS_DEV_STATE_IN_FS_METADATA	(1UL << 1)
 #define BTRFS_DEV_STATE_MISSING		(1UL << 2)
 #define BTRFS_DEV_STATE_REPLACE_TGT	(1UL << 3)
+#define BTRFS_DEV_STATE_FLUSH_SENT	(1UL << 4)
 
 struct btrfs_device {
 	struct list_head dev_list;
-- 
2.15.0


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

* Re: [PATCH v2 0/5] define BTRFS_DEV_STATE
  2017-12-04  4:54 [PATCH v2 0/5] define BTRFS_DEV_STATE Anand Jain
                   ` (4 preceding siblings ...)
  2017-12-04  4:54 ` [PATCH v2 5/5] btrfs: cleanup device states define BTRFS_DEV_STATE_FLUSH_SENT Anand Jain
@ 2017-12-04 20:28 ` David Sterba
  2017-12-05  1:20   ` Anand Jain
  5 siblings, 1 reply; 10+ messages in thread
From: David Sterba @ 2017-12-04 20:28 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Mon, Dec 04, 2017 at 12:54:51PM +0800, Anand Jain wrote:
> As of now device properties and states are being represented as int
> variable. So clean that up using bitwise operations. Also patches in
> the ML such as device failed state needs this cleanup as well.
> 
> V2:
>  Accepts all comments from Nikolay.
>  Drops can_discard.
>  Adds BTRFS_DEV_STATE_REPLACE_TGT and BTRFS_DEV_STATE_FLUSH_SENT
>     patches.
> 
> Anand Jain (5):
>   btrfs: cleanup device states define BTRFS_DEV_STATE_WRITEABLE
>   btrfs: cleanup device states define BTRFS_DEV_STATE_IN_FS_METADATA
>   btrfs: cleanup device states define BTRFS_DEV_STATE_MISSING
>   btrfs: cleanup device states define BTRFS_DEV_STATE_REPLACE_TGT
>   btrfs: cleanup device states define BTRFS_DEV_STATE_FLUSH_SENT

1-5 added to next. I had to tweak the whitespace in the conditions.

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

* Re: [PATCH v2 0/5] define BTRFS_DEV_STATE
  2017-12-04 20:28 ` [PATCH v2 0/5] define BTRFS_DEV_STATE David Sterba
@ 2017-12-05  1:20   ` Anand Jain
  2017-12-05 13:07     ` David Sterba
  0 siblings, 1 reply; 10+ messages in thread
From: Anand Jain @ 2017-12-05  1:20 UTC (permalink / raw)
  To: dsterba, linux-btrfs



On 12/05/2017 04:28 AM, David Sterba wrote:
> On Mon, Dec 04, 2017 at 12:54:51PM +0800, Anand Jain wrote:
>> As of now device properties and states are being represented as int
>> variable. So clean that up using bitwise operations. Also patches in
>> the ML such as device failed state needs this cleanup as well.
>>
>> V2:
>>   Accepts all comments from Nikolay.
>>   Drops can_discard.
>>   Adds BTRFS_DEV_STATE_REPLACE_TGT and BTRFS_DEV_STATE_FLUSH_SENT
>>      patches.
>>
>> Anand Jain (5):
>>    btrfs: cleanup device states define BTRFS_DEV_STATE_WRITEABLE
>>    btrfs: cleanup device states define BTRFS_DEV_STATE_IN_FS_METADATA
>>    btrfs: cleanup device states define BTRFS_DEV_STATE_MISSING
>>    btrfs: cleanup device states define BTRFS_DEV_STATE_REPLACE_TGT
>>    btrfs: cleanup device states define BTRFS_DEV_STATE_FLUSH_SENT
> 
> 1-5 added to next. I had to tweak the whitespace in the conditions.

  Oops I did run, checkpatch, not too sure how did I still missed it.
  Sorry about that. Thanks.
  Anand

> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH v2 0/5] define BTRFS_DEV_STATE
  2017-12-05  1:20   ` Anand Jain
@ 2017-12-05 13:07     ` David Sterba
  2017-12-05 21:24       ` Anand Jain
  0 siblings, 1 reply; 10+ messages in thread
From: David Sterba @ 2017-12-05 13:07 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Tue, Dec 05, 2017 at 09:20:38AM +0800, Anand Jain wrote:
> 
> 
> On 12/05/2017 04:28 AM, David Sterba wrote:
> > On Mon, Dec 04, 2017 at 12:54:51PM +0800, Anand Jain wrote:
> >> As of now device properties and states are being represented as int
> >> variable. So clean that up using bitwise operations. Also patches in
> >> the ML such as device failed state needs this cleanup as well.
> >>
> >> V2:
> >>   Accepts all comments from Nikolay.
> >>   Drops can_discard.
> >>   Adds BTRFS_DEV_STATE_REPLACE_TGT and BTRFS_DEV_STATE_FLUSH_SENT
> >>      patches.
> >>
> >> Anand Jain (5):
> >>    btrfs: cleanup device states define BTRFS_DEV_STATE_WRITEABLE
> >>    btrfs: cleanup device states define BTRFS_DEV_STATE_IN_FS_METADATA
> >>    btrfs: cleanup device states define BTRFS_DEV_STATE_MISSING
> >>    btrfs: cleanup device states define BTRFS_DEV_STATE_REPLACE_TGT
> >>    btrfs: cleanup device states define BTRFS_DEV_STATE_FLUSH_SENT
> > 
> > 1-5 added to next. I had to tweak the whitespace in the conditions.
> 
>   Oops I did run, checkpatch, not too sure how did I still missed it.

Checkpatch will not help, this is a matter of style used in btrfs code.
We should tweak the coding style so it looks consistent and familiar to
us. I read a lot of code so it's quite obvious to me and need to fix it
if it's feasible or ask the submitter.

An example:

@@ -3403,7 +3403,8 @@  static int barrier_all_devices(struct btrfs_fs_info *info)
 			continue;
 		if (!dev->bdev)
 			continue;
-		if (!dev->in_fs_metadata || !dev->writeable)
+		if (!dev->in_fs_metadata ||
+			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
 			continue;
 
 		write_dev_flush(dev);

The condition on the next line should start under the first one like

-		if (!dev->in_fs_metadata || !dev->writeable)
+		if (!dev->in_fs_metadata ||
+		    !test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
 			continue;

As it makes a bit clear what's the condition and what's the statement.
This can become tricky with condition terms that do not fit on one line
or are tested in ( ) :

@@ -3403,8 +3403,10 @@  static int barrier_all_devices(struct btrfs_fs_info *info)
 			continue;
 		if (!dev->bdev)
 			continue;
-		if (!dev->in_fs_metadata ||
-			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
+		if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
+						&dev->dev_state) ||
+			!test_bit(BTRFS_DEV_STATE_WRITEABLE,
+						&dev->dev_state))
 			continue;

Became

@@ -3395,7 +3395,7 @@ static int barrier_all_devices(struct btrfs_fs_info *info)
                        continue;
                if (!dev->bdev)
                        continue;
-               if (!dev->in_fs_metadata ||
+               if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &dev->dev_state) ||
                    !test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
                        continue;

You can notice in other patches, that the || operator ends up on column 81,
which is exactly where checkpatch would complain but I will not, as the
operator is completely hidden. The end result is IMHO better.

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

* Re: [PATCH v2 0/5] define BTRFS_DEV_STATE
  2017-12-05 13:07     ` David Sterba
@ 2017-12-05 21:24       ` Anand Jain
  0 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2017-12-05 21:24 UTC (permalink / raw)
  To: dsterba, linux-btrfs



On 12/05/2017 09:07 PM, David Sterba wrote:
> On Tue, Dec 05, 2017 at 09:20:38AM +0800, Anand Jain wrote:
>>
>>
>> On 12/05/2017 04:28 AM, David Sterba wrote:
>>> On Mon, Dec 04, 2017 at 12:54:51PM +0800, Anand Jain wrote:
>>>> As of now device properties and states are being represented as int
>>>> variable. So clean that up using bitwise operations. Also patches in
>>>> the ML such as device failed state needs this cleanup as well.
>>>>
>>>> V2:
>>>>    Accepts all comments from Nikolay.
>>>>    Drops can_discard.
>>>>    Adds BTRFS_DEV_STATE_REPLACE_TGT and BTRFS_DEV_STATE_FLUSH_SENT
>>>>       patches.
>>>>
>>>> Anand Jain (5):
>>>>     btrfs: cleanup device states define BTRFS_DEV_STATE_WRITEABLE
>>>>     btrfs: cleanup device states define BTRFS_DEV_STATE_IN_FS_METADATA
>>>>     btrfs: cleanup device states define BTRFS_DEV_STATE_MISSING
>>>>     btrfs: cleanup device states define BTRFS_DEV_STATE_REPLACE_TGT
>>>>     btrfs: cleanup device states define BTRFS_DEV_STATE_FLUSH_SENT
>>>
>>> 1-5 added to next. I had to tweak the whitespace in the conditions.
>>
>>    Oops I did run, checkpatch, not too sure how did I still missed it.
> 
> Checkpatch will not help, this is a matter of style used in btrfs code.
> We should tweak the coding style so it looks consistent and familiar to
> us. I read a lot of code so it's quite obvious to me and need to fix it
> if it's feasible or ask the submitter.
> 
> An example:
> 
> @@ -3403,7 +3403,8 @@  static int barrier_all_devices(struct btrfs_fs_info *info)
>   			continue;
>   		if (!dev->bdev)
>   			continue;
> -		if (!dev->in_fs_metadata || !dev->writeable)
> +		if (!dev->in_fs_metadata ||
> +			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
>   			continue;
>   
>   		write_dev_flush(dev);
> 
> The condition on the next line should start under the first one like
> 
> -		if (!dev->in_fs_metadata || !dev->writeable)
> +		if (!dev->in_fs_metadata ||
> +		    !test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
>   			continue;
> 
> As it makes a bit clear what's the condition and what's the statement.
> This can become tricky with condition terms that do not fit on one line
> or are tested in ( ) :
> 
> @@ -3403,8 +3403,10 @@  static int barrier_all_devices(struct btrfs_fs_info *info)
>   			continue;
>   		if (!dev->bdev)
>   			continue;
> -		if (!dev->in_fs_metadata ||
> -			!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
> +		if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
> +						&dev->dev_state) ||
> +			!test_bit(BTRFS_DEV_STATE_WRITEABLE,
> +						&dev->dev_state))
>   			continue;
> 
> Became
> 
> @@ -3395,7 +3395,7 @@ static int barrier_all_devices(struct btrfs_fs_info *info)
>                          continue;
>                  if (!dev->bdev)
>                          continue;
> -               if (!dev->in_fs_metadata ||
> +               if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &dev->dev_state) ||
>                      !test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
>                          continue;
> 
> You can notice in other patches, that the || operator ends up on column 81,
> which is exactly where checkpatch would complain but I will not, as the
> operator is completely hidden. The end result is IMHO better.

    Ah. Thanks for explaining. I like the one you used,
    much easy to read.

- Anand

> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

end of thread, other threads:[~2017-12-05 21:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-04  4:54 [PATCH v2 0/5] define BTRFS_DEV_STATE Anand Jain
2017-12-04  4:54 ` [PATCH v2 1/5] btrfs: cleanup device states define BTRFS_DEV_STATE_WRITEABLE Anand Jain
2017-12-04  4:54 ` [PATCH v2 2/5] btrfs: cleanup device states define BTRFS_DEV_STATE_IN_FS_METADATA Anand Jain
2017-12-04  4:54 ` [PATCH v2 3/5] btrfs: cleanup device states define BTRFS_DEV_STATE_MISSING Anand Jain
2017-12-04  4:54 ` [PATCH v2 4/5] btrfs: cleanup device states define BTRFS_DEV_STATE_REPLACE_TGT Anand Jain
2017-12-04  4:54 ` [PATCH v2 5/5] btrfs: cleanup device states define BTRFS_DEV_STATE_FLUSH_SENT Anand Jain
2017-12-04 20:28 ` [PATCH v2 0/5] define BTRFS_DEV_STATE David Sterba
2017-12-05  1:20   ` Anand Jain
2017-12-05 13:07     ` David Sterba
2017-12-05 21:24       ` Anand Jain

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.