Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
On Thu, Jul 30, 2020 at 09:40:32AM -0500, John Donnelly wrote: > > > > On Jul 30, 2020, at 12:21 AM, Greg KH wrote: > > > > On Wed, Jul 29, 2020 at 06:45:46PM -0500, John Donnelly wrote: > >> > >> > >> On 7/29/20 9:16 AM, Mike Snitzer wrote: > >>> On Wed, Jul 29 2020 at 7:55am -0400, > >>> Greg KH wrote: > >>> > On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote: > > On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: > >> This mail needs to be saent to sta...@vger.kernel.org (now cc'd). > >> > >> Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 > > > > Now backported, thanks. > > Nope, it broke the build, I need something that actually works :) > > >>> > >>> OK, I'll defer to John Donnelly to get back with you (and rest of > >>> stable@). He is more invested due to SUSE also having this issue. I > >>> can put focus to it if John cannot sort this out. > >>> > >>> Mike > >>> > >> > >> > >> Hi. > >> > >> > >> Thank you for reaching out. > >> > >> What specifically is broken? . If it that applying > >> 2df3bae9a6543e90042291707b8db0cbfbae9ee9 to 4.14.y is failing? > > > > yes, try it yourself and see! > > Hi . > > Yes . > > 2df3bae9a6543e90042291707b8db0cbfbae9ee9 > > Needs refactored to work in 4.14.y (now .190) as there is a conflict in > arguments as noted in my original submittal ;-) . > I also noticed there are warning to functions " defined but not used > [-Wunused-function] “ too. > > Do you want another PATCH v2 message in a new email thread, or can I > append it to this this thread ? I do not care, either should be fine.
Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
> On Jul 30, 2020, at 12:21 AM, Greg KH wrote: > > On Wed, Jul 29, 2020 at 06:45:46PM -0500, John Donnelly wrote: >> >> >> On 7/29/20 9:16 AM, Mike Snitzer wrote: >>> On Wed, Jul 29 2020 at 7:55am -0400, >>> Greg KH wrote: >>> On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote: > On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: >> This mail needs to be saent to sta...@vger.kernel.org (now cc'd). >> >> Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 > > Now backported, thanks. Nope, it broke the build, I need something that actually works :) >>> >>> OK, I'll defer to John Donnelly to get back with you (and rest of >>> stable@). He is more invested due to SUSE also having this issue. I >>> can put focus to it if John cannot sort this out. >>> >>> Mike >>> >> >> >> Hi. >> >> >> Thank you for reaching out. >> >> What specifically is broken? . If it that applying >> 2df3bae9a6543e90042291707b8db0cbfbae9ee9 to 4.14.y is failing? > > yes, try it yourself and see! Hi . Yes . 2df3bae9a6543e90042291707b8db0cbfbae9ee9 Needs refactored to work in 4.14.y (now .190) as there is a conflict in arguments as noted in my original submittal ;-) . I also noticed there are warning to functions " defined but not used [-Wunused-function] “ too. Do you want another PATCH v2 message in a new email thread, or can I append it to this this thread ? Please advice. Thanks. JD.
Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
On Wed, Jul 29, 2020 at 06:45:46PM -0500, John Donnelly wrote: > > > On 7/29/20 9:16 AM, Mike Snitzer wrote: > > On Wed, Jul 29 2020 at 7:55am -0400, > > Greg KH wrote: > > > > > On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote: > > > > On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: > > > > > This mail needs to be saent to sta...@vger.kernel.org (now cc'd). > > > > > > > > > > Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 > > > > > > > > Now backported, thanks. > > > > > > Nope, it broke the build, I need something that actually works :) > > > > > > > OK, I'll defer to John Donnelly to get back with you (and rest of > > stable@). He is more invested due to SUSE also having this issue. I > > can put focus to it if John cannot sort this out. > > > > Mike > > > > > Hi. > > > Thank you for reaching out. > > What specifically is broken? . If it that applying > 2df3bae9a6543e90042291707b8db0cbfbae9ee9 to 4.14.y is failing? yes, try it yourself and see!
Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
On 7/29/20 9:16 AM, Mike Snitzer wrote: On Wed, Jul 29 2020 at 7:55am -0400, Greg KH wrote: On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote: On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: This mail needs to be saent to sta...@vger.kernel.org (now cc'd). Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 Now backported, thanks. Nope, it broke the build, I need something that actually works :) OK, I'll defer to John Donnelly to get back with you (and rest of stable@). He is more invested due to SUSE also having this issue. I can put focus to it if John cannot sort this out. Mike Hi. Thank you for reaching out. What specifically is broken? . If it that applying 2df3bae9a6543e90042291707b8db0cbfbae9ee9 to 4.14.y is failing? JD.
Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
On Wed, Jul 29 2020 at 7:55am -0400, Greg KH wrote: > On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote: > > On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: > > > This mail needs to be saent to sta...@vger.kernel.org (now cc'd). > > > > > > Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 > > > > Now backported, thanks. > > Nope, it broke the build, I need something that actually works :) > OK, I'll defer to John Donnelly to get back with you (and rest of stable@). He is more invested due to SUSE also having this issue. I can put focus to it if John cannot sort this out. Mike
Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote: > On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: > > This mail needs to be saent to sta...@vger.kernel.org (now cc'd). > > > > Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 > > Now backported, thanks. Nope, it broke the build, I need something that actually works :)
Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: > This mail needs to be saent to sta...@vger.kernel.org (now cc'd). > > Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 Now backported, thanks. greg k-h
Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
> On Jul 27, 2020, at 3:17 PM, Sasha Levin wrote: > > On Mon, Jul 27, 2020 at 02:38:52PM -0500, John Donnelly wrote: >> >> >>> On Jul 27, 2020, at 2:18 PM, Sasha Levin wrote: >>> >>> On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: This mail needs to be saent to sta...@vger.kernel.org (now cc'd). Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 >>> >>> Hm, what's the issue that this patch addresses? It's not clear from the >>> commit message. >>> >>> -- >>> Thanks, >>> Sasha >> >> HI Sasha , >> >> In an off-line conversation I had with Mike , he indicated that : >> >> >> commit 1b17159e52bb31f982f82a6278acd7fab1d3f67b >> Author: Mike Snitzer >> Date: Fri Feb 28 18:00:53 2020 -0500 >> >> dm bio record: save/restore bi_end_io and bi_integrity >> >> >> commit 248aa2645aa7fc9175d1107c2593cc90d4af5a4e >> Author: Mike Snitzer >> Date: Fri Feb 28 18:11:53 2020 -0500 >> >> dm integrity: use dm_bio_record and dm_bio_restore >> >> >> Were picked up in "stable" kernels picked up even though >> neither was marked for sta...@vger.kernel.org >> >> Adding this missing commit : >> >> 2df3bae9a6543e90042291707b8db0cbfbae9ee9 >> >> >> Completes the series > > Should we just revert those two commits instead if they're not needed? > > -- > Thanks, > Sasha As I stated above: > Fixes: 705559706d62038b74c5088114c1799cf2c9dce8 (dm bio record: > save/restore bi_end_io and bi_integrity, version 4.14.188) > > 70555970 introduced a mkfs.ext4 hang on a LVM device that has been > modified with lvconvert --cachemode=writethrough. It corrects an issue we discovered in 4.14.188 .Any other branches those two commits have migrated to will likely have the same regression. I am confident linux-4.14.y will be better off with it ;-)
Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
On Mon, Jul 27 2020 at 4:17pm -0400, Sasha Levin wrote: > On Mon, Jul 27, 2020 at 02:38:52PM -0500, John Donnelly wrote: > > > > > >>On Jul 27, 2020, at 2:18 PM, Sasha Levin wrote: > >> > >>On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: > >>>This mail needs to be saent to sta...@vger.kernel.org (now cc'd). > >>> > >>>Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 > >> > >>Hm, what's the issue that this patch addresses? It's not clear from the > >>commit message. > >> > >>-- > >>Thanks, > >>Sasha > > > >HI Sasha , > > > >In an off-line conversation I had with Mike , he indicated that : > > > > > >commit 1b17159e52bb31f982f82a6278acd7fab1d3f67b > >Author: Mike Snitzer > >Date: Fri Feb 28 18:00:53 2020 -0500 > > > > dm bio record: save/restore bi_end_io and bi_integrity > > > > > >commit 248aa2645aa7fc9175d1107c2593cc90d4af5a4e > >Author: Mike Snitzer > >Date: Fri Feb 28 18:11:53 2020 -0500 > > > > dm integrity: use dm_bio_record and dm_bio_restore > > > > > >Were picked up in "stable" kernels picked up even though > >neither was marked for sta...@vger.kernel.org > > > >Adding this missing commit : > > > >2df3bae9a6543e90042291707b8db0cbfbae9ee9 > > > > > >Completes the series > > Should we just revert those two commits instead if they're not needed? I'd be fine with that, exceept I haven't looked to see whether any other stable commits conflict with reverting them. But you have my blessing to give it a shot ;) Mike
Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
On Mon, Jul 27, 2020 at 02:38:52PM -0500, John Donnelly wrote: On Jul 27, 2020, at 2:18 PM, Sasha Levin wrote: On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: This mail needs to be saent to sta...@vger.kernel.org (now cc'd). Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 Hm, what's the issue that this patch addresses? It's not clear from the commit message. -- Thanks, Sasha HI Sasha , In an off-line conversation I had with Mike , he indicated that : commit 1b17159e52bb31f982f82a6278acd7fab1d3f67b Author: Mike Snitzer Date: Fri Feb 28 18:00:53 2020 -0500 dm bio record: save/restore bi_end_io and bi_integrity commit 248aa2645aa7fc9175d1107c2593cc90d4af5a4e Author: Mike Snitzer Date: Fri Feb 28 18:11:53 2020 -0500 dm integrity: use dm_bio_record and dm_bio_restore Were picked up in "stable" kernels picked up even though neither was marked for sta...@vger.kernel.org Adding this missing commit : 2df3bae9a6543e90042291707b8db0cbfbae9ee9 Completes the series Should we just revert those two commits instead if they're not needed? -- Thanks, Sasha
re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
> On Jul 27, 2020, at 2:18 PM, Sasha Levin wrote: > > On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: >> This mail needs to be saent to sta...@vger.kernel.org (now cc'd). >> >> Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 > > Hm, what's the issue that this patch addresses? It's not clear from the > commit message. > > -- > Thanks, > Sasha HI Sasha , In an off-line conversation I had with Mike , he indicated that : commit 1b17159e52bb31f982f82a6278acd7fab1d3f67b Author: Mike Snitzer Date: Fri Feb 28 18:00:53 2020 -0500 dm bio record: save/restore bi_end_io and bi_integrity commit 248aa2645aa7fc9175d1107c2593cc90d4af5a4e Author: Mike Snitzer Date: Fri Feb 28 18:11:53 2020 -0500 dm integrity: use dm_bio_record and dm_bio_restore Were picked up in "stable" kernels picked up even though neither was marked for sta...@vger.kernel.org Adding this missing commit : 2df3bae9a6543e90042291707b8db0cbfbae9ee9 Completes the series Thank you , John.
Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote: This mail needs to be saent to sta...@vger.kernel.org (now cc'd). Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9 Hm, what's the issue that this patch addresses? It's not clear from the commit message. -- Thanks, Sasha
Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
This mail needs to be saent to sta...@vger.kernel.org (now cc'd).
Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9
Thanks,
Mike
On Mon, Jul 27 2020 at 9:40am -0400,
John Donnelly wrote:
> From: Mike Snitzer
>
> 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
>
> (cherry picked from commit 2df3bae9a6543e90042291707b8db0cbfbae9ee9)
>
> Fixes: 705559706d62038b74c5088114c1799cf2c9dce8 (dm bio record:
> save/restore bi_end_io and bi_integrity, version 4.14.188)
>
> 70555970 introduced a mkfs.ext4 hang on a LVM device that has been
> modified with lvconvert --cachemode=writethrough.
>
> Signed-off-by: John Donnelly
> Tested-by: John Donnelly
> Reviewed-by: Somasundaram Krishnasamy
>
> conflict: drivers/md/dm-cache-target.c - Corrected syntax of
> writethrough_mode(>feature) that was caught by
> arm compiler.
>
> cc: sta...@vger.kernel.org
> cc: snit...@redhat.com
> ---
> drivers/md/dm-cache-target.c | 54
> 1 file changed, 37 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> index 69cdb29ef6be..8241b7c36655 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;
> @@ -868,16 +869,23 @@ static void check_if_tick_bio_needed(struct
> cache *cache, struct bio *bio)
> spin_unlock_irqrestore(>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)
> {
> @@ -971,23 +979,25 @@ static void writethrough_endio(struct bio *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);
> - pb->cache = cache;
> - pb->cblock = cblock;
> - dm_hook_bio(>hook_info, bio, writethrough_endio, NULL);
> - dm_bio_record(>bio_details, bio);
> + BUG_ON(!origin_bio);
> - remap_to_origin_clear_discard(pb->cache, bio, oblock);
> + 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_cache(cache, bio, cblock);
> }
> /*
> @@ -1873,7 +1883,7 @@ static int map_bio(struct cache *cache, struct
> bio *bio, dm_oblock_t block,
> } else {
> if (bio_data_dir(bio) == WRITE && writethrough_mode(>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
>
(resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
From: Mike Snitzer
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
(cherry picked from commit 2df3bae9a6543e90042291707b8db0cbfbae9ee9)
Fixes: 705559706d62038b74c5088114c1799cf2c9dce8 (dm bio record:
save/restore bi_end_io and bi_integrity, version 4.14.188)
70555970 introduced a mkfs.ext4 hang on a LVM device that has been
modified with lvconvert --cachemode=writethrough.
Signed-off-by: John Donnelly
Tested-by: John Donnelly
Reviewed-by: Somasundaram Krishnasamy
conflict: drivers/md/dm-cache-target.c - Corrected syntax of
writethrough_mode(>feature) that was caught by
arm compiler.
cc: sta...@vger.kernel.org
cc: snit...@redhat.com
---
drivers/md/dm-cache-target.c | 54
1 file changed, 37 insertions(+), 17 deletions(-)
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 69cdb29ef6be..8241b7c36655 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;
@@ -868,16 +869,23 @@ static void check_if_tick_bio_needed(struct cache
*cache, struct bio *bio)
spin_unlock_irqrestore(>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)
{
@@ -971,23 +979,25 @@ static void writethrough_endio(struct bio *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);
- pb->cache = cache;
- pb->cblock = cblock;
- dm_hook_bio(>hook_info, bio, writethrough_endio, NULL);
- dm_bio_record(>bio_details, bio);
+ BUG_ON(!origin_bio);
- remap_to_origin_clear_discard(pb->cache, bio, oblock);
+ 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_cache(cache, bio, cblock);
}
/*
@@ -1873,7 +1883,7 @@ static int map_bio(struct cache *cache, struct bio
*bio, dm_oblock_t block,
} else {
if (bio_data_dir(bio) == WRITE && writethrough_mode(>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 +2142,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 +2602,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(>features)) {
+ /*
[PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache
From: Mike Snitzer
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
(cherry picked from commit 2df3bae9a6543e90042291707b8db0cbfbae9ee9)
Fixes: 705559706d62038b74c5088114c1799cf2c9dce8 (dm bio record:
save/restore bi_end_io and bi_integrity, version 4.14.188)
70555970 introduced a mkfs.ext4 hang on a LVM device that has been
modified with lvconvert --cachemode=writethrough.
Signed-off-by: John Donnelly
Tested-by: John Donnelly
Reviewed-by: Somasundaram Krishnasamy
conflict: drivers/md/dm-cache-target.c - Corrected syntax of
writethrough_mode(>feature) that was caught by
arm compiler.
cc: sta...@vger.kernel.org
cc: snit...@redhat.com
---
drivers/md/dm-cache-target.c | 54
1 file changed, 37 insertions(+), 17 deletions(-)
diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 69cdb29ef6be..8241b7c36655 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;
@@ -868,16 +869,23 @@ static void check_if_tick_bio_needed(struct cache
*cache, struct bio *bio)
spin_unlock_irqrestore(>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)
{
@@ -971,23 +979,25 @@ static void writethrough_endio(struct bio *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);
- pb->cache = cache;
- pb->cblock = cblock;
- dm_hook_bio(>hook_info, bio, writethrough_endio, NULL);
- dm_bio_record(>bio_details, bio);
+ BUG_ON(!origin_bio);
- remap_to_origin_clear_discard(pb->cache, bio, oblock);
+ 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_cache(cache, bio, cblock);
}
/*
@@ -1873,7 +1883,7 @@ static int map_bio(struct cache *cache, struct bio
*bio, dm_oblock_t block,
} else {
if (bio_data_dir(bio) == WRITE && writethrough_mode(>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 +2142,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 +2602,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(>features)) {
+ /*
