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

[Edward Goggin]

On Wednesday, December 06, 2006 5:06 PM, Alasdair G Kergon wrote

> How about this instead?

Much better idea.  I wish I thought of it!

Is the rest of the patch OK?

Do I need to do anything more with this?

> 
> --- 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?
> 

Agreed, that is the way I initially implemented it and I ran into
the exact problem you describe.

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

Yup, but while it's a stretch, possibly some code path may
want to hold a reference to the mapped device even if the
device was already removed from the hash table.