sg_persist triggers block kernel event ???

sg_persist triggers block kernel event ???

[Christophe Varoqui]

[-- Attachment #1.1: Type: text/plain, Size: 1015 bytes --]

Hi list,

I observe this on a debian 7.5 server with a udevadm monitor running in the
background :

# sg_persist -n -k /dev/sdbh
  PR generation=0x0, there are NO registered reservation keys

KERNEL[448809.342461] change
/devices/pci0000:20/0000:20:02.2/0000:24:00.0/host0/rport-0:0-3/target0:0:1/0:0:1:12/block/sdbh
(block)
ACTION=change
DEVNAME=/dev/sdbh
DEVPATH=/devices/pci0000:20/0000:20:02.2/0000:24:00.0/host0/rport-0:0-3/target0:0:1/0:0:1:12/block/sdbh
DEVTYPE=disk
MAJOR=67
MINOR=176
SEQNUM=261605
SUBSYSTEM=block

Every sg_persist command, with any options, trigger events.

On this server with more than 200 scsi devices, each receiving one read-key
and one read-reservation every 10 minutes, this triggers quite a eavy load
caused by 2 udev triggers :

1/ multipath -v0 $devpath
2/ udisks-lvm-pv-export $pv_uuid


Question is, is it normal for a "--in" sg_persist command to trigger a
change event on the scsi device ? If not, what we can do about it ?

Best regards,
Christophe Varoqui
www.opensvc.com

[-- Attachment #1.2: Type: text/html, Size: 1451 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: sg_persist triggers block kernel event ???

[Christophe Varoqui]

[-- Attachment #1.1: Type: text/plain, Size: 2006 bytes --]

Reproduced on a fairly recent kernel :

cvaroqui@clementine:~$ sudo sg_persist -k /dev/sda
  ATA       SAMSUNG MZMTD512  DXT4
  Peripheral device type: disk
PR in: command not supported

KERNEL[227056.238465] change
/devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda
(block)
ACTION=change
DEVNAME=/dev/sda
DEVPATH=/devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda
DEVTYPE=disk
MAJOR=8
MINOR=0
SEQNUM=2603
SUBSYSTEM=block

cvaroqui@clementine:~$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=14.04
DISTRIB_CODENAME=trusty
DISTRIB_DESCRIPTION="Ubuntu Trusty Tahr (development branch)"

cvaroqui@clementine:~$ uname -a
Linux clementine 3.13.0-23-generic #45-Ubuntu SMP Fri Apr 4 06:58:38 UTC
2014 x86_64 x86_64 x86_64 GNU/Linux

Regards,
Christophe Varoqui
www.opensvc.com


On Sat, May 3, 2014 at 10:12 PM, Christophe Varoqui <
christophe.varoqui@opensvc.com> wrote:

> Hi list,
>
> I observe this on a debian 7.5 server with a udevadm monitor running in
> the background :
>
> # sg_persist -n -k /dev/sdbh
>   PR generation=0x0, there are NO registered reservation keys
>
> KERNEL[448809.342461] change
> /devices/pci0000:20/0000:20:02.2/0000:24:00.0/host0/rport-0:0-3/target0:0:1/0:0:1:12/block/sdbh
> (block)
> ACTION=change
> DEVNAME=/dev/sdbh
>
> DEVPATH=/devices/pci0000:20/0000:20:02.2/0000:24:00.0/host0/rport-0:0-3/target0:0:1/0:0:1:12/block/sdbh
> DEVTYPE=disk
> MAJOR=67
> MINOR=176
> SEQNUM=261605
> SUBSYSTEM=block
>
> Every sg_persist command, with any options, trigger events.
>
> On this server with more than 200 scsi devices, each receiving one
> read-key and one read-reservation every 10 minutes, this triggers quite a
> eavy load caused by 2 udev triggers :
>
> 1/ multipath -v0 $devpath
> 2/ udisks-lvm-pv-export $pv_uuid
>
>
> Question is, is it normal for a "--in" sg_persist command to trigger a
> change event on the scsi device ? If not, what we can do about it ?
>
> Best regards,
> Christophe Varoqui
> www.opensvc.com
>
>

[-- Attachment #1.2: Type: text/html, Size: 3091 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: sg_persist triggers block kernel event ???

[Christophe Varoqui]

[-- Attachment #1.1: Type: text/plain, Size: 3237 bytes --]

Adding more information to this case,

also reproduced on a sles11 and with mpathpersist in place of sg_persist
(sde and sdf are paths of dm-4) :

# mpathpersist -i -k /dev/mapper/igrM2h-U2Ci-O3jL
KERNEL[1399153645.728611] change
/devices/platform/host2/session1/target2:0:0/2:0:0:2/block/sdf (block)
UDEV_LOG=3
ACTION=change
DEVPATH=/devices/platform/host2/session1/target2:0:0/2:0:0:2/block/sdf
SUBSYSTEM=block
DEVNAME=sdf
DEVTYPE=disk
SEQNUM=1811
MAJOR=8
MINOR=80

KERNEL[1399153645.729377] change
/devices/platform/host3/session2/target3:0:0/3:0:0:2/block/sde (block)
UDEV_LOG=3
ACTION=change
DEVPATH=/devices/platform/host3/session2/target3:0:0/3:0:0:2/block/sde
SUBSYSTEM=block
DEVNAME=sde
DEVTYPE=disk
SEQNUM=1812
MAJOR=8
MINOR=64

Persistent Reserve IN command failed

KERNEL[1399153645.730116] change   /devices/virtual/block/dm-4 (block)
UDEV_LOG=3
ACTION=change
DEVPATH=/devices/virtual/block/dm-4
SUBSYSTEM=block
DEVNAME=dm-4
DEVTYPE=disk
SEQNUM=1813
MAJOR=252
MINOR=4

Regards,
Christophe Varoqui
www.opensvc.com


On Sat, May 3, 2014 at 10:38 PM, Christophe Varoqui <
christophe.varoqui@opensvc.com> wrote:

> Reproduced on a fairly recent kernel :
>
> cvaroqui@clementine:~$ sudo sg_persist -k /dev/sda
>   ATA       SAMSUNG MZMTD512  DXT4
>   Peripheral device type: disk
> PR in: command not supported
>
> KERNEL[227056.238465] change
> /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda
> (block)
> ACTION=change
> DEVNAME=/dev/sda
>
> DEVPATH=/devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sda
> DEVTYPE=disk
> MAJOR=8
> MINOR=0
> SEQNUM=2603
> SUBSYSTEM=block
>
> cvaroqui@clementine:~$ cat /etc/lsb-release
> DISTRIB_ID=Ubuntu
> DISTRIB_RELEASE=14.04
> DISTRIB_CODENAME=trusty
> DISTRIB_DESCRIPTION="Ubuntu Trusty Tahr (development branch)"
>
> cvaroqui@clementine:~$ uname -a
> Linux clementine 3.13.0-23-generic #45-Ubuntu SMP Fri Apr 4 06:58:38 UTC
> 2014 x86_64 x86_64 x86_64 GNU/Linux
>
> Regards,
> Christophe Varoqui
> www.opensvc.com
>
>
> On Sat, May 3, 2014 at 10:12 PM, Christophe Varoqui <
> christophe.varoqui@opensvc.com> wrote:
>
>> Hi list,
>>
>> I observe this on a debian 7.5 server with a udevadm monitor running in
>> the background :
>>
>> # sg_persist -n -k /dev/sdbh
>>   PR generation=0x0, there are NO registered reservation keys
>>
>> KERNEL[448809.342461] change
>> /devices/pci0000:20/0000:20:02.2/0000:24:00.0/host0/rport-0:0-3/target0:0:1/0:0:1:12/block/sdbh
>> (block)
>> ACTION=change
>> DEVNAME=/dev/sdbh
>>
>> DEVPATH=/devices/pci0000:20/0000:20:02.2/0000:24:00.0/host0/rport-0:0-3/target0:0:1/0:0:1:12/block/sdbh
>> DEVTYPE=disk
>> MAJOR=67
>> MINOR=176
>> SEQNUM=261605
>> SUBSYSTEM=block
>>
>> Every sg_persist command, with any options, trigger events.
>>
>> On this server with more than 200 scsi devices, each receiving one
>> read-key and one read-reservation every 10 minutes, this triggers quite a
>> eavy load caused by 2 udev triggers :
>>
>> 1/ multipath -v0 $devpath
>> 2/ udisks-lvm-pv-export $pv_uuid
>>
>>
>> Question is, is it normal for a "--in" sg_persist command to trigger a
>> change event on the scsi device ? If not, what we can do about it ?
>>
>> Best regards,
>> Christophe Varoqui
>> www.opensvc.com
>>
>>
>

[-- Attachment #1.2: Type: text/html, Size: 5121 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: sg_persist triggers block kernel event ???

[Christophe Varoqui]

[-- Attachment #1.1: Type: text/plain, Size: 2432 bytes --]

It seems sg_persist is doing an "open rw => close" for --in commands,
causing a kernel change-event.
I can not verify at the moment if the following patch would work ...
comments ?

--- sg_persist.c.orig 2014-05-04 01:10:01.987981956 +0200
+++ sg_persist.c 2014-05-04 01:15:34.880008000 +0200
@@ -1029,6 +1029,8 @@
     struct sg_simple_inquiry_resp inq_resp;
     const char * cp;
     struct opts_t opts;
+    int omode = 0;
+    const char *omode_desc = NULL;

     memset(&opts, 0, sizeof(opts));
     opts.prin = 1;
@@ -1292,10 +1294,18 @@
         sg_cmds_close_device(sg_fd);
     }

-    if ((sg_fd = sg_cmds_open_device(device_name, 0 /* rw */,
+    if (opts.prin) {
+        omode = 1;
+        omode_desc = "ro";
+    } else {
+        omode = 0;
+        omode_desc = "rw";
+    }
+
+    if ((sg_fd = sg_cmds_open_device(device_name, omode,
                                      opts.verbose)) < 0) {
-        pr2serr("sg_persist: error opening file (rw): %s: %s\n",
device_name,
-                safe_strerror(-sg_fd));
+        pr2serr("sg_persist: error opening file (%s): %s: %s\n",
omode_desc,
+                device_name, safe_strerror(-sg_fd));
         return SG_LIB_FILE_ERROR;
     }

Regards,
Christophe Varoqui
www.opensvc.com


On Sat, May 3, 2014 at 10:12 PM, Christophe Varoqui <
christophe.varoqui@opensvc.com> wrote:

> Hi list,
>
> I observe this on a debian 7.5 server with a udevadm monitor running in
> the background :
>
> # sg_persist -n -k /dev/sdbh
>   PR generation=0x0, there are NO registered reservation keys
>
> KERNEL[448809.342461] change
> /devices/pci0000:20/0000:20:02.2/0000:24:00.0/host0/rport-0:0-3/target0:0:1/0:0:1:12/block/sdbh
> (block)
> ACTION=change
> DEVNAME=/dev/sdbh
>
> DEVPATH=/devices/pci0000:20/0000:20:02.2/0000:24:00.0/host0/rport-0:0-3/target0:0:1/0:0:1:12/block/sdbh
> DEVTYPE=disk
> MAJOR=67
> MINOR=176
> SEQNUM=261605
> SUBSYSTEM=block
>
> Every sg_persist command, with any options, trigger events.
>
> On this server with more than 200 scsi devices, each receiving one
> read-key and one read-reservation every 10 minutes, this triggers quite a
> eavy load caused by 2 udev triggers :
>
> 1/ multipath -v0 $devpath
> 2/ udisks-lvm-pv-export $pv_uuid
>
>
> Question is, is it normal for a "--in" sg_persist command to trigger a
> change event on the scsi device ? If not, what we can do about it ?
>
> Best regards,
> Christophe Varoqui
> www.opensvc.com
>
>

[-- Attachment #1.2: Type: text/html, Size: 3846 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: sg_persist triggers block kernel event ???

[Hannes Reinecke]

On 05/04/2014 01:34 AM, Christophe Varoqui wrote:
> It seems sg_persist is doing an "open rw => close" for --in commands,
> causing a kernel change-event.
Yep.

Look for 'watch' in the udev rules, that's precisely what it's doing.

(Bloody annoying if you ask me. Generally I recommend to remove that 
thing from the rules).

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: sg_persist triggers block kernel event ???

[Zdenek Kabelac]

Dne 3.5.2014 22:12, Christophe Varoqui napsal(a):
> Hi list,
>
> I observe this on a debian 7.5 server with a udevadm monitor running in the
> background :
>
> # sg_persist -n -k /dev/sdbh
>    PR generation=0x0, there are NO registered reservation keys
>

Whoever opens   device in 'read-write' mode  and then closes device triggers 
udev change event. (udev rescans whatever has changed on device after this)

That's how it works for many years already.
It's not a bug and some even says it's a feature....

Zdenek

Re: sg_persist triggers block kernel event ???

[Zdenek Kabelac]

Dne 4.5.2014 18:54, Hannes Reinecke napsal(a):
> On 05/04/2014 01:34 AM, Christophe Varoqui wrote:
>> It seems sg_persist is doing an "open rw => close" for --in commands,
>> causing a kernel change-event.
> Yep.
>
> Look for 'watch' in the udev rules, that's precisely what it's doing.
>
> (Bloody annoying if you ask me. Generally I recommend to remove that thing
> from the rules).

When watch rule is disabled/removed in  udev rules - your udev db becomes 
invalid when i.e. you run  command like  'mkfs' - since the udev db will not 
be updated to list information about newly formatted filesystem.

Of course there are many cases where disabling  watch rule makes sense
(i.e. you export lots of disks to virtual guests) - but unless you are
familiar with udev and you know what you are doing - think twice before disabling.

Zdenek

Re: sg_persist triggers block kernel event ???

[Christophe Varoqui]

[-- Attachment #1.1: Type: text/plain, Size: 2743 bytes --]

Indeed, I'd rather let udev do its work on legitimate device changes.
Problem is, this change event caused by sg_persist prin commands in not
legitimate : no device change can be caused by interrogating the disk pr
status.

Doug informed me newer kernels accept prin commands inside a open-ro=>close
sequence, which I now verified by testing with the patch I sent ... but
older kernels do not. My patch is thus not backward compatible as-is.

Any advice on how to treat this backward-compatibility issue ?

For information, here's a more recent version of the patch I sent to Doug
earlier to address not-Linux platform behaviour stability :

--- sg_persist.c.orig 2014-05-04 01:10:01.987981956 +0200
+++ sg_persist.c 2014-05-04 08:41:29.482017943 +0200
@@ -1029,6 +1029,8 @@
     struct sg_simple_inquiry_resp inq_resp;
     const char * cp;
     struct opts_t opts;
+    int omode = 0;
+    const char *omode_desc = "rw";

     memset(&opts, 0, sizeof(opts));
     opts.prin = 1;
@@ -1292,10 +1294,17 @@
         sg_cmds_close_device(sg_fd);
     }

-    if ((sg_fd = sg_cmds_open_device(device_name, 0 /* rw */,
+#ifdef SG_LIB_LINUX
+    if (opts.prin) {
+        omode = 1;
+        omode_desc = "ro";
+    }
+#endif
+
+    if ((sg_fd = sg_cmds_open_device(device_name, omode,
                                      opts.verbose)) < 0) {
-        pr2serr("sg_persist: error opening file (rw): %s: %s\n",
device_name,
-                safe_strerror(-sg_fd));
+        pr2serr("sg_persist: error opening file (%s): %s: %s\n",
omode_desc,
+                device_name, safe_strerror(-sg_fd));
         return SG_LIB_FILE_ERROR;
     }

Regards,
Christophe Varoqui
www.opensvc.com



On Sun, May 4, 2014 at 7:23 PM, Zdenek Kabelac <zkabelac@redhat.com> wrote:

> Dne 4.5.2014 18:54, Hannes Reinecke napsal(a):
>
>  On 05/04/2014 01:34 AM, Christophe Varoqui wrote:
>>
>>> It seems sg_persist is doing an "open rw => close" for --in commands,
>>> causing a kernel change-event.
>>>
>> Yep.
>>
>> Look for 'watch' in the udev rules, that's precisely what it's doing.
>>
>> (Bloody annoying if you ask me. Generally I recommend to remove that thing
>> from the rules).
>>
>
> When watch rule is disabled/removed in  udev rules - your udev db becomes
> invalid when i.e. you run  command like  'mkfs' - since the udev db will
> not be updated to list information about newly formatted filesystem.
>
> Of course there are many cases where disabling  watch rule makes sense
> (i.e. you export lots of disks to virtual guests) - but unless you are
> familiar with udev and you know what you are doing - think twice before
> disabling.
>
> Zdenek
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>

[-- Attachment #1.2: Type: text/html, Size: 5156 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: sg_persist triggers block kernel event ???

[Christophe Varoqui]

[-- Attachment #1.1: Type: text/plain, Size: 5196 bytes --]

Doug,

would this patch address the Linux kernel backward-compatibility concern
you raised ?

Note, you would have to substitute the 3000 version by the appropriate
version that introduced support for prin commands submitted on a open-ro
device ...

--- sg_persist.c.orig 2014-05-04 01:10:01.987981956 +0200
+++ sg_persist.c 2014-05-04 20:44:34.665292548 +0200
@@ -16,6 +16,7 @@
 #include <string.h>
 #include <ctype.h>
 #include <getopt.h>
+#include <sys/ioctl.h>
 #define __STDC_FORMAT_MACROS 1

 #include <inttypes.h>
@@ -26,6 +27,7 @@
 #include "sg_lib.h"
 #include "sg_cmds_basic.h"
 #include "sg_cmds_extra.h"
+#include "sg_io_linux.h"

 static const char * version_str = "0.44 20140202";

@@ -1029,6 +1031,8 @@
     struct sg_simple_inquiry_resp inq_resp;
     const char * cp;
     struct opts_t opts;
+    int omode = 0;
+    const char *omode_desc = "rw";

     memset(&opts, 0, sizeof(opts));
     opts.prin = 1;
@@ -1292,10 +1296,31 @@
         sg_cmds_close_device(sg_fd);
     }

-    if ((sg_fd = sg_cmds_open_device(device_name, 0 /* rw */,
+#ifdef SG_LIB_LINUX
+    /*
+     * PRIN commands do not provoke device changes, so we should avoid to
+     * open the device read-write so that the Linux kernel does not
generate
+     * a change event.
+     * Older kernel do not support PRIN commands submission on a read-only
+     * opened device, so don't try in this case.
+     */
+    int v;
+    if (opts.prin) {
+        sg_fd = sg_cmds_open_device(device_name, 1, opts.verbose);
+        if (sg_fd >= 0) {
+            if (ioctl(sg_fd, SG_GET_VERSION_NUM, &v) >= 0 && v >= 30000) {
+                omode = 1;
+                omode_desc = "ro";
+            }
+            sg_cmds_close_device(sg_fd);
+        }
+    }
+#endif
+
+    if ((sg_fd = sg_cmds_open_device(device_name, omode,
                                      opts.verbose)) < 0) {
-        pr2serr("sg_persist: error opening file (rw): %s: %s\n",
device_name,
-                safe_strerror(-sg_fd));
+        pr2serr("sg_persist: error opening file (%s): %s: %s\n",
omode_desc,
+                device_name, safe_strerror(-sg_fd));
         return SG_LIB_FILE_ERROR;
     }




Best regards,
Christophe Varoqui
www.opensvc.com


On Sun, May 4, 2014 at 8:01 PM, Christophe Varoqui <
christophe.varoqui@opensvc.com> wrote:

> Indeed, I'd rather let udev do its work on legitimate device changes.
> Problem is, this change event caused by sg_persist prin commands in not
> legitimate : no device change can be caused by interrogating the disk pr
> status.
>
> Doug informed me newer kernels accept prin commands inside a
> open-ro=>close sequence, which I now verified by testing with the patch I
> sent ... but older kernels do not. My patch is thus not backward compatible
> as-is.
>
> Any advice on how to treat this backward-compatibility issue ?
>
> For information, here's a more recent version of the patch I sent to Doug
> earlier to address not-Linux platform behaviour stability :
>
> --- sg_persist.c.orig 2014-05-04 01:10:01.987981956 +0200
> +++ sg_persist.c 2014-05-04 08:41:29.482017943 +0200
> @@ -1029,6 +1029,8 @@
>      struct sg_simple_inquiry_resp inq_resp;
>      const char * cp;
>      struct opts_t opts;
> +    int omode = 0;
> +    const char *omode_desc = "rw";
>
>      memset(&opts, 0, sizeof(opts));
>      opts.prin = 1;
> @@ -1292,10 +1294,17 @@
>          sg_cmds_close_device(sg_fd);
>      }
>
> -    if ((sg_fd = sg_cmds_open_device(device_name, 0 /* rw */,
> +#ifdef SG_LIB_LINUX
> +    if (opts.prin) {
> +        omode = 1;
> +        omode_desc = "ro";
> +    }
> +#endif
> +
> +    if ((sg_fd = sg_cmds_open_device(device_name, omode,
>                                       opts.verbose)) < 0) {
> -        pr2serr("sg_persist: error opening file (rw): %s: %s\n",
> device_name,
> -                safe_strerror(-sg_fd));
> +        pr2serr("sg_persist: error opening file (%s): %s: %s\n",
> omode_desc,
> +                device_name, safe_strerror(-sg_fd));
>          return SG_LIB_FILE_ERROR;
>      }
>
> Regards,
> Christophe Varoqui
> www.opensvc.com
>
>
>
> On Sun, May 4, 2014 at 7:23 PM, Zdenek Kabelac <zkabelac@redhat.com>wrote:
>
>> Dne 4.5.2014 18:54, Hannes Reinecke napsal(a):
>>
>>  On 05/04/2014 01:34 AM, Christophe Varoqui wrote:
>>>
>>>> It seems sg_persist is doing an "open rw => close" for --in commands,
>>>> causing a kernel change-event.
>>>>
>>> Yep.
>>>
>>> Look for 'watch' in the udev rules, that's precisely what it's doing.
>>>
>>> (Bloody annoying if you ask me. Generally I recommend to remove that
>>> thing
>>> from the rules).
>>>
>>
>> When watch rule is disabled/removed in  udev rules - your udev db becomes
>> invalid when i.e. you run  command like  'mkfs' - since the udev db will
>> not be updated to list information about newly formatted filesystem.
>>
>> Of course there are many cases where disabling  watch rule makes sense
>> (i.e. you export lots of disks to virtual guests) - but unless you are
>> familiar with udev and you know what you are doing - think twice before
>> disabling.
>>
>> Zdenek
>>
>> --
>> dm-devel mailing list
>> dm-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/dm-devel
>>
>
>

[-- Attachment #1.2: Type: text/html, Size: 9177 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]