* [PATCH] btrfs: delete unused member nobarriers
@ 2017-03-31 11:26 Anand Jain
2017-04-03 12:06 ` David Sterba
2017-04-05 3:51 ` [PATCH v2] " Anand Jain
0 siblings, 2 replies; 5+ messages in thread
From: Anand Jain @ 2017-03-31 11:26 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: delete unused member nobarriers
2017-03-31 11:26 [PATCH] btrfs: delete unused member nobarriers Anand Jain
@ 2017-04-03 12:06 ` David Sterba
2017-04-04 10:59 ` Anand Jain
2017-04-05 3:51 ` [PATCH v2] " Anand Jain
1 sibling, 1 reply; 5+ messages in thread
From: David Sterba @ 2017-04-03 12:06 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, dsterba
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: delete unused member nobarriers
2017-04-03 12:06 ` David Sterba
@ 2017-04-04 10:59 ` Anand Jain
2017-04-05 13:26 ` David Sterba
0 siblings, 1 reply; 5+ messages in thread
From: Anand Jain @ 2017-04-04 10:59 UTC (permalink / raw)
To: dsterba; +Cc: linux-btrfs
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] btrfs: delete unused member nobarriers
2017-03-31 11:26 [PATCH] btrfs: delete unused member nobarriers Anand Jain
2017-04-03 12:06 ` David Sterba
@ 2017-04-05 3:51 ` Anand Jain
1 sibling, 0 replies; 5+ messages in thread
From: Anand Jain @ 2017-04-05 3:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: delete unused member nobarriers
2017-04-04 10:59 ` Anand Jain
@ 2017-04-05 13:26 ` David Sterba
0 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2017-04-05 13:26 UTC (permalink / raw)
To: Anand Jain; +Cc: dsterba, linux-btrfs
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-05 13:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-31 11:26 [PATCH] btrfs: delete unused member nobarriers Anand Jain
2017-04-03 12:06 ` David Sterba
2017-04-04 10:59 ` Anand Jain
2017-04-05 13:26 ` David Sterba
2017-04-05 3:51 ` [PATCH v2] " 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.