[dm-devel] [PATCH 1/3] dm: don't grab target io reference in dm_zone_map_bio

[Damien Le Moal]

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