All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Fix segfault when destroying in-use filters in pvs -a.
@ 2010-03-10 21:43 Milan Broz
  2010-03-11 19:57 ` Takahiro Yasui
  0 siblings, 1 reply; 4+ messages in thread
From: Milan Broz @ 2010-03-10 21:43 UTC (permalink / raw)
  To: lvm-devel

Because pvs -a iterate throught the whole device cache
we should not refresh filters inside iteration.

Fix it by introducing simple reference counter and test
it in refresh_filters() call.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=571963
---
 lib/commands/toolcontext.c      |    5 +++++
 lib/device/dev-cache.c          |    2 ++
 lib/device/dev-cache.h          |    1 +
 lib/filters/filter-composite.c  |    1 +
 lib/filters/filter-md.c         |    1 +
 lib/filters/filter-persistent.c |    1 +
 lib/filters/filter-regex.c      |    1 +
 lib/filters/filter-sysfs.c      |    1 +
 lib/filters/filter.c            |    1 +
 9 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c
index 7aac361..a78efe8 100644
--- a/lib/commands/toolcontext.c
+++ b/lib/commands/toolcontext.c
@@ -1240,6 +1240,11 @@ int refresh_filters(struct cmd_context *cmd)
 {
 	int r, saved_ignore_suspended_devices = ignore_suspended_devices();
 
+	if (cmd->filter && cmd->filter->in_use) {
+		log_debug("Skipping filter refresh currently in use.");
+		return 1;
+	}
+
 	if (cmd->filter) {
 		cmd->filter->destroy(cmd->filter);
 		cmd->filter = NULL;
diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c
index 45c0e8f..933d83a 100644
--- a/lib/device/dev-cache.c
+++ b/lib/device/dev-cache.c
@@ -743,12 +743,14 @@ struct dev_iter *dev_iter_create(struct dev_filter *f, int dev_scan)
 
 	di->current = btree_first(_cache.devices);
 	di->filter = f;
+	di->filter->in_use++;
 
 	return di;
 }
 
 void dev_iter_destroy(struct dev_iter *iter)
 {
+	iter->filter->in_use--;
 	dm_free(iter);
 }
 
diff --git a/lib/device/dev-cache.h b/lib/device/dev-cache.h
index 96bf013..9314a5c 100644
--- a/lib/device/dev-cache.h
+++ b/lib/device/dev-cache.h
@@ -24,6 +24,7 @@
 struct dev_filter {
 	int (*passes_filter) (struct dev_filter * f, struct device * dev);
 	void (*destroy) (struct dev_filter * f);
+	unsigned in_use;
 	void *private;
 };
 
diff --git a/lib/filters/filter-composite.c b/lib/filters/filter-composite.c
index d1606d3..55aedaf 100644
--- a/lib/filters/filter-composite.c
+++ b/lib/filters/filter-composite.c
@@ -70,6 +70,7 @@ struct dev_filter *composite_filter_create(int n, struct dev_filter **filters)
 	cft->passes_filter = _and_p;
 	cft->destroy = _composite_destroy;
 	cft->private = filters_copy;
+	cft->in_use = 0;
 
 	return cft;
 }
diff --git a/lib/filters/filter-md.c b/lib/filters/filter-md.c
index c1ecff7..0e9857d 100644
--- a/lib/filters/filter-md.c
+++ b/lib/filters/filter-md.c
@@ -60,6 +60,7 @@ struct dev_filter *md_filter_create(void)
 	f->passes_filter = _ignore_md;
 	f->destroy = _destroy;
 	f->private = NULL;
+	f->in_use = 0;
 
 	return f;
 }
diff --git a/lib/filters/filter-persistent.c b/lib/filters/filter-persistent.c
index 9243a94..3c7639c 100644
--- a/lib/filters/filter-persistent.c
+++ b/lib/filters/filter-persistent.c
@@ -317,6 +317,7 @@ struct dev_filter *persistent_filter_create(struct dev_filter *real,
 	f->passes_filter = _lookup_p;
 	f->destroy = _persistent_destroy;
 	f->private = pf;
+	f->in_use = 0;
 
 	return f;
 
diff --git a/lib/filters/filter-regex.c b/lib/filters/filter-regex.c
index 1d415a4..7903314 100644
--- a/lib/filters/filter-regex.c
+++ b/lib/filters/filter-regex.c
@@ -207,6 +207,7 @@ struct dev_filter *regex_filter_create(struct config_value *patterns)
 	f->passes_filter = _accept_p;
 	f->destroy = _regex_destroy;
 	f->private = rf;
+	f->in_use = 0;
 	return f;
 
       bad:
diff --git a/lib/filters/filter-sysfs.c b/lib/filters/filter-sysfs.c
index 1220b3a..30e29b3 100644
--- a/lib/filters/filter-sysfs.c
+++ b/lib/filters/filter-sysfs.c
@@ -317,6 +317,7 @@ struct dev_filter *sysfs_filter_create(const char *sysfs_dir)
 	f->passes_filter = _accept_p;
 	f->destroy = _destroy;
 	f->private = ds;
+	f->in_use = 0;
 	return f;
 
  bad:
diff --git a/lib/filters/filter.c b/lib/filters/filter.c
index b33d099..7e3edfe 100644
--- a/lib/filters/filter.c
+++ b/lib/filters/filter.c
@@ -319,6 +319,7 @@ struct dev_filter *lvm_type_filter_create(const char *proc,
 	f->passes_filter = _passes_lvm_type_device_filter;
 	f->destroy = lvm_type_filter_destroy;
 	f->private = NULL;
+	f->in_use = 0;
 
 	if (!_scan_proc_dev(proc, cn)) {
 		dm_free(f);
-- 
1.7.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH] Fix segfault when destroying in-use filters in pvs -a.
  2010-03-10 21:43 [PATCH] Fix segfault when destroying in-use filters in pvs -a Milan Broz
@ 2010-03-11 19:57 ` Takahiro Yasui
  2010-03-12 16:56   ` Takahiro Yasui
  0 siblings, 1 reply; 4+ messages in thread
From: Takahiro Yasui @ 2010-03-11 19:57 UTC (permalink / raw)
  To: lvm-devel

On 03/10/10 16:43, Milan Broz wrote:
> Because pvs -a iterate throught the whole device cache
> we should not refresh filters inside iteration.
> 
> Fix it by introducing simple reference counter and test
> it in refresh_filters() call.
> 
> Fixes https://bugzilla.redhat.com/show_bug.cgi?id=571963

I've tested this patch. By a pvs command with this patch, no segfault
occurred on the same test cases which caused segfault.

Tested-by: Takahiro Yasui <tyasui@redhat.com>

Thanks,
Taka



^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH] Fix segfault when destroying in-use filters in pvs -a.
  2010-03-11 19:57 ` Takahiro Yasui
