On Mon, Apr 11, 2022 at 09:50:57AM +0900, Damien Le Moal wrote: > On 4/11/22 09:36, Ming Lei wrote: > > On Mon, Apr 11, 2022 at 09:18:56AM +0900, Damien Le Moal wrote: > >> On 4/9/22 02:12, Ming Lei wrote: > >>> dm_zone_map_bio() is only called from __map_bio in which the io's > >>> reference is grabbed already, and the reference won't be released > >>> until the bio is submitted, so no necessary to do it dm_zone_map_bio > >>> any more. > >> > >> I do not think that this patch is correct. Removing the extra reference on > >> the io can lead to problems if the io is completed in the target > >> ->map(ti, clone) call or before dm_zone_map_bio_end() is called for the > >> DM_MAPIO_SUBMITTED or DM_MAPIO_REMAPPED cases. dm_zone_map_bio_end() needs > > > > __map_bio(): > > ... > > dm_io_inc_pending(io); > > ... > > dm_zone_map_bio(tio); > > ... > > dm-crypt (for instance) may terminate the clone bio immediately in its > ->map() function context, resulting in the bio_endio()clone) -> > clone_endio() -> dm_io_dec_pending() call chain. > > With that, the io is gone and dm_zone_map_bio_end() will not have a valid > reference on the orig bio. Any target can complete io during ->map. Here looks nothing is special with dm-crypt or dm-zone, why does only dm zone need extra reference? The reference counter is initialized as 1 in init_clone_info(), dm_io_inc_pending() in __map_bio() increases it to 2, so after the above call chain you mentioned is done, the counter becomes 1. The original bio can't be completed until dm_io_dec_pending() in dm_split_and_process_bio() is called. Or maybe I miss any extra requirement from dm-zone? thanks, Ming -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
