[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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




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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]