@ 2010-03-12 16:56   ` Takahiro Yasui
  2010-03-15 10:37     ` Milan Broz
  0 siblings, 1 reply; 4+ messages in thread
From: Takahiro Yasui @ 2010-03-12 16:56 UTC (permalink / raw)
  To: lvm-devel

Milan,

On 03/11/10 14:57, Takahiro Yasui wrote:
> On 03/10/10 16:43, Milan Broz wrote:
>> Because pvs -a iterate throught the whole device cache
>> we should not refresh filters inside iteration.
>>
>> Fix it by introducing simple reference counter and test
>> it in refresh_filters() call.
>>
>> Fixes https://bugzilla.redhat.com/show_bug.cgi?id=571963

This patch worked fine in my test case, but I have one question about
your patch. I assume that you added refresh_filters() because it was
necessary, but this patch prevents filter from being refreshed if it
is in use.

Don't we need to refresh the filter when in_use flag is decremented
and becomes 0 after or in dev_iter_destroy()?

Thanks,
Taka



^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH] Fix segfault when destroying in-use filters in pvs -a.
  2010-03-12 16:56   ` Takahiro Yasui
@ 2010-03-15 10:37     ` Milan Broz
  0 siblings, 0 replies; 4+ messages in thread
From: Milan Broz @ 2010-03-15 10:37 UTC (permalink / raw)
  To: lvm-devel

On 03/12/2010 05:56 PM, Takahiro Yasui wrote:
>>> Fix it by introducing simple reference counter and test
>>> it in refresh_filters() call.
>>>
>>> Fixes https://bugzilla.redhat.com/show_bug.cgi?id=571963
> 
> This patch worked fine in my test case, but I have one question about
> your patch. I assume that you added refresh_filters() because it was
> necessary, but this patch prevents filter from being refreshed if it
> is in use.
> 
> Don't we need to refresh the filter when in_use flag is decremented
> and becomes 0 after or in dev_iter_destroy()?

well, the patch is workaround. It should be correct fot _this_
one place.

The full refresh (including filter refresh) should be done in
scan_vgs_for_pvs() (before the iteration loop) so later refresh
should not be needed.
(So we should fix code to not full scan here in the first place...)

I see several more problems with repeated full refresh now,
so maybe some more intrusive solution will be needed.

Milan



^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-03-15 10:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-10 21:43 [PATCH] Fix segfault when destroying in-use filters in pvs -a Milan Broz
2010-03-11 19:57 ` Takahiro Yasui
2010-03-12 16:56   ` Takahiro Yasui
2010-03-15 10:37     ` Milan Broz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.