| Message ID | 8c201990ae0acb3b80437f9de2e432b60ecf8e4a.1497544265.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/16/2017 12:49 AM, David Sterba wrote: > We need a device capable of device barriers so we can test the flush > code. To aid testing, add a per-filesystem status flag that affects the > barriers regerdless of the device capabilities and obviously does not > give the same guarantees. > > It's off by default, sysfs tunable will follow. > > Signed-off-by: David Sterba <dsterba@suse.com> > --- > fs/btrfs/ctree.h | 1 + > fs/btrfs/disk-io.c | 8 +++++--- > fs/btrfs/volumes.h | 1 + > 3 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index f0f5f28784b6..dcf4404f7d61 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -716,6 +716,7 @@ struct btrfs_delayed_root; > #define BTRFS_FS_LOG1_ERR 12 > #define BTRFS_FS_LOG2_ERR 13 > #define BTRFS_FS_QUOTA_OVERRIDE 14 > +#define BTRFS_FS_FORCE_DEV_FLUSH 15 > > /* > * Indicate that a whole-filesystem exclusive operation is running > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 59a732a13370..659a3b4645d2 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -3494,7 +3494,8 @@ static void write_dev_flush(struct btrfs_device *device) > struct request_queue *q = bdev_get_queue(device->bdev); > struct bio *bio = device->flush_bio; > > - if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) > + if (!test_bit(BTRFS_FS_FORCE_DEV_FLUSH, &device->fs_info->flags) > + && !test_bit(QUEUE_FLAG_WC, &q->queue_flags)) > return; Now I understand what you meant. But the most common case in our test set up is a device with write cache. So BTRFS_FS_FORCE_DEV_FLUSH does not bring any additional force. IMO. Thanks, Anand > bio_reset(bio); > @@ -3505,6 +3506,7 @@ static void write_dev_flush(struct btrfs_device *device) > bio->bi_private = &device->flush_wait; > > submit_bio(bio); > + device->flush_bio_sent = 1; > } > > /* > @@ -3512,12 +3514,12 @@ static void write_dev_flush(struct btrfs_device *device) > */ > static int wait_dev_flush(struct btrfs_device *device) > { > - struct request_queue *q = bdev_get_queue(device->bdev); > struct bio *bio = device->flush_bio; > > - if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) > + if (!device->flush_bio_sent) > return 0; > > + device->flush_bio_sent = 0; > wait_for_completion_io(&device->flush_wait); > > return bio->bi_error; > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 35327efecdbb..6f45fd60d15a 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -75,6 +75,7 @@ struct btrfs_device { > int can_discard; > int is_tgtdev_for_dev_replace; > int last_flush_error; > + int flush_bio_sent; > > #ifdef __BTRFS_NEED_DEVICE_DATA_ORDERED > seqcount_t data_seqcount; > -- 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
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >> index 59a732a13370..659a3b4645d2 100644 >> --- a/fs/btrfs/disk-io.c >> +++ b/fs/btrfs/disk-io.c >> @@ -3494,7 +3494,8 @@ static void write_dev_flush(struct btrfs_device >> *device) >> struct request_queue *q = bdev_get_queue(device->bdev); >> struct bio *bio = device->flush_bio; >> - if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) >> + if (!test_bit(BTRFS_FS_FORCE_DEV_FLUSH, &device->fs_info->flags) >> + && !test_bit(QUEUE_FLAG_WC, &q->queue_flags)) >> return; > > > Now I understand what you meant. But the most common case in our test > set up is a device with write cache. So BTRFS_FS_FORCE_DEV_FLUSH does > not bring any additional force. IMO. > > Thanks, Anand Or one another idea is we could remove !test_bit(QUEUE_FLAG_WC, &q->queue_flags) which purpose is to only fail early. If we remove it there is consistency in our code with or with out the write cache. 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
On Fri, Jun 16, 2017 at 06:27:20AM +0800, Anand Jain wrote: > > > > > >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > >> index 59a732a13370..659a3b4645d2 100644 > >> --- a/fs/btrfs/disk-io.c > >> +++ b/fs/btrfs/disk-io.c > >> @@ -3494,7 +3494,8 @@ static void write_dev_flush(struct btrfs_device > >> *device) > >> struct request_queue *q = bdev_get_queue(device->bdev); > >> struct bio *bio = device->flush_bio; > >> - if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) > >> + if (!test_bit(BTRFS_FS_FORCE_DEV_FLUSH, &device->fs_info->flags) > >> + && !test_bit(QUEUE_FLAG_WC, &q->queue_flags)) > >> return; > > > > > > Now I understand what you meant. But the most common case in our test > > set up is a device with write cache. So BTRFS_FS_FORCE_DEV_FLUSH does > > not bring any additional force. IMO. > > > > Thanks, Anand > > Or one another idea is we could remove > > !test_bit(QUEUE_FLAG_WC, &q->queue_flags) > > which purpose is to only fail early. I'd prefer to keep it that way. > If we remove it there is consistency in our code with or > with out the write cache. -- 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
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index f0f5f28784b6..dcf4404f7d61 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -716,6 +716,7 @@ struct btrfs_delayed_root; #define BTRFS_FS_LOG1_ERR 12 #define BTRFS_FS_LOG2_ERR 13 #define BTRFS_FS_QUOTA_OVERRIDE 14 +#define BTRFS_FS_FORCE_DEV_FLUSH 15 /* * Indicate that a whole-filesystem exclusive operation is running diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 59a732a13370..659a3b4645d2 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -3494,7 +3494,8 @@ static void write_dev_flush(struct btrfs_device *device) struct request_queue *q = bdev_get_queue(device->bdev); struct bio *bio = device->flush_bio; - if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) + if (!test_bit(BTRFS_FS_FORCE_DEV_FLUSH, &device->fs_info->flags) + && !test_bit(QUEUE_FLAG_WC, &q->queue_flags)) return; bio_reset(bio); @@ -3505,6 +3506,7 @@ static void write_dev_flush(struct btrfs_device *device) bio->bi_private = &device->flush_wait; submit_bio(bio); + device->flush_bio_sent = 1; } /* @@ -3512,12 +3514,12 @@ static void write_dev_flush(struct btrfs_device *device) */ static int wait_dev_flush(struct btrfs_device *device) { - struct request_queue *q = bdev_get_queue(device->bdev); struct bio *bio = device->flush_bio; - if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) + if (!device->flush_bio_sent) return 0; + device->flush_bio_sent = 0; wait_for_completion_io(&device->flush_wait); return bio->bi_error; diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 35327efecdbb..6f45fd60d15a 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -75,6 +75,7 @@ struct btrfs_device { int can_discard; int is_tgtdev_for_dev_replace; int last_flush_error; + int flush_bio_sent; #ifdef __BTRFS_NEED_DEVICE_DATA_ORDERED seqcount_t data_seqcount;
We need a device capable of device barriers so we can test the flush code. To aid testing, add a per-filesystem status flag that affects the barriers regerdless of the device capabilities and obviously does not give the same guarantees. It's off by default, sysfs tunable will follow. Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/ctree.h | 1 + fs/btrfs/disk-io.c | 8 +++++--- fs/btrfs/volumes.h | 1 + 3 files changed, 7 insertions(+), 3 deletions(-)