* [PATCH 0/2 v2] Fix fallout from changes to FUA and PREFLUSH definitions
@ 2017-05-31 7:44 Jan Kara
2017-05-31 7:44 ` [PATCH 1/2] dm: Make flush bios explicitely sync Jan Kara
2017-05-31 7:44 ` [PATCH 2/2] md: " Jan Kara
0 siblings, 2 replies; 5+ messages in thread
From: Jan Kara @ 2017-05-31 7:44 UTC (permalink / raw)
To: Shaohua Li; +Cc: Jan Kara, linux-raid, Mike Snitzer, dm-devel
Hello,
here are patches for DM/MD that address a possible performance issue caused by
commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as synchronous".
The core of the problem is that above mentioned commit removed REQ_SYNC flag
from WRITE_{FUA|PREFLUSH|...} definitions. generic_make_request_checks()
however strips REQ_FUA and REQ_PREFLUSH flags from a bio when the storage
doesn't report volatile write cache and thus write effectively becomes
asynchronous which can lead to performance regressions.
DM/MD guys, can you please have a look at patches and take them through your
trees? Thanks!
Honza
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2] dm: Make flush bios explicitely sync 2017-05-31 7:44 [PATCH 0/2 v2] Fix fallout from changes to FUA and PREFLUSH definitions Jan Kara @ 2017-05-31 7:44 ` Jan Kara 2017-05-31 14:53 ` Mike Snitzer 2017-05-31 7:44 ` [PATCH 2/2] md: " Jan Kara 1 sibling, 1 reply; 5+ messages in thread From: Jan Kara @ 2017-05-31 7:44 UTC (permalink / raw) To: Shaohua Li; +Cc: Jan Kara, Mike Snitzer, dm-devel, stable Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...} definitions. generic_make_request_checks() however strips REQ_FUA and REQ_PREFLUSH flags from a bio when the storage doesn't report volatile write cache and thus write effectively becomes asynchronous which can lead to performance regressions Fix the problem by making sure all bios which are synchronous are properly marked with REQ_SYNC. CC: Mike Snitzer <snitzer@redhat.com> CC: dm-devel@redhat.com Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3 CC: stable@vger.kernel.org Signed-off-by: Jan Kara <jack@suse.cz> --- drivers/md/dm-bufio.c | 2 +- drivers/md/dm-integrity.c | 3 ++- drivers/md/dm-raid1.c | 2 +- drivers/md/dm-snap-persistent.c | 3 ++- drivers/md/dm.c | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index cd8139593ccd..840c1496b2b1 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -1334,7 +1334,7 @@ int dm_bufio_issue_flush(struct dm_bufio_client *c) { struct dm_io_request io_req = { .bi_op = REQ_OP_WRITE, - .bi_op_flags = REQ_PREFLUSH, + .bi_op_flags = REQ_PREFLUSH | REQ_SYNC, .mem.type = DM_IO_KMEM, .mem.ptr.addr = NULL, .client = c->dm_io, diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c index c7f7c8d76576..923323e22320 100644 --- a/drivers/md/dm-integrity.c +++ b/drivers/md/dm-integrity.c @@ -783,7 +783,8 @@ static void write_journal(struct dm_integrity_c *ic, unsigned commit_start, unsi for (i = 0; i < commit_sections; i++) rw_section_mac(ic, commit_start + i, true); } - rw_journal(ic, REQ_OP_WRITE, REQ_FUA, commit_start, commit_sections, &io_comp); + rw_journal(ic, REQ_OP_WRITE, REQ_FUA | REQ_SYNC, commit_start, + commit_sections, &io_comp); } else { unsigned to_end; io_comp.in_flight = (atomic_t)ATOMIC_INIT(2); diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c index a95cbb80fb34..e61c45047c25 100644 --- a/drivers/md/dm-raid1.c +++ b/drivers/md/dm-raid1.c @@ -260,7 +260,7 @@ static int mirror_flush(struct dm_target *ti) struct mirror *m; struct dm_io_request io_req = { .bi_op = REQ_OP_WRITE, - .bi_op_flags = REQ_PREFLUSH, + .bi_op_flags = REQ_PREFLUSH | REQ_SYNC, .mem.type = DM_IO_KMEM, .mem.ptr.addr = NULL, .client = ms->io_client, diff --git a/drivers/md/dm-snap-persistent.c b/drivers/md/dm-snap-persistent.c index b93476c3ba3f..c5534d294773 100644 --- a/drivers/md/dm-snap-persistent.c +++ b/drivers/md/dm-snap-persistent.c @@ -741,7 +741,8 @@ static void persistent_commit_exception(struct dm_exception_store *store, /* * Commit exceptions to disk. */ - if (ps->valid && area_io(ps, REQ_OP_WRITE, REQ_PREFLUSH | REQ_FUA)) + if (ps->valid && area_io(ps, REQ_OP_WRITE, + REQ_PREFLUSH | REQ_FUA | REQ_SYNC)) ps->valid = 0; /* diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 6ef9500226c0..37ccd73c79ec 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1657,7 +1657,7 @@ static struct mapped_device *alloc_dev(int minor) bio_init(&md->flush_bio, NULL, 0); md->flush_bio.bi_bdev = md->bdev; - md->flush_bio.bi_opf = REQ_OP_WRITE | REQ_PREFLUSH; + md->flush_bio.bi_opf = REQ_OP_WRITE | REQ_PREFLUSH | REQ_SYNC; dm_stats_init(&md->stats); -- 2.12.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] dm: Make flush bios explicitely sync 2017-05-31 7:44 ` [PATCH 1/2] dm: Make flush bios explicitely sync Jan Kara @ 2017-05-31 14:53 ` Mike Snitzer 0 siblings, 0 replies; 5+ messages in thread From: Mike Snitzer @ 2017-05-31 14:53 UTC (permalink / raw) To: Jan Kara; +Cc: dm-devel, Shaohua Li On Wed, May 31 2017 at 3:44am -0400, Jan Kara <jack@suse.cz> wrote: > Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as > synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...} > definitions. generic_make_request_checks() however strips REQ_FUA and > REQ_PREFLUSH flags from a bio when the storage doesn't report volatile > write cache and thus write effectively becomes asynchronous which can > lead to performance regressions > > Fix the problem by making sure all bios which are synchronous are > properly marked with REQ_SYNC. > > CC: Mike Snitzer <snitzer@redhat.com> > CC: dm-devel@redhat.com > Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3 > CC: stable@vger.kernel.org > Signed-off-by: Jan Kara <jack@suse.cz> thanks, staged for 4.12-rc4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] md: Make flush bios explicitely sync 2017-05-31 7:44 [PATCH 0/2 v2] Fix fallout from changes to FUA and PREFLUSH definitions Jan Kara 2017-05-31 7:44 ` [PATCH 1/2] dm: Make flush bios explicitely sync Jan Kara @ 2017-05-31 7:44 ` Jan Kara 2017-05-31 16:23 ` Shaohua Li 1 sibling, 1 reply; 5+ messages in thread From: Jan Kara @ 2017-05-31 7:44 UTC (permalink / raw) To: Shaohua Li; +Cc: Jan Kara, linux-raid, stable Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...} definitions. generic_make_request_checks() however strips REQ_FUA and REQ_PREFLUSH flags from a bio when the storage doesn't report volatile write cache and thus write effectively becomes asynchronous which can lead to performance regressions Fix the problem by making sure all bios which are synchronous are properly marked with REQ_SYNC. CC: linux-raid@vger.kernel.org CC: Shaohua Li <shli@kernel.org> Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3 CC: stable@vger.kernel.org Signed-off-by: Jan Kara <jack@suse.cz> --- drivers/md/md.c | 2 +- drivers/md/raid5-cache.c | 4 ++-- drivers/md/raid5-ppl.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index 10367ffe92e3..212a6777ff31 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -765,7 +765,7 @@ void md_super_write(struct mddev *mddev, struct md_rdev *rdev, test_bit(FailFast, &rdev->flags) && !test_bit(LastDev, &rdev->flags)) ff = MD_FAILFAST; - bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH | REQ_FUA | ff; + bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH | REQ_FUA | ff; atomic_inc(&mddev->pending_writes); submit_bio(bio); diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c index 4c00bc248287..0a7af8b0a80a 100644 --- a/drivers/md/raid5-cache.c +++ b/drivers/md/raid5-cache.c @@ -1782,7 +1782,7 @@ static int r5l_log_write_empty_meta_block(struct r5l_log *log, sector_t pos, mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum, mb, PAGE_SIZE)); if (!sync_page_io(log->rdev, pos, PAGE_SIZE, page, REQ_OP_WRITE, - REQ_FUA, false)) { + REQ_SYNC | REQ_FUA, false)) { __free_page(page); return -EIO; } @@ -2388,7 +2388,7 @@ r5c_recovery_rewrite_data_only_stripes(struct r5l_log *log, mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum, mb, PAGE_SIZE)); sync_page_io(log->rdev, ctx->pos, PAGE_SIZE, page, - REQ_OP_WRITE, REQ_FUA, false); + REQ_OP_WRITE, REQ_SYNC | REQ_FUA, false); sh->log_start = ctx->pos; list_add_tail(&sh->r5c, &log->stripe_in_journal_list); atomic_inc(&log->stripe_in_journal_count); diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c index 5d25bebf3328..ccce92e68d7f 100644 --- a/drivers/md/raid5-ppl.c +++ b/drivers/md/raid5-ppl.c @@ -907,8 +907,8 @@ static int ppl_write_empty_header(struct ppl_log *log) pplhdr->checksum = cpu_to_le32(~crc32c_le(~0, pplhdr, PAGE_SIZE)); if (!sync_page_io(rdev, rdev->ppl.sector - rdev->data_offset, - PPL_HEADER_SIZE, page, REQ_OP_WRITE | REQ_FUA, 0, - false)) { + PPL_HEADER_SIZE, page, REQ_OP_WRITE | REQ_SYNC | + REQ_FUA, 0, false)) { md_error(rdev->mddev, rdev); ret = -EIO; } -- 2.12.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] md: Make flush bios explicitely sync 2017-05-31 7:44 ` [PATCH 2/2] md: " Jan Kara @ 2017-05-31 16:23 ` Shaohua Li 0 siblings, 0 replies; 5+ messages in thread From: Shaohua Li @ 2017-05-31 16:23 UTC (permalink / raw) To: Jan Kara; +Cc: linux-raid, stable On Wed, May 31, 2017 at 09:44:33AM +0200, Jan Kara wrote: > Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as > synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...} > definitions. generic_make_request_checks() however strips REQ_FUA and > REQ_PREFLUSH flags from a bio when the storage doesn't report volatile > write cache and thus write effectively becomes asynchronous which can > lead to performance regressions > > Fix the problem by making sure all bios which are synchronous are > properly marked with REQ_SYNC. Applied, thanks! > CC: linux-raid@vger.kernel.org > CC: Shaohua Li <shli@kernel.org> > Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3 > CC: stable@vger.kernel.org > Signed-off-by: Jan Kara <jack@suse.cz> > --- > drivers/md/md.c | 2 +- > drivers/md/raid5-cache.c | 4 ++-- > drivers/md/raid5-ppl.c | 4 ++-- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 10367ffe92e3..212a6777ff31 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -765,7 +765,7 @@ void md_super_write(struct mddev *mddev, struct md_rdev *rdev, > test_bit(FailFast, &rdev->flags) && > !test_bit(LastDev, &rdev->flags)) > ff = MD_FAILFAST; > - bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH | REQ_FUA | ff; > + bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH | REQ_FUA | ff; > > atomic_inc(&mddev->pending_writes); > submit_bio(bio); > diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c > index 4c00bc248287..0a7af8b0a80a 100644 > --- a/drivers/md/raid5-cache.c > +++ b/drivers/md/raid5-cache.c > @@ -1782,7 +1782,7 @@ static int r5l_log_write_empty_meta_block(struct r5l_log *log, sector_t pos, > mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum, > mb, PAGE_SIZE)); > if (!sync_page_io(log->rdev, pos, PAGE_SIZE, page, REQ_OP_WRITE, > - REQ_FUA, false)) { > + REQ_SYNC | REQ_FUA, false)) { > __free_page(page); > return -EIO; > } > @@ -2388,7 +2388,7 @@ r5c_recovery_rewrite_data_only_stripes(struct r5l_log *log, > mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum, > mb, PAGE_SIZE)); > sync_page_io(log->rdev, ctx->pos, PAGE_SIZE, page, > - REQ_OP_WRITE, REQ_FUA, false); > + REQ_OP_WRITE, REQ_SYNC | REQ_FUA, false); > sh->log_start = ctx->pos; > list_add_tail(&sh->r5c, &log->stripe_in_journal_list); > atomic_inc(&log->stripe_in_journal_count); > diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c > index 5d25bebf3328..ccce92e68d7f 100644 > --- a/drivers/md/raid5-ppl.c > +++ b/drivers/md/raid5-ppl.c > @@ -907,8 +907,8 @@ static int ppl_write_empty_header(struct ppl_log *log) > pplhdr->checksum = cpu_to_le32(~crc32c_le(~0, pplhdr, PAGE_SIZE)); > > if (!sync_page_io(rdev, rdev->ppl.sector - rdev->data_offset, > - PPL_HEADER_SIZE, page, REQ_OP_WRITE | REQ_FUA, 0, > - false)) { > + PPL_HEADER_SIZE, page, REQ_OP_WRITE | REQ_SYNC | > + REQ_FUA, 0, false)) { > md_error(rdev->mddev, rdev); > ret = -EIO; > } > -- > 2.12.3 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-05-31 16:23 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-31 7:44 [PATCH 0/2 v2] Fix fallout from changes to FUA and PREFLUSH definitions Jan Kara 2017-05-31 7:44 ` [PATCH 1/2] dm: Make flush bios explicitely sync Jan Kara 2017-05-31 14:53 ` Mike Snitzer 2017-05-31 7:44 ` [PATCH 2/2] md: " Jan Kara 2017-05-31 16:23 ` Shaohua Li
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.