[dm-devel] [PATCH] dm-snap-persistent-fix-dtr-cleanup.patch

[Jonathan Brassow]

On Mar 19, 2009, at 5:42 PM, Alasdair G Kergon wrote:

> On Wed, Mar 04, 2009 at 03:33:56PM -0600, Jon Brassow wrote:
>> @@ -481,10 +484,17 @@ static void persistent_dtr(struct dm_exc
>> {
>> 	struct pstore *ps = get_info(store);
>>
>> -	destroy_workqueue(ps->metadata_wq);
>> -	dm_io_client_destroy(ps->io_client);
>> -	vfree(ps->callbacks);
>> +	/* Created in read_header */
>> +	if (ps->io_client)
>> +		dm_io_client_destroy(ps->io_client);
>> 	free_area(ps);
>> +
>> +	/* Allocated in persistent_read_metadata */
>> +	if (ps->callbacks)
>> +		vfree(ps->callbacks);
>> +
>> +	/* Don't need to check these, because they are done in ctr */
>> +	destroy_workqueue(ps->metadata_wq);
>> 	kfree(ps);
>> }
>
> I presume it's safe to reorder those operations as the workqueue is
> guaranteed to be empty?  It needs an inline comment though.
> (Or can we just keep the original, logical, order?)

The patch puts them in the order they /should/ have been in in the  
first place; that is, the reverse order in which they are created.   
The inline comments I put in state where/when those items are created,  
and hence why they need checks before being freed.  Perhaps you are  
suggesting the comments could be better. (?)

  brassow