diff mbox series

[7/7] dm: call dm_zone_endio after the target endio callback for zoned devices

Message ID 20230106083317.93938-8-p.raghav@samsung.com (mailing list archive)
State New, archived
Headers show
Series block zoned cleanups | expand

Commit Message

Pankaj Raghav Jan. 6, 2023, 8:33 a.m. UTC 
dm_zone_endio() updates the bi_sector of orig bio for zoned devices that
uses either native append or append emulation, and it is called before the
endio of the target. But target endio can still update the clone bio
after dm_zone_endio is called, thereby, the orig bio does not contain
the updated information anymore.

Currently, this is not a problem as the targets that support zoned devices
such as dm-zoned, dm-linear, and dm-crypt do not have an endio function,
and even if they do (such as dm-flakey), they don't modify the
bio->bi_iter.bi_sector of the cloned bio that is used to update the
orig_bio's bi_sector in dm_zone_endio function.

Call dm_zone_endio for zoned devices after calling the target's endio
function.

Reviewed-by: Mike Snitzer <snitzer@kernel.org>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 drivers/md/dm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Christoph Hellwig Jan. 10, 2023, 6:58 a.m. UTC | #1 
On Fri, Jan 06, 2023 at 09:33:17AM +0100, Pankaj Raghav wrote:
> dm_zone_endio() updates the bi_sector of orig bio for zoned devices that
> uses either native append or append emulation, and it is called before the
> endio of the target. But target endio can still update the clone bio
> after dm_zone_endio is called, thereby, the orig bio does not contain
> the updated information anymore.
> 
> Currently, this is not a problem as the targets that support zoned devices
> such as dm-zoned, dm-linear, and dm-crypt do not have an endio function,
> and even if they do (such as dm-flakey), they don't modify the
> bio->bi_iter.bi_sector of the cloned bio that is used to update the
> orig_bio's bi_sector in dm_zone_endio function.
> 
> Call dm_zone_endio for zoned devices after calling the target's endio
> function.

This looks sensible, but I fail to see why we need this or how it fits
into the earlier block layer part of the series.
Pankaj Raghav Jan. 10, 2023, 9:07 a.m. UTC | #2 
On 2023-01-10 07:58, Christoph Hellwig wrote:
> On Fri, Jan 06, 2023 at 09:33:17AM +0100, Pankaj Raghav wrote:
>> dm_zone_endio() updates the bi_sector of orig bio for zoned devices that
>> uses either native append or append emulation, and it is called before the
>> endio of the target. But target endio can still update the clone bio
>> after dm_zone_endio is called, thereby, the orig bio does not contain
>> the updated information anymore.
>>
>> Currently, this is not a problem as the targets that support zoned devices
>> such as dm-zoned, dm-linear, and dm-crypt do not have an endio function,
>> and even if they do (such as dm-flakey), they don't modify the
>> bio->bi_iter.bi_sector of the cloned bio that is used to update the
>> orig_bio's bi_sector in dm_zone_endio function.
>>
>> Call dm_zone_endio for zoned devices after calling the target's endio
>> function.
> 
> This looks sensible, but I fail to see why we need this or how it fits
> into the earlier block layer part of the series.
>


I just extracted the cleanup from my old series. Do you think it is better
to send it as a separate patch instead of adding it along in this series?
Christoph Hellwig Jan. 10, 2023, 9:43 a.m. UTC | #3 
On Tue, Jan 10, 2023 at 10:07:39AM +0100, Pankaj Raghav wrote:
> I just extracted the cleanup from my old series. Do you think it is better
> to send it as a separate patch instead of adding it along in this series?

dm and block have different maintainers.  So unless there is a
dependency it's better to split the patches out.
diff mbox series

Patch

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index b424a6ee27ba..fdef74fe8bd1 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1109,10 +1109,6 @@  static void clone_endio(struct bio *bio)
 			disable_write_zeroes(md);
 	}
 
-	if (static_branch_unlikely(&zoned_enabled) &&
-	    unlikely(bdev_is_zoned(bio->bi_bdev)))
-		dm_zone_endio(io, bio);
-
 	if (endio) {
 		int r = endio(ti, bio, &error);
 		switch (r) {
@@ -1141,6 +1137,10 @@  static void clone_endio(struct bio *bio)
 		}
 	}
 
+	if (static_branch_unlikely(&zoned_enabled) &&
+	    unlikely(bdev_is_zoned(bio->bi_bdev)))
+		dm_zone_endio(io, bio);
+
 	if (static_branch_unlikely(&swap_bios_enabled) &&
 	    unlikely(swap_bios_limit(ti, bio)))
 		up(&md->swap_bios_semaphore);