avoid scanning suspended & otherwise inaccessible devices

Hi,

this is the second patch from the aforementioned pair debugging session (of Milan and myself) and fixes another class of bugs (including repair deadlocks). It turned out that the persistent device filter takes precedence over everything else, including the ignore_suspended_devices()-related checks. This is clearly wrong and a reasonable solution is to just not trust the cache at all when ignore_suspended_devices() is in effect.

I believe this may address at least RHBZ 606931 and possibly other BZs, and a deadlock we have reproduced locally on our bar cluster (which is not in bugzilla, as far as I know). The bug could certainly cause a lot of grief...

Yours,
Petr.

index 6574d69..c9f709b 100644
@@ -266,7 +266,7 @@ static int _lookup_p(struct dev_filter *f, struct device *dev)
 	void *l = dm_hash_lookup(pf->devices, dev_name(dev));
 	struct str_list *sl;
 
-	if (!l) {
+	if (!l || ignore_suspended_devices()) {
 		l = pf->real->passes_filter(pf->real, dev) ?
 		    PF_GOOD_DEVICE : PF_BAD_DEVICE;
 

Hi again,

I have noticed that Mikul?? ran into a similar problem where real filters are being skipped due to the persistent one. I am wondering what is the actual performance improvement from the persistent filter?

Let's look at the individual cases:

- sysfs&proc: persistent only helps if it prevents *any* fall-through to real filters... this does not seem to be happening in practice; i.e. no benefit for sysfs
- regex: minimal to no benefit
- md: persistent saves a read on devices that get passed through this; however, this is dangerous as per RHBZ 598135
- lvm/suspended: this is outright wrong as per parent post
- lvm/partition: similar to the md case: saves IO in the good case, produces incorrect results sometimes

Basically, the persistent device cache is responsible for a few corner cases (related to 598135 where the cache is out of sync with reality) and a few bugs where the benefit of the persistent cache would be nonexistent anyway.

Overall, we have a modest reduction in IO to non-LVM devices in exchange for a couple deadlock and data corruption bugs. We can work around some of those, but I reckon that the whole idea of the persistent cache is a misfeature.

The only cases where this makes any sense is MD detection and partition table detection (as per above; other filters don't issue IO to the device and are simply too cheap to optimise in this way), with at least the MD case being prone to introducing data corruption. This leaves partition table detection, which only happens on a subset of the devices and could be optimised for the positive case by looking into sysfs, eg.

So I think there are two basic options: either we can move the persistent device filter down the chain to only cover filters where it makes sense, or disable it altogether. I am not very fond of the option #3, keep adding workarounds similar to the parent post.

Yours,
Petr.

This is up for review as part of the udev cache integration work. The filter was intended to be recording persistently which PVs are in which VGs - which is where its best value would come from - but that still hasn't been added to it, years later. Its value diminished after /sys/block appeared and was used.

You can always test its value on any specific system by disabling it in lvm.conf (devices/write_cache_state = 0).

Alasdair