[PATCH] btrfs: delete unused member nobarriers

[PATCH] btrfs: delete unused member nobarriers

[Anand Jain]

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

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 08b74daf35d0..9de35bca1f67 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3521,9 +3521,6 @@ static int write_dev_flush(struct btrfs_device *device, int wait)
 	struct bio *bio;
 	int ret = 0;
 
-	if (device->nobarriers)
-		return 0;
-
 	if (wait) {
 		bio = device->flush_bio;
 		if (!bio)
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 59be81206dd7..fa0b79422695 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -123,7 +123,6 @@ struct btrfs_device {
 	struct list_head resized_list;
 
 	/* for sending down flush barriers */
-	int nobarriers;
 	struct bio *flush_bio;
 	struct completion flush_wait;
 
-- 
2.10.0

Re: [PATCH] btrfs: delete unused member nobarriers

[David Sterba]

Please update the changelog to say why it's ok to remove it, eg. the
commit that removed the last user.

commit b25de9d6da49b1a8760a89672283128aa8c78345
Author: Christoph Hellwig <hch@lst.de>
Date:   Fri Apr 24 21:41:01 2015 +0200

    block: remove BIO_EOPNOTSUPP

Otherwise the patch is ok.

Re: [PATCH] btrfs: delete unused member nobarriers

[Anand Jain]

On 04/03/2017 08:06 PM, David Sterba wrote:
> Please update the changelog to say why it's ok to remove it, eg. the
> commit that removed the last user.
>
> commit b25de9d6da49b1a8760a89672283128aa8c78345
> Author: Christoph Hellwig <hch@lst.de>
> Date:   Fri Apr 24 21:41:01 2015 +0200
>
>     block: remove BIO_EOPNOTSUPP

  Ah. I should have search the commit log. sorry about that.


IMO, there is a bug in generic_make_request_checks() in which
it should rather return EOPNOTSUPP, instead of EIO if QUEUE_FLAG_WC
is not supported.

------------------------------------------------
1853 static noinline_for_stack bool
1854 generic_make_request_checks(struct bio *bio)
1855 {
::

1892         /*
1893          * Filter flush bio's early so that make_request based
1894          * drivers without flush support don't have to worry
1895          * about them.
1896          */
1897         if ((bio->bi_opf & (REQ_PREFLUSH | REQ_FUA)) &&
1898             !test_bit(QUEUE_FLAG_WC, &q->queue_flags)) {
1899                 bio->bi_opf &= ~(REQ_PREFLUSH | REQ_FUA);
1900                 if (!nr_sectors) {
1901                         err = 0;
1902                         goto end_io; <- this should goto  not_supported
1903                 }
1904         }

::

1946 not_supported:
1947         err = -EOPNOTSUPP;
------------------------------------------------


  Pls ignore this patch.

  I have submitted
[PATCH] btrfs: check if the device is flush capable

  which will remain unaffected by the above bug/not-a-bug
  in the blk core code.

Thanks, Anand


> Otherwise the patch is ok.

Re: [PATCH] btrfs: delete unused member nobarriers

[David Sterba]

On Tue, Apr 04, 2017 at 06:59:19PM +0800, Anand Jain wrote:
> 
> 
> On 04/03/2017 08:06 PM, David Sterba wrote:
> > Please update the changelog to say why it's ok to remove it, eg. the
> > commit that removed the last user.
> >
> > commit b25de9d6da49b1a8760a89672283128aa8c78345
> > Author: Christoph Hellwig <hch@lst.de>
> > Date:   Fri Apr 24 21:41:01 2015 +0200
> >
> >     block: remove BIO_EOPNOTSUPP
> 
>   Ah. I should have search the commit log. sorry about that.
> 
> 
> IMO, there is a bug in generic_make_request_checks() in which
> it should rather return EOPNOTSUPP, instead of EIO if QUEUE_FLAG_WC
> is not supported.

I think it works as intended, ie. it's not a bug. The code has been
added to avoid reporting an error code in the mentioned case. See
generic_make_request that would exit, or need to handle EOPNOTSUPP each
time. If you still think this is a bug, please report it to the
blocklayer maintainers.

> ------------------------------------------------
> 1853 static noinline_for_stack bool
> 1854 generic_make_request_checks(struct bio *bio)
> 1855 {
> ::
> 
> 1892         /*
> 1893          * Filter flush bio's early so that make_request based
> 1894          * drivers without flush support don't have to worry
> 1895          * about them.
> 1896          */
> 1897         if ((bio->bi_opf & (REQ_PREFLUSH | REQ_FUA)) &&
> 1898             !test_bit(QUEUE_FLAG_WC, &q->queue_flags)) {
> 1899                 bio->bi_opf &= ~(REQ_PREFLUSH | REQ_FUA);
> 1900                 if (!nr_sectors) {
> 1901                         err = 0;
> 1902                         goto end_io; <- this should goto  not_supported
> 1903                 }
> 1904         }
> 
> ::
> 
> 1946 not_supported:
> 1947         err = -EOPNOTSUPP;
> ------------------------------------------------
> 
> 
>   Pls ignore this patch.
> 
>   I have submitted
> [PATCH] btrfs: check if the device is flush capable
> 
>   which will remain unaffected by the above bug/not-a-bug
>   in the blk core code.

[PATCH v2] btrfs: delete unused member nobarriers

[Anand Jain]

The last consumer of nobarriers is removed by the commit [1] and sync
won't fail with EOPNOTSUPP anymore. Thus, now even when write cache is
write through it just return success without actually transpiring such
a request to the block device/lun.

[1]
commit b25de9d6da49b1a8760a89672283128aa8c78345
block: remove BIO_EOPNOTSUPP

And, as the device/lun write cache state may change dynamically saving
such as state won't help either. So deleting the member nobarriers.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 v2: We need this patch. Update the commit log.

 fs/btrfs/disk-io.c | 3 ---
 fs/btrfs/volumes.h | 1 -
 2 files changed, 4 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 3c476b118440..b6d047250ce2 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3517,9 +3517,6 @@ static void btrfs_dev_issue_flush(struct work_struct *work)
  */
 static int write_dev_flush(struct btrfs_device *device, int wait)
 {
-	if (device->nobarriers)
-		return 0;
-
 	if (wait) {
 		int ret;
 
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 0df50bc65578..1168b78c5f1d 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -123,7 +123,6 @@ struct btrfs_device {
 	struct list_head resized_list;
 
 	/* for sending down flush barriers */
-	int nobarriers;
 	struct completion flush_wait;
 	struct work_struct flush_work;
 	int last_flush_error;
-- 
2.10.0