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. What am I missing here ? > > dm_submit_bio(): > dm_split_and_process_bio > init_clone_info(&ci, md, map, bio) > atomic_set(&io->io_count, 1) > ... > __map_bio() > ... > dm_io_dec_pending() > > The target io can only be completed after the above line returns, > so the extra reference in dm_zone_map_bio() doesn't make any difference, > does it? > > > Thanks, > Ming > -- Damien Le Moal Western Digital Research -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
