[(resend) PATCH v3: {linux-4.14.y} ] dm cache: submit writethrough writes in parallel to origin and cache
From: Mike Snitzer snitzer@redhat.com
Discontinue issuing writethrough write IO in series to the origin and then cache.
Use bio_clone_fast() to create a new origin clone bio that will be mapped to the origin device and then bio_chain() it to the bio that gets remapped to the cache device. The origin clone bio does _not_ have a copy of the per_bio_data -- as such check_if_tick_bio_needed() will not be called.
The cache bio (parent bio) will not complete until the origin bio has completed -- this fulfills bio_clone_fast()'s requirements as well as the requirement to not complete the original IO until the write IO has completed to both the origin and cache device.
Signed-off-by: Mike Snitzer snitzer@redhat.com
(cherry picked from commit 2df3bae9a6543e90042291707b8db0cbfbae9ee9)
Fixes: 4ec34f2196d125ff781170ddc6c3058c08ec5e73 (dm bio record: save/restore bi_end_io and bi_integrity )
4ec34f21 introduced a mkfs.ext4 hang on a LVM device that has been modified with lvconvert --cachemode=writethrough.
CC:stable@vger.kernel.org for 4.14.y
Signed-off-by: John Donnelly john.p.donnelly@oracle.com Reviewed-by: Somasundaram Krishnasamy somasundaram.krishnasamy@oracle.com
conflicts: drivers/md/dm-cache-target.c. - Corrected usage of writethrough_mode(&cache->feature) that was caught by compiler, and removed unused static functions : writethrough_endio(), defer_writethrough_bio(), wake_deferred_writethrough_worker() that generated warnings. --- drivers/md/dm-cache-target.c | 92 ++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 55 deletions(-)
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c index 69cdb29ef6be..2732d1df05fa 100644 --- a/drivers/md/dm-cache-target.c +++ b/drivers/md/dm-cache-target.c @@ -450,6 +450,7 @@ struct cache { struct work_struct migration_worker; struct delayed_work waker; struct dm_bio_prison_v2 *prison; + struct bio_set *bs;
mempool_t *migration_pool;
@@ -537,11 +538,6 @@ static void wake_deferred_bio_worker(struct cache *cache) queue_work(cache->wq, &cache->deferred_bio_worker); }
-static void wake_deferred_writethrough_worker(struct cache *cache) -{ - queue_work(cache->wq, &cache->deferred_writethrough_worker); -} - static void wake_migration_worker(struct cache *cache) { if (passthrough_mode(&cache->features)) @@ -868,16 +864,23 @@ static void check_if_tick_bio_needed(struct cache *cache, struct bio *bio) spin_unlock_irqrestore(&cache->lock, flags); }
-static void remap_to_origin_clear_discard(struct cache *cache, struct bio *bio, - dm_oblock_t oblock) +static void __remap_to_origin_clear_discard(struct cache *cache, struct bio *bio, + dm_oblock_t oblock, bool bio_has_pbd) { - // FIXME: this is called way too much. - check_if_tick_bio_needed(cache, bio); + if (bio_has_pbd) + check_if_tick_bio_needed(cache, bio); remap_to_origin(cache, bio); if (bio_data_dir(bio) == WRITE) clear_discard(cache, oblock_to_dblock(cache, oblock)); }
+static void remap_to_origin_clear_discard(struct cache *cache, struct bio *bio, + dm_oblock_t oblock) +{ + // FIXME: check_if_tick_bio_needed() is called way too much through this interface + __remap_to_origin_clear_discard(cache, bio, oblock, true); +} + static void remap_to_cache_dirty(struct cache *cache, struct bio *bio, dm_oblock_t oblock, dm_cblock_t cblock) { @@ -937,57 +940,26 @@ static void issue_op(struct bio *bio, void *context) accounted_request(cache, bio); }
-static void defer_writethrough_bio(struct cache *cache, struct bio *bio) -{ - unsigned long flags; - - spin_lock_irqsave(&cache->lock, flags); - bio_list_add(&cache->deferred_writethrough_bios, bio); - spin_unlock_irqrestore(&cache->lock, flags); - - wake_deferred_writethrough_worker(cache); -} - -static void writethrough_endio(struct bio *bio) -{ - struct per_bio_data *pb = get_per_bio_data(bio, PB_DATA_SIZE_WT); - - dm_unhook_bio(&pb->hook_info, bio); - - if (bio->bi_status) { - bio_endio(bio); - return; - } - - dm_bio_restore(&pb->bio_details, bio); - remap_to_cache(pb->cache, bio, pb->cblock); - - /* - * We can't issue this bio directly, since we're in interrupt - * context. So it gets put on a bio list for processing by the - * worker thread. - */ - defer_writethrough_bio(pb->cache, bio); -} - /* - * FIXME: send in parallel, huge latency as is. * When running in writethrough mode we need to send writes to clean blocks - * to both the cache and origin devices. In future we'd like to clone the - * bio and send them in parallel, but for now we're doing them in - * series as this is easier. + * to both the cache and origin devices. Clone the bio and send them in parallel. */ -static void remap_to_origin_then_cache(struct cache *cache, struct bio *bio, - dm_oblock_t oblock, dm_cblock_t cblock) +static void remap_to_origin_and_cache(struct cache *cache, struct bio *bio, + dm_oblock_t oblock, dm_cblock_t cblock) { - struct per_bio_data *pb = get_per_bio_data(bio, PB_DATA_SIZE_WT); + struct bio *origin_bio = bio_clone_fast(bio, GFP_NOIO, cache->bs); + + BUG_ON(!origin_bio);
- pb->cache = cache; - pb->cblock = cblock; - dm_hook_bio(&pb->hook_info, bio, writethrough_endio, NULL); - dm_bio_record(&pb->bio_details, bio); + bio_chain(origin_bio, bio); + /* + * Passing false to __remap_to_origin_clear_discard() skips + * all code that might use per_bio_data (since clone doesn't have it) + */ + __remap_to_origin_clear_discard(cache, origin_bio, oblock, false); + submit_bio(origin_bio);
- remap_to_origin_clear_discard(pb->cache, bio, oblock); + remap_to_cache(cache, bio, cblock); }
/*---------------------------------------------------------------- @@ -1873,7 +1845,7 @@ static int map_bio(struct cache *cache, struct bio *bio, dm_oblock_t block, } else { if (bio_data_dir(bio) == WRITE && writethrough_mode(&cache->features) && !is_dirty(cache, cblock)) { - remap_to_origin_then_cache(cache, bio, block, cblock); + remap_to_origin_and_cache(cache, bio, block, cblock); accounted_begin(cache, bio); } else remap_to_cache_dirty(cache, bio, block, cblock); @@ -2132,6 +2104,9 @@ static void destroy(struct cache *cache) kfree(cache->ctr_args[i]); kfree(cache->ctr_args);
+ if (cache->bs) + bioset_free(cache->bs); + kfree(cache); }
@@ -2589,6 +2564,13 @@ static int cache_create(struct cache_args *ca, struct cache **result) cache->features = ca->features; ti->per_io_data_size = get_per_bio_data_size(cache);
+ if (writethrough_mode(&cache->features)) { + /* Create bioset for writethrough bios issued to origin */ + cache->bs = bioset_create(BIO_POOL_SIZE, 0, 0); + if (!cache->bs) + goto bad; + } + cache->callbacks.congested_fn = cache_is_congested; dm_table_add_target_callbacks(ti->table, &cache->callbacks);
On Tue, Aug 04, 2020 at 07:33:05AM -0500, John Donnelly wrote:
From: Mike Snitzer snitzer@redhat.com
Discontinue issuing writethrough write IO in series to the origin and then cache.
Use bio_clone_fast() to create a new origin clone bio that will be mapped to the origin device and then bio_chain() it to the bio that gets remapped to the cache device. The origin clone bio does _not_ have a copy of the per_bio_data -- as such check_if_tick_bio_needed() will not be called.
The cache bio (parent bio) will not complete until the origin bio has completed -- this fulfills bio_clone_fast()'s requirements as well as the requirement to not complete the original IO until the write IO has completed to both the origin and cache device.
Signed-off-by: Mike Snitzer snitzer@redhat.com
(cherry picked from commit 2df3bae9a6543e90042291707b8db0cbfbae9ee9)
Fixes: 4ec34f2196d125ff781170ddc6c3058c08ec5e73 (dm bio record: save/restore bi_end_io and bi_integrity )
4ec34f21 introduced a mkfs.ext4 hang on a LVM device that has been modified with lvconvert --cachemode=writethrough.
CC:stable@vger.kernel.org for 4.14.y
Signed-off-by: John Donnelly john.p.donnelly@oracle.com Reviewed-by: Somasundaram Krishnasamy somasundaram.krishnasamy@oracle.com
conflicts: drivers/md/dm-cache-target.c. - Corrected usage of writethrough_mode(&cache->feature) that was caught by compiler, and removed unused static functions : writethrough_endio(), defer_writethrough_bio(), wake_deferred_writethrough_worker() that generated warnings.
What is this "conflicts nonsense"? You don't see that in any other kernel patch changelog, do you?
drivers/md/dm-cache-target.c | 92 ++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 55 deletions(-)
Please fix your email client up, it's totally broken and this does not work at all and is getting frustrating from my side here.
Try sending emails to yourself and see if you can apply the patches, as the one you sent here does not work, again:
------------ ~/linux/stable/linux-4.14.y $ b4 am https://lore.kernel.org/r/8CFF8DA9-C105-461C-8F5A-DA2BF448A135@oracle.com Looking up https://lore.kernel.org/r/8CFF8DA9-C105-461C-8F5A-DA2BF448A135%40oracle.com Grabbing thread from lore.kernel.org/lkml Analyzing 1 messages in the thread --- Writing ./20200804_john_p_donnelly_dm_cache_submit_writethrough_writes_in_parallel_to_origin_and_cache.mbx [(resend) PATCH v3: {linux-4.14.y} ] dm cache: submit writethrough writes in parallel to origin and cache --- Total patches: 1 --- Link: https://lore.kernel.org/r/8CFF8DA9-C105-461C-8F5A-DA2BF448A135@oracle.com Base: not found (applies clean to current tree) git am ./20200804_john_p_donnelly_dm_cache_submit_writethrough_writes_in_parallel_to_origin_and_cache.mbx ~/linux/stable/linux-4.14.y $ patch -p1 < 20200804_john_p_donnelly_dm_cache_submit_writethrough_writes_in_parallel_to_origin_and_cache.mbx checking file drivers/md/dm-cache-target.c patch: **** malformed patch at line 70: }
------------
{sigh}
You can do better.
greg k-h
On Tue, Aug 04 2020 at 8:47am -0400, Greg KH gregkh@linuxfoundation.org wrote:
On Tue, Aug 04, 2020 at 07:33:05AM -0500, John Donnelly wrote:
From: Mike Snitzer snitzer@redhat.com
Discontinue issuing writethrough write IO in series to the origin and then cache.
Use bio_clone_fast() to create a new origin clone bio that will be mapped to the origin device and then bio_chain() it to the bio that gets remapped to the cache device. The origin clone bio does _not_ have a copy of the per_bio_data -- as such check_if_tick_bio_needed() will not be called.
The cache bio (parent bio) will not complete until the origin bio has completed -- this fulfills bio_clone_fast()'s requirements as well as the requirement to not complete the original IO until the write IO has completed to both the origin and cache device.
Signed-off-by: Mike Snitzer snitzer@redhat.com
(cherry picked from commit 2df3bae9a6543e90042291707b8db0cbfbae9ee9)
Fixes: 4ec34f2196d125ff781170ddc6c3058c08ec5e73 (dm bio record: save/restore bi_end_io and bi_integrity )
4ec34f21 introduced a mkfs.ext4 hang on a LVM device that has been modified with lvconvert --cachemode=writethrough.
CC:stable@vger.kernel.org for 4.14.y
Signed-off-by: John Donnelly john.p.donnelly@oracle.com Reviewed-by: Somasundaram Krishnasamy somasundaram.krishnasamy@oracle.com
conflicts: drivers/md/dm-cache-target.c. - Corrected usage of writethrough_mode(&cache->feature) that was caught by compiler, and removed unused static functions : writethrough_endio(), defer_writethrough_bio(), wake_deferred_writethrough_worker() that generated warnings.
What is this "conflicts nonsense"? You don't see that in any other kernel patch changelog, do you?
drivers/md/dm-cache-target.c | 92 ++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 55 deletions(-)
Please fix your email client up, it's totally broken and this does not work at all and is getting frustrating from my side here.
Try sending emails to yourself and see if you can apply the patches, as the one you sent here does not work, again:
John's inability to submit a patch that can apply aside: I do not like how this patch header is constructed (yet attributed "From" me). It is devoid of detail as it relates to stable@.
Greg, please don't apply the v4 of this patch either. I'll craft a proper stable@ patch that explains the reason for change and why we're left having to resolve conflicts in stable@.
But first I need to focus on sending DM changes to Linus for v5.9 merge.
Thanks, Mike
On Tue, Aug 04, 2020 at 02:20:38PM -0400, Mike Snitzer wrote:
On Tue, Aug 04 2020 at 8:47am -0400, Greg KH gregkh@linuxfoundation.org wrote:
On Tue, Aug 04, 2020 at 07:33:05AM -0500, John Donnelly wrote:
From: Mike Snitzer snitzer@redhat.com
Discontinue issuing writethrough write IO in series to the origin and then cache.
Use bio_clone_fast() to create a new origin clone bio that will be mapped to the origin device and then bio_chain() it to the bio that gets remapped to the cache device. The origin clone bio does _not_ have a copy of the per_bio_data -- as such check_if_tick_bio_needed() will not be called.
The cache bio (parent bio) will not complete until the origin bio has completed -- this fulfills bio_clone_fast()'s requirements as well as the requirement to not complete the original IO until the write IO has completed to both the origin and cache device.
Signed-off-by: Mike Snitzer snitzer@redhat.com
(cherry picked from commit 2df3bae9a6543e90042291707b8db0cbfbae9ee9)
Fixes: 4ec34f2196d125ff781170ddc6c3058c08ec5e73 (dm bio record: save/restore bi_end_io and bi_integrity )
4ec34f21 introduced a mkfs.ext4 hang on a LVM device that has been modified with lvconvert --cachemode=writethrough.
CC:stable@vger.kernel.org for 4.14.y
Signed-off-by: John Donnelly john.p.donnelly@oracle.com Reviewed-by: Somasundaram Krishnasamy somasundaram.krishnasamy@oracle.com
conflicts: drivers/md/dm-cache-target.c. - Corrected usage of writethrough_mode(&cache->feature) that was caught by compiler, and removed unused static functions : writethrough_endio(), defer_writethrough_bio(), wake_deferred_writethrough_worker() that generated warnings.
What is this "conflicts nonsense"? You don't see that in any other kernel patch changelog, do you?
drivers/md/dm-cache-target.c | 92 ++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 55 deletions(-)
Please fix your email client up, it's totally broken and this does not work at all and is getting frustrating from my side here.
Try sending emails to yourself and see if you can apply the patches, as the one you sent here does not work, again:
John's inability to submit a patch that can apply aside: I do not like how this patch header is constructed (yet attributed "From" me). It is devoid of detail as it relates to stable@.
Greg, please don't apply the v4 of this patch either. I'll craft a proper stable@ patch that explains the reason for change and why we're left having to resolve conflicts in stable@.
But first I need to focus on sending DM changes to Linus for v5.9 merge.
Ok, no worries, I'll drop all of these from my review queue and wait for something from you sometime in the future.
thanks,
greg k-h
On Wed, Aug 05 2020 at 10:32am -0400, Greg KH gregkh@linuxfoundation.org wrote:
On Tue, Aug 04, 2020 at 02:20:38PM -0400, Mike Snitzer wrote:
On Tue, Aug 04 2020 at 8:47am -0400, Greg KH gregkh@linuxfoundation.org wrote:
On Tue, Aug 04, 2020 at 07:33:05AM -0500, John Donnelly wrote:
From: Mike Snitzer snitzer@redhat.com
Discontinue issuing writethrough write IO in series to the origin and then cache.
Use bio_clone_fast() to create a new origin clone bio that will be mapped to the origin device and then bio_chain() it to the bio that gets remapped to the cache device. The origin clone bio does _not_ have a copy of the per_bio_data -- as such check_if_tick_bio_needed() will not be called.
The cache bio (parent bio) will not complete until the origin bio has completed -- this fulfills bio_clone_fast()'s requirements as well as the requirement to not complete the original IO until the write IO has completed to both the origin and cache device.
Signed-off-by: Mike Snitzer snitzer@redhat.com
(cherry picked from commit 2df3bae9a6543e90042291707b8db0cbfbae9ee9)
Fixes: 4ec34f2196d125ff781170ddc6c3058c08ec5e73 (dm bio record: save/restore bi_end_io and bi_integrity )
4ec34f21 introduced a mkfs.ext4 hang on a LVM device that has been modified with lvconvert --cachemode=writethrough.
CC:stable@vger.kernel.org for 4.14.y
Signed-off-by: John Donnelly john.p.donnelly@oracle.com Reviewed-by: Somasundaram Krishnasamy somasundaram.krishnasamy@oracle.com
conflicts: drivers/md/dm-cache-target.c. - Corrected usage of writethrough_mode(&cache->feature) that was caught by compiler, and removed unused static functions : writethrough_endio(), defer_writethrough_bio(), wake_deferred_writethrough_worker() that generated warnings.
What is this "conflicts nonsense"? You don't see that in any other kernel patch changelog, do you?
drivers/md/dm-cache-target.c | 92 ++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 55 deletions(-)
Please fix your email client up, it's totally broken and this does not work at all and is getting frustrating from my side here.
Try sending emails to yourself and see if you can apply the patches, as the one you sent here does not work, again:
John's inability to submit a patch that can apply aside: I do not like how this patch header is constructed (yet attributed "From" me). It is devoid of detail as it relates to stable@.
Greg, please don't apply the v4 of this patch either. I'll craft a proper stable@ patch that explains the reason for change and why we're left having to resolve conflicts in stable@.
But first I need to focus on sending DM changes to Linus for v5.9 merge.
Ok, no worries, I'll drop all of these from my review queue and wait for something from you sometime in the future.
Hey Greg,
SO I've looked this required 4.14 stable@ backport over. Because 4.14 already has these commits (to fix a dm integrity issue): 1b17159e52b dm bio record: save/restore bi_end_io and bi_integrity 248aa2645aa dm integrity: use dm_bio_record and dm_bio_restore
DM-cache's 4.14 writethrough mode got broken because its implementation (ab)used dm_hook_bio+dm_bio_record and predates 4.15's switch to using bio_chain() via commit 2df3bae9a654. Without commit 2df3bae9a654 the dm_hook_bio+dm_bio_record changes from commit 1b17159e52b break dm-cache's writethrough support.
So 4.14-stable now needs these 3 upstream 4.15 commits: 8e3c3827776f dm cache: pass cache structure to mode functions 2df3bae9a654 dm cache: submit writethrough writes in parallel to origin and cache 9958f1d9a04e dm cache: remove all obsolete writethrough-specific code
Applying those commits to v4.14.190 with: git cherry-pick -x 8e3c3827776f^..9958f1d9a04e
results in a kernel that successfully builds and should fix 4.14-stable's broken dm-cache writethrough support.
Are you ok with queueing up applying these 3 upstream commits to 4.14-stable or do you need me to send a patchset?
Thanks, Mike
On Wed, Aug 05, 2020 at 03:34:09PM -0400, Mike Snitzer wrote:
On Wed, Aug 05 2020 at 10:32am -0400, Greg KH gregkh@linuxfoundation.org wrote:
On Tue, Aug 04, 2020 at 02:20:38PM -0400, Mike Snitzer wrote:
On Tue, Aug 04 2020 at 8:47am -0400, Greg KH gregkh@linuxfoundation.org wrote:
On Tue, Aug 04, 2020 at 07:33:05AM -0500, John Donnelly wrote:
From: Mike Snitzer snitzer@redhat.com
Discontinue issuing writethrough write IO in series to the origin and then cache.
Use bio_clone_fast() to create a new origin clone bio that will be mapped to the origin device and then bio_chain() it to the bio that gets remapped to the cache device. The origin clone bio does _not_ have a copy of the per_bio_data -- as such check_if_tick_bio_needed() will not be called.
The cache bio (parent bio) will not complete until the origin bio has completed -- this fulfills bio_clone_fast()'s requirements as well as the requirement to not complete the original IO until the write IO has completed to both the origin and cache device.
Signed-off-by: Mike Snitzer snitzer@redhat.com
(cherry picked from commit 2df3bae9a6543e90042291707b8db0cbfbae9ee9)
Fixes: 4ec34f2196d125ff781170ddc6c3058c08ec5e73 (dm bio record: save/restore bi_end_io and bi_integrity )
4ec34f21 introduced a mkfs.ext4 hang on a LVM device that has been modified with lvconvert --cachemode=writethrough.
CC:stable@vger.kernel.org for 4.14.y
Signed-off-by: John Donnelly john.p.donnelly@oracle.com Reviewed-by: Somasundaram Krishnasamy somasundaram.krishnasamy@oracle.com
conflicts: drivers/md/dm-cache-target.c. - Corrected usage of writethrough_mode(&cache->feature) that was caught by compiler, and removed unused static functions : writethrough_endio(), defer_writethrough_bio(), wake_deferred_writethrough_worker() that generated warnings.
What is this "conflicts nonsense"? You don't see that in any other kernel patch changelog, do you?
drivers/md/dm-cache-target.c | 92 ++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 55 deletions(-)
Please fix your email client up, it's totally broken and this does not work at all and is getting frustrating from my side here.
Try sending emails to yourself and see if you can apply the patches, as the one you sent here does not work, again:
John's inability to submit a patch that can apply aside: I do not like how this patch header is constructed (yet attributed "From" me). It is devoid of detail as it relates to stable@.
Greg, please don't apply the v4 of this patch either. I'll craft a proper stable@ patch that explains the reason for change and why we're left having to resolve conflicts in stable@.
But first I need to focus on sending DM changes to Linus for v5.9 merge.
Ok, no worries, I'll drop all of these from my review queue and wait for something from you sometime in the future.
Hey Greg,
SO I've looked this required 4.14 stable@ backport over. Because 4.14 already has these commits (to fix a dm integrity issue): 1b17159e52b dm bio record: save/restore bi_end_io and bi_integrity 248aa2645aa dm integrity: use dm_bio_record and dm_bio_restore
DM-cache's 4.14 writethrough mode got broken because its implementation (ab)used dm_hook_bio+dm_bio_record and predates 4.15's switch to using bio_chain() via commit 2df3bae9a654. Without commit 2df3bae9a654 the dm_hook_bio+dm_bio_record changes from commit 1b17159e52b break dm-cache's writethrough support.
So 4.14-stable now needs these 3 upstream 4.15 commits: 8e3c3827776f dm cache: pass cache structure to mode functions 2df3bae9a654 dm cache: submit writethrough writes in parallel to origin and cache 9958f1d9a04e dm cache: remove all obsolete writethrough-specific code
Applying those commits to v4.14.190 with: git cherry-pick -x 8e3c3827776f^..9958f1d9a04e
results in a kernel that successfully builds and should fix 4.14-stable's broken dm-cache writethrough support.
Are you ok with queueing up applying these 3 upstream commits to 4.14-stable or do you need me to send a patchset?
This is good enough, now all queued up, thanks!
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
John Donnelly -
Mike Snitzer