Skip to contentSkip to conversation

Remove devices from sysfs cache

PATCH In review Hannes Reinecke 2 participants 6 messages Jul 25, 2007 – Aug 2, 2007 Device mapper

lore
Share & tools
lore

Hannes Reinecke

author
#0 lore raw

PATCH 1/1 Remove devices from sysfs cache

Hi Christophe,

whenever a device is finally removed from a multipath map we should also remove it from the sysfs cache. Otherwise we'll hogging up memory with multipathd. And we might rely on wrong information as the device in the sysfs cache might in fact be a different device if the system decided to assign the same number to a different device or if some values have changed.

Please apply.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

multipathd: remove sysfs devices from cache

Whenever a device is really removed from any multipath map we should also remove it from the cache. Otherwise we'll induce a memory leak.

Signed-off-by Hannes Reinecke <[email protected]>
libmultipath/sysfs.c +19 -0
index d8c65b2..b9621ac 100644
@@ -342,6 +342,25 @@ struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_device
 	return NULL;
 }
 
+void sysfs_device_put(struct sysfs_device *dev)
+{
+	struct sysfs_dev *sysdev_loop;
+
+	list_for_each_entry(sysdev_loop, &sysfs_dev_list, node) {
+		if (&sysdev_loop->dev == dev) {
+			dbg("removed dev '%s' from cache",
+			    sysdev_loop->dev.devpath);
+			list_del(&sysdev_loop->node);
+			free(sysdev_loop);
+			return;
+		}
+	}
+	dbg("dev '%s' not found in cache",
+	    sysdev_loop->dev.devpath);
+
+	return;
+}
+
 char *sysfs_attr_get_value(const char *devpath, const char *attr_name)
 {
 	char path_full[PATH_SIZE];
libmultipath/sysfs.h +1 -0
index 6d83489..e7fa3e7 100644
@@ -18,6 +18,7 @@ void sysfs_device_set_values(struct sysfs_device *dev, const char *devpath,
 struct sysfs_device *sysfs_device_get(const char *devpath);
 struct sysfs_device *sysfs_device_get_parent(struct sysfs_device *dev);
 struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_device *dev, const char *subsystem);
+void sysfs_device_put(struct sysfs_device *dev);
 char *sysfs_attr_get_value(const char *devpath, const char *attr_name);
 int sysfs_resolve_link(char *path, size_t size);
multipathd/main.c +7 -1
index b1620b5..da5fd8f 100644
@@ -442,8 +442,14 @@ out:
 static int
 uev_remove_path (struct sysfs_device * dev, struct vectors * vecs)
 {
+	int retval;
+
 	condlog(2, "%s: remove path (uevent)", dev->kernel);
-	return ev_remove_path(dev->kernel, vecs);
+	retval = ev_remove_path(dev->kernel, vecs);
+	if (!retval)
+		sysfs_device_put(dev);
+
+	return retval;
 }
 
 int
#1 lore raw

New multipath-tools release?

Hi Christophe,

Quoted 3 lines · Hannes Reinecke

Hi Christophe,

whenever a device is finally removed from a multipath map we should also remove it from the sysfs cache. Otherwise we'll hogging up memory with multipathd. And we might rely on wrong information as the device in the sysfs cache might in fact be a different device if the system decided to assign the same number to a different device or if some values have changed.

Now with this fix in, would it make any sense to tag a 0.4.8-rc1? The last multipath-tools release has been about a year ago and it would really be nice to have a new release, so there's a new base to diff against.
Cheers,
-- Guido

author
#2 lore raw

Re: New multipath-tools release?

Quoted 10 lines · Guido Guenther

Hi Christophe,

Quoted 8 lines · Hannes Reinecke

Hi Christophe,

whenever a device is finally removed from a multipath map we should also remove it from the sysfs cache. Otherwise we'll hogging up memory with multipathd. And we might rely on wrong information as the device in the sysfs cache might in fact be a different device if the system decided to assign the same number to a different device or if some values have changed.

Quoted 4 lines

Now with this fix in, would it make any sense to tag a 0.4.8-rc1? The last multipath-tools release has been about a year ago and it would really be nice to have a new release, so there's a new base to diff against.

In priciple a good idea. However, I do have some to-do items on my list yet:

- netlink messages: The FC transport class has a netlink interface nowadays. It may be an idea to utilize this information in to update the path status.
- Enhance path checker. Currently the path checker is run at periodic intervals regardless of the path state. This leads to quite some load on systems with many paths, even when all paths are healthy. We should rather call the path checker only if a path has failed, as we're being notified by a failed path soon enough via I/O errors. And we might even be using the netlink events from FC here, too.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
#3 lore raw

Re: New multipath-tools release?

Quoted 13 lines · Hannes Reinecke

In priciple a good idea. However, I do have some to-do items on my list yet:

- netlink messages: The FC transport class has a netlink interface nowadays. It may be an idea to utilize this information in to update the path status.
- Enhance path checker. Currently the path checker is run at periodic intervals regardless of the path state. This leads to quite some load on systems with many paths, even when all paths are healthy. We should rather call the path checker only if a path has failed, as we're being notified by a failed path soon enough via I/O errors. And we might even be using the netlink events from FC here, too.

My point exactly: let's tag a new stable version and add these nice things to the (then) next release. I think the stability of the master branch improved quiet a bit during the last couple of commits, It'd be nice to have this out of the door for distributions to build upon before doing more intrusive changes again.
Cheers,
-- Guido

author
#4 lore raw

Re: New multipath-tools release?

Quoted 14 lines · Guido Guenther
Quoted 13 lines · Hannes Reinecke

In priciple a good idea. However, I do have some to-do items on my list yet:

- netlink messages: The FC transport class has a netlink interface nowadays. It may be an idea to utilize this information in to update the path status.
- Enhance path checker. Currently the path checker is run at periodic intervals regardless of the path state. This leads to quite some load on systems with many paths, even when all paths are healthy. We should rather call the path checker only if a path has failed, as we're being notified by a failed path soon enough via I/O errors. And we might even be using the netlink events from FC here, too.

Quoted 5 lines

My point exactly: let's tag a new stable version and add these nice things to the (then) next release. I think the stability of the master branch improved quiet a bit during the last couple of commits, It'd be nice to have this out of the door for distributions to build upon before doing more intrusive changes again.

(Ah, so there is an RHEL update coming :-)

But yes, you are right. Putting out a new version before adding those is probably a good thing.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
#5 lore raw

Re: New multipath-tools release?

Quoted 1 line · Hannes Reinecke

(Ah, so there is an RHEL update coming :-)

Hmm..no, the kids called lenny and still has some months before it has to grow up but given that 0.4.7 doesn't even work with current kernels, a release would actually make some sense.
Cheers,
-- Guido