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.
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
