[dm-devel] Re: dm-emc hardware handler patches

[Alasdair G Kergon]

On Tue, Dec 05, 2006 at 03:42:41PM -0500, goggin, edward wrote:
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -627,6 +627,7 @@ static int parse_hw_handler(struct arg_s
>  	struct hw_handler_type *hwht;
>  	unsigned hw_argc;
>  	struct dm_target *ti = m->ti;
> +	struct mapped_device *md;
>  
>  	static struct param _params[] = {
>  		{0, 1024, "invalid number of hardware handler args"},


How about this instead?

--- linux-2.6.19.orig/drivers/md/dm-hw-handler.h	2006-12-06 20:49:24.000000000 +0000
+++ linux-2.6.19/drivers/md/dm-hw-handler.h	2006-12-06 21:18:46.000000000 +0000
@@ -16,6 +16,7 @@
 struct hw_handler_type;
 struct hw_handler {
 	struct hw_handler_type *type;
+	struct mapped_device *md;
 	void *context;
 };
 

Then each hw-handler wouldn't need its own code to store it and
we'd just have:

--- linux-2.6.19.orig/drivers/md/dm-mpath.c	2006-12-06 20:55:37.000000000 +0000
+++ linux-2.6.19/drivers/md/dm-mpath.c	2006-12-06 21:35:09.000000000 +0000
@@ -666,6 +666,9 @@ static int parse_hw_handler(struct arg_s
 		return -EINVAL;
 	}
 
+	m->hw_handler.md = dm_table_get_md(ti->table);
+	dm_put(m->hw_handler.md);
+
 	r = hwht->create(&m->hw_handler, hw_argc - 1, as->argv);
 	if (r) {
 		dm_put_hw_handler(hwht);



> +	/*
> +	 * No need to hold a reference on the mapped device here
> +	 * since one is already held for the duration of the
> +	 * mapped device open.
> +	 */

I think it's stronger than that: a (self-)reference must not be held here,
or how would the reference count ever subsequently drop to zero?

Makes you wonder in what circumstances the dm_get() in dm_table_get_md()
is ever required.

Alasdair
-- 
agk at redhat.com