PATCH: do not unnecessarily open partition device nodes (and make udev hate us)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

After patching parted with my do not use BLKPG patch, I started to get EBUSY
errors on commit_to_os. Note this is not caused by the do not use BLKPG patch,
this was already happening, but instead of parted silently ignoring the errors
(and the kernel not knowing about the changes, which is bad). The error now
actually gets reported.

The problem turns out to be in:
libparted/arch/linux.c: _flush_cache()

Which walks all the partitions of the disk and does
BLKFLSBUF calls on them. This causes the following:

commit_to_os -> device_open -> fd = open /dev/sda ->
_flush_cache -> for each /dev/sda# open, ioctl, close
-> ioctl(fd, BLKRRPART) -> EBUSY

What is happening here is that the:
for each /dev/sda# open, ioctl, close

Is causing udev change events for all the /dev/sda#
nodes, which causes udev to call blkid on all these nodes
(on systems which use DeviceKit), so blkid has /dev/sda# nodes
open while BLKRRPART gets called on /dev/sda -> EBUSY.

I've checked with 2 independend storage subsystem kernel developers,
and /dev/sda and /dev/sda#, guarantee cache coherency now a days. So there is
no need to do this for 2.6, which also eliminates the need to call
_flush_cache() on device open at all.

I've attached a patch which implements this.

Regards,

Hans



diff -up parted-1.9.0/libparted/arch/linux.c~ parted-1.9.0/libparted/arch/linux.c
--- parted-1.9.0/libparted/arch/linux.c~	2009-08-27 11:23:59.000000000 +0200
+++ parted-1.9.0/libparted/arch/linux.c	2009-08-27 14:28:47.000000000 +0200
@@ -586,6 +586,19 @@ _get_linux_version ()
         return kver = KERNEL_VERSION (major, minor, teeny);
 }
 
+static int
+_have_kern26 ()
+{
+        static int have_kern26 = -1;
+        int kver;
+
+        if (have_kern26 != -1)
+                return have_kern26;
+
+        kver = _get_linux_version();
+        return have_kern26 = kver >= KERNEL_VERSION (2,6,0) ? 1 : 0;
+}
+  
 static void
 _device_set_sector_size (PedDevice* dev)
 {
@@ -1354,8 +1367,8 @@ linux_is_busy (PedDevice* dev)
         return 0;
 }
 
-/* we need to flush the master device, and all the partition devices,
- * because there is no coherency between the caches.
+/* we need to flush the master device, and with kernel < 2.6 all the partition
+ * devices, because there is no coherency between the caches with old kernels.
  * We should only flush unmounted partition devices, because:
  *  - there is never a need to flush them (we're not doing IO there)
  *  - flushing a device that is mounted causes unnecessary IO, and can
@@ -1373,21 +1386,23 @@ _flush_cache (PedDevice* dev)
 
         ioctl (arch_specific->fd, BLKFLSBUF);
 
-        for (i = 1; i < 16; i++) {
-                char*           name;
-                int             fd;
+        if (!_have_kern26()) {
+                for (i = 1; i < 16; i++) {
+                        char*           name;
+                        int             fd;
 
-                name = _device_get_part_path (dev, i);
-                if (!name)
-                        break;
-                if (!_partition_is_mounted_by_path (name)) {
-                        fd = open (name, WR_MODE, 0);
-                        if (fd > 0) {
-                                ioctl (fd, BLKFLSBUF);
-                                close (fd);
+                        name = _device_get_part_path (dev, i);
+                        if (!name)
+                                break;
+                        if (!_partition_is_mounted_by_path (name)) {
+                                fd = open (name, WR_MODE, 0);
+                                if (fd > 0) {
+                                        ioctl (fd, BLKFLSBUF);
+                                        close (fd);
+                                }
                         }
+                        free (name);
                 }
-                free (name);
         }
 }
 
@@ -1428,7 +1443,9 @@ retry:
                 dev->read_only = 0;
         }
 
-        _flush_cache (dev);
+        /* With kernels < 2.6 flush cache for cache coherence issues */
+        if (!_have_kern26())
+                _flush_cache (dev);
 
         return 1;
 }
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux