Hi You are right, thanks for the review. I grepped for other uses of dm_table_put and they seem correct - just these two were bad. Alasdair: push this patch to Linus soon. Another point: it looks somehow scary to free the table before unlocking _hash_lock --- most of the time, _hash_lock is took only on behalf of userspace ioctls (no problem with that), but it is also took in dm_copy_name_and_uuid that can be called from uevent code: dm_table_event -> event_callback -> dm_send_uevents -> takes _hash_lock And dm_table_event is called directly from targets, targets wait for dm_table_event to finish in their destructor --- so calling the destructor with locked _hash_lock seems to be a deadlock possibility --- what do you think? Mikulas On Tue, 6 Jan 2009, Kiyoshi Ueda wrote: > Hi Alasdair, Mikulas, > > Although I'm not sure this is correct, don't we need to convert > the following 2 dm_table_put()s to dm_table_destroy()? > Otherwise, the created table won't be destroyed and memory leak > will happen in such error cases? > > drivers/md/dm-ioctl.c:table_load() > 1060 r = dm_table_create(&t, get_mode(param), param->target_count, md); > 1061 if (r) > 1062 goto out; > 1063 > 1064 r = populate_table(t, param, param_size); > 1065 if (r) { > 1066 dm_table_put(t); > 1067 goto out; > 1068 } > 1069 > 1070 down_write(&_hash_lock); > 1071 hc = dm_get_mdptr(md); > 1072 if (!hc || hc->md != md) { > 1073 DMWARN("device has been removed from the dev hash table."); > 1074 dm_table_put(t); > 1075 up_write(&_hash_lock); > 1076 r = -ENXIO; > 1077 goto out; > 1078 } > > Thanks, > Kiyoshi Ueda > Fix an error introduced in dm-table-rework-reference-counting.patch. When there is failure after table initialization, we need to use dm_table_destroy, not dm_table_put, to free the table. dm_table_put may be used only after dm_table_get. Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx> CC: Kiyoshi Ueda <k-ueda@xxxxxxxxxxxxx> --- drivers/md/dm-ioctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Index: linux-2.6.28-devel/drivers/md/dm-ioctl.c =================================================================== --- linux-2.6.28-devel.orig/drivers/md/dm-ioctl.c 2009-01-06 22:06:01.000000000 +0100 +++ linux-2.6.28-devel/drivers/md/dm-ioctl.c 2009-01-06 22:07:05.000000000 +0100 @@ -1063,7 +1063,7 @@ static int table_load(struct dm_ioctl *p r = populate_table(t, param, param_size); if (r) { - dm_table_put(t); + dm_table_destroy(t); goto out; } @@ -1071,7 +1071,7 @@ static int table_load(struct dm_ioctl *p hc = dm_get_mdptr(md); if (!hc || hc->md != md) { DMWARN("device has been removed from the dev hash table."); - dm_table_put(t); + dm_table_destroy(t); up_write(&_hash_lock); r = -ENXIO; goto out; -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel