mirror of
https://github.com/torvalds/linux.git
synced 2024-11-29 23:51:37 +00:00
dm: fix BLK_STS_DM_REQUEUE handling when dm_io represents split bio
Commit7dd76d1fee
("dm: improve bio splitting and associated IO accounting") removed using cloned bio when dm io splitting is needed. Using bio_trim()+bio_inc_remaining() rather than bio_split()+bio_chain() causes multiple dm_io instances to share the same original bio, and it works fine if IOs are completed successfully. But a regression was caused for the case when BLK_STS_DM_REQUEUE is returned from any one of DM's cloned bios (whose dm_io share the same orig_bio). In this BLK_STS_DM_REQUEUE case only the mapped subset of the original bio for the current exact dm_io needs to be re-submitted. However, since the original bio is shared among all dm_io instances, the ->orig_bio actually only represents the last dm_io instance, so requeue can't work as expected. Also when more than one dm_io is requeued, the same original bio is requeued from all dm_io's completion handler, then race is caused. Fix this issue by still allocating one clone bio for completing io only, then io accounting can rely on ->orig_bio being unmodified. This is needed because the dm_io's sector_offset and sectors members are recorded relative to an unmodified ->orig_bio. In the future, we can go back to using bio_trim()+bio_inc_remaining() for dm's io splitting but then delay needing a bio clone only when handling BLK_STS_DM_REQUEUE, but that approach is a bit complicated (so it needs a development cycle): 1) bio clone needs to be done in task context 2) a block interface for unwinding bio is required Fixes:7dd76d1fee
("dm: improve bio splitting and associated IO accounting") Reported-by: Benjamin Marzinski <bmarzins@redhat.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
This commit is contained in:
parent
78ccef9123
commit
61b6e2e532
@ -272,6 +272,7 @@ struct dm_io {
|
|||||||
atomic_t io_count;
|
atomic_t io_count;
|
||||||
struct mapped_device *md;
|
struct mapped_device *md;
|
||||||
|
|
||||||
|
struct bio *split_bio;
|
||||||
/* The three fields represent mapped part of original bio */
|
/* The three fields represent mapped part of original bio */
|
||||||
struct bio *orig_bio;
|
struct bio *orig_bio;
|
||||||
unsigned int sector_offset; /* offset to end of orig_bio */
|
unsigned int sector_offset; /* offset to end of orig_bio */
|
||||||
|
@ -594,6 +594,7 @@ static struct dm_io *alloc_io(struct mapped_device *md, struct bio *bio)
|
|||||||
atomic_set(&io->io_count, 2);
|
atomic_set(&io->io_count, 2);
|
||||||
this_cpu_inc(*md->pending_io);
|
this_cpu_inc(*md->pending_io);
|
||||||
io->orig_bio = bio;
|
io->orig_bio = bio;
|
||||||
|
io->split_bio = NULL;
|
||||||
io->md = md;
|
io->md = md;
|
||||||
spin_lock_init(&io->lock);
|
spin_lock_init(&io->lock);
|
||||||
io->start_time = jiffies;
|
io->start_time = jiffies;
|
||||||
@ -887,7 +888,7 @@ static void dm_io_complete(struct dm_io *io)
|
|||||||
{
|
{
|
||||||
blk_status_t io_error;
|
blk_status_t io_error;
|
||||||
struct mapped_device *md = io->md;
|
struct mapped_device *md = io->md;
|
||||||
struct bio *bio = io->orig_bio;
|
struct bio *bio = io->split_bio ? io->split_bio : io->orig_bio;
|
||||||
|
|
||||||
if (io->status == BLK_STS_DM_REQUEUE) {
|
if (io->status == BLK_STS_DM_REQUEUE) {
|
||||||
unsigned long flags;
|
unsigned long flags;
|
||||||
@ -1693,9 +1694,11 @@ static void dm_split_and_process_bio(struct mapped_device *md,
|
|||||||
* Remainder must be passed to submit_bio_noacct() so it gets handled
|
* Remainder must be passed to submit_bio_noacct() so it gets handled
|
||||||
* *after* bios already submitted have been completely processed.
|
* *after* bios already submitted have been completely processed.
|
||||||
*/
|
*/
|
||||||
bio_trim(bio, io->sectors, ci.sector_count);
|
WARN_ON_ONCE(!dm_io_flagged(io, DM_IO_WAS_SPLIT));
|
||||||
trace_block_split(bio, bio->bi_iter.bi_sector);
|
io->split_bio = bio_split(bio, io->sectors, GFP_NOIO,
|
||||||
bio_inc_remaining(bio);
|
&md->queue->bio_split);
|
||||||
|
bio_chain(io->split_bio, bio);
|
||||||
|
trace_block_split(io->split_bio, bio->bi_iter.bi_sector);
|
||||||
submit_bio_noacct(bio);
|
submit_bio_noacct(bio);
|
||||||
out:
|
out:
|
||||||
/*
|
/*
|
||||||
|
Loading…
Reference in New Issue
Block a user