Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#782 closed enhancement (fixed)

Support sg_io_v4 on /dev/bsg/*

Reported by: Alexander K Austin Owned by: Alex Samorukov
Priority: minor Milestone: Release 6.6
Component: all Version: 6.5
Keywords: linux scsi Cc:

Description

This enables usage on /dev/bsg/* devices. It has been trivially tested on my machine.

Attachments (3)

smart_add_bsg.patch​ (9.1 KB ) - added by Alexander K Austin 8 years ago.
Patch on top of 6.5 to enable sg_io_v4
sgio.patch​ (8.8 KB ) - added by Alex Samorukov 7 years ago.
Fixed patch against r4523
sgio.2.patch​ (10.7 KB ) - added by Alex Samorukov 7 years ago.
Updated SG_IO patch to use v3 and v4 api from the same function

Download all attachments as: .zip

Change History (23)

by Alexander K Austin, 8 years ago

Attachment: smart_add_bsg.patch​ added

Patch on top of 6.5 to enable sg_io_v4

comment:1 by Christian Franke, 8 years ago

Milestone: → undecided

comment:2 by Alexander K Austin, 8 years ago

Without this patch, smartctl uses SCSI_IOCTL_SEND_COMMAND when given a /dev/bsg/* device, which results in "program smartctl is using a deprecated SCSI ioctl, please convert it to SG_IO" in dmesg.

comment:3 by Christian Franke, 8 years ago

Keywords: linux scsi added

comment:4 by Alex Samorukov, 7 years ago

Last edited 7 years ago by Alex Samorukov (previous) (diff)

comment:5 by Alex Samorukov, 7 years ago

There is at least one problem found:

Patched version:

surikat:/home/samm/src/smartmontools# ./smartctl.patched -a -d scsi /dev/bsg/2\:0\:0\:0
smartctl 6.6 2017-10-06 r4514 [i686-linux-3.2.0-4-686-pae] (local build)
Copyright (C) 2002-17, Bruce Allen, Christian Franke, www.smartmontools.org

Warning: DEFAULT entry missing in drive database file(s)
=== START OF INFORMATION SECTION ===
Vendor:               IBM-ESXS
Product:              MAU3073NC     FN
Revision:             BC12
User Capacity:        73,407,488,000 bytes [73.4 GB]
Logical block size:   512 bytes
Rotation Rate:        15000 rpm
Serial number:        ANF9P5802M68
Device type:          disk
Transport protocol:   Parallel SCSI (SPI-4)
Local Time is:        Fri Oct  6 10:31:20 2017 CEST
device Test Unit Ready  [Invalid argument]
A mandatory SMART command failed: exiting. To continue, add one or more '-T permissive' options.

Non-patched:

surikat:/home/samm/src/smartmontools# ./smartctl -a -d scsi /dev/bsg/2\:0\:0\:0
smartctl 6.6 2017-10-06 r4514 [i686-linux-3.2.0-4-686-pae] (local build)
Copyright (C) 2002-17, Bruce Allen, Christian Franke, www.smartmontools.org

Warning: DEFAULT entry missing in drive database file(s)
=== START OF INFORMATION SECTION ===
Vendor:               IBM-ESXS
Product:              MAU3073NC     FN
Revision:             BC12
User Capacity:        73,407,488,000 bytes [73.4 GB]
Logical block size:   512 bytes
Rotation Rate:        15000 rpm
Serial number:        ANF9P5802M68
Device type:          disk
Transport protocol:   Parallel SCSI (SPI-4)
Local Time is:        Fri Oct  6 10:31:53 2017 CEST
SMART support is:     Available - device has SMART capability.
SMART support is:     Enabled
Temperature Warning:  Enabled

=== START OF READ SMART DATA SECTION ===
SMART Health Status: OK

Current Drive Temperature:     25 C
Drive Trip Temperature:        65 C

Manufactured in week 33 of year 2005
Specified cycle count over device lifetime:  10000
Accumulated start-stop cycles:  41
Elements in grown defect list: 1

Error counter log:
           Errors Corrected by           Total   Correction     Gigabytes    Total
               ECC          rereads/    errors   algorithm      processed    uncorrected
           fast | delayed   rewrites  corrected  invocations   [10^9 bytes]  errors
read:          0      456         0         0          0     861129.721           0
write:         0       28         0         0          0      35898.106           0
verify:        0        0         0         0          0         14.681           0

Non-medium error count:     1423

SMART Self-test log
Num  Test              Status                 segment  LifeTime  LBA_first_err [SK ASC ASQ]
     Description                              number   (hours)
# 1  Background long   Completed                   -   65535                 - [-   -    -]
# 2  Background long   Completed                   -   65535                 - [-   -    -]
# 3  Background long   Aborted (by user command)   -   52368                 - [-   -    -]
# 4  Background short  Aborted (by user command)   -   52368                 - [-   -    -]
# 5  Background short  Completed                   -   52095                 - [-   -    -]
# 6  Background long   Completed                   -   52092                 - [-   -    -]
# 7  Background long   Aborted (by user command)   -   52089                 - [-   -    -]
# 8  Background long   Aborted (by user command)   -   52089                 - [-   -    -]
# 9  Background short  Aborted (by user command)   -   52087                 - [-   -    -]
#10  Background short  Aborted (by user command)   -   52087                 - [-   -    -]
#11  Background short  Aborted (by user command)   -   52087                 - [-   -    -]
#12  Background short  Aborted (by user command)   -   52087                 - [-   -    -]
#13  Background long   Completed                   -   52056                 - [-   -    -]
#14  Background short  Completed                   -   52053                 - [-   -    -]

Long (extended) Self Test duration: 1724 seconds [28.7 minutes]

surikat:/home/samm/src/smartmontools#
surikat:/home/samm/src/smartmontools# ./smartctl -a -d scsi /dev/bsg/2\:0\:0\:0
smartctl 6.6 2017-10-06 r4514 [i686-linux-3.2.0-4-686-pae] (local build)
Copyright (C) 2002-17, Bruce Allen, Christian Franke, www.smartmontools.org

Warning: DEFAULT entry missing in drive database file(s)
=== START OF INFORMATION SECTION ===
Vendor:               IBM-ESXS
Product:              MAU3073NC     FN
Revision:             BC12
User Capacity:        73,407,488,000 bytes [73.4 GB]
Logical block size:   512 bytes
Rotation Rate:        15000 rpm
Serial number:        ANF9P5802M68
Device type:          disk
Transport protocol:   Parallel SCSI (SPI-4)
Local Time is:        Fri Oct  6 10:31:58 2017 CEST
SMART support is:     Available - device has SMART capability.
SMART support is:     Enabled
Temperature Warning:  Enabled

=== START OF READ SMART DATA SECTION ===
SMART Health Status: OK

Current Drive Temperature:     25 C
Drive Trip Temperature:        65 C

Manufactured in week 33 of year 2005
Specified cycle count over device lifetime:  10000
Accumulated start-stop cycles:  41
Elements in grown defect list: 1

Error counter log:
           Errors Corrected by           Total   Correction     Gigabytes    Total
               ECC          rereads/    errors   algorithm      processed    uncorrected
           fast | delayed   rewrites  corrected  invocations   [10^9 bytes]  errors
read:          0      456         0         0          0     861129.722           0
write:         0       28         0         0          0      35898.106           0
verify:        0        0         0         0          0         14.681           0

Non-medium error count:     1423

SMART Self-test log
Num  Test              Status                 segment  LifeTime  LBA_first_err [SK ASC ASQ]
     Description                              number   (hours)
# 1  Background long   Completed                   -   65535                 - [-   -    -]
# 2  Background long   Completed                   -   65535                 - [-   -    -]
# 3  Background long   Aborted (by user command)   -   52368                 - [-   -    -]
# 4  Background short  Aborted (by user command)   -   52368                 - [-   -    -]
# 5  Background short  Completed                   -   52095                 - [-   -    -]
# 6  Background long   Completed                   -   52092                 - [-   -    -]
# 7  Background long   Aborted (by user command)   -   52089                 - [-   -    -]
# 8  Background long   Aborted (by user command)   -   52089                 - [-   -    -]
# 9  Background short  Aborted (by user command)   -   52087                 - [-   -    -]
#10  Background short  Aborted (by user command)   -   52087                 - [-   -    -]
#11  Background short  Aborted (by user command)   -   52087                 - [-   -    -]
#12  Background short  Aborted (by user command)   -   52087                 - [-   -    -]
#13  Background long   Completed                   -   52056                 - [-   -    -]
#14  Background short  Completed                   -   52053                 - [-   -    -]

Long (extended) Self Test duration: 1724 seconds [28.7 minutes]

comment:6 by Alex Samorukov, 7 years ago

P.S. it actually works with with -T permissive and only function fails is device Test Unit Ready. Also if using -x i see scsiPrintBackgroundResults Failed [Invalid argument] instead of Background scan results log.

Please fix this issues and resubmit the patch or let me know if you need more debugging.

Last edited 7 years ago by Alex Samorukov (previous) (diff)

comment:7 by Alex Samorukov, 7 years ago

Also some commands fails on the SAT disk, see diff:

  • .txt 

    surikat:/home/samm/src/smartmontools# diff -u out_normal.txt out_patched.txt
    old new  
    1313Device is:        In smartctl database [for details use: -P show]
    1414ATA Version is:   ACS-2 (minor revision not indicated)
    1515SATA Version is:  SATA 3.0, 6.0 Gb/s (current: 1.5 Gb/s)
    16 Local Time is:    Fri Oct  6 10:45:23 2017 CEST
     16Local Time is:    Fri Oct  6 10:45:31 2017 CEST
    1717SMART support is: Available - device has SMART capability.
    1818SMART support is: Enabled
    1919AAM feature is:   Unavailable
    … …  
    2222Write cache is:   Enabled
    2323DSN feature is:   Unavailable
    2424ATA Security is:  Disabled, NOT FROZEN [SEC1]
    25 Wt Cache Reorder: Enabled
     25Write SCT (Get) Feature Control Command failed: Operation not supported
     26Wt Cache Reorder: Unknown (SCT Feature Control command failed)
    2627
    2728=== START OF READ SMART DATA SECTION ===
    28 SMART Status command failed
    29 Please get assistance from http://www.smartmontools.org/
    30 Register values returned from SMART Status command are:
    31  ERR=0x00, SC=0x00, LL=0x00, LM=0x00, LH=0x00, DEV=0x00, STS=0x00
    32 SMART Status not supported: Invalid ATA output register values
     29SMART Status command failed: Invalid argument
    3330SMART overall-health self-assessment test result: PASSED
    3431Warning: This result is based on an Attribute check.
    3532
    … …  
    15415101 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    15515200 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    156153
    157 SCT Temperature History Version:     2
    158 Temperature Sampling Period:         1 minute
    159 Temperature Logging Interval:        1 minute
    160 Min/Max recommended Temperature:      0/60 Celsius
    161 Min/Max Temperature Limit:           -41/85 Celsius
    162 Temperature History Size (Index):    478 (179)
    163 
    164 Index    Estimated Time   Temperature Celsius
    165  180    2017-10-06 02:48    25  ******
    166  ...    ..(  3 skipped).    ..  ******
    167  184    2017-10-06 02:52    25  ******
    168  185    2017-10-06 02:53    26  *******
    169  ...    ..(  3 skipped).    ..  *******
    170  189    2017-10-06 02:57    26  *******
    171  190    2017-10-06 02:58    27  ********
    172  ...    ..( 11 skipped).    ..  ********
    173  202    2017-10-06 03:10    27  ********
    174  203    2017-10-06 03:11    26  *******
    175  ...    ..(  4 skipped).    ..  *******
    176  208    2017-10-06 03:16    26  *******
    177  209    2017-10-06 03:17    25  ******
    178  ...    ..( 28 skipped).    ..  ******
    179  238    2017-10-06 03:46    25  ******
    180  239    2017-10-06 03:47    26  *******
    181  ...    ..(  4 skipped).    ..  *******
    182  244    2017-10-06 03:52    26  *******
    183  245    2017-10-06 03:53    27  ********
    184  ...    ..( 11 skipped).    ..  ********
    185  257    2017-10-06 04:05    27  ********
    186  258    2017-10-06 04:06    26  *******
    187  ...    ..(  4 skipped).    ..  *******
    188  263    2017-10-06 04:11    26  *******
    189  264    2017-10-06 04:12    25  ******
    190  ...    ..( 24 skipped).    ..  ******
    191  289    2017-10-06 04:37    25  ******
    192  290    2017-10-06 04:38    26  *******
    193  291    2017-10-06 04:39    26  *******
    194  292    2017-10-06 04:40    26  *******
    195  293    2017-10-06 04:41    27  ********
    196  ...    ..( 13 skipped).    ..  ********
    197  307    2017-10-06 04:55    27  ********
    198  308    2017-10-06 04:56    26  *******
    199  ...    ..(  4 skipped).    ..  *******
    200  313    2017-10-06 05:01    26  *******
    201  314    2017-10-06 05:02    25  ******
    202  ...    ..( 31 skipped).    ..  ******
    203  346    2017-10-06 05:34    25  ******
    204  347    2017-10-06 05:35    26  *******
    205  ...    ..(  3 skipped).    ..  *******
    206  351    2017-10-06 05:39    26  *******
    207  352    2017-10-06 05:40    27  ********
    208  ...    ..( 18 skipped).    ..  ********
    209  371    2017-10-06 05:59    27  ********
    210  372    2017-10-06 06:00    26  *******
    211  ...    ..( 11 skipped).    ..  *******
    212  384    2017-10-06 06:12    26  *******
    213  385    2017-10-06 06:13    25  ******
    214  ...    ..( 10 skipped).    ..  ******
    215  396    2017-10-06 06:24    25  ******
    216  397    2017-10-06 06:25    26  *******
    217  ...    ..(  2 skipped).    ..  *******
    218  400    2017-10-06 06:28    26  *******
    219  401    2017-10-06 06:29    27  ********
    220  ...    ..( 20 skipped).    ..  ********
    221  422    2017-10-06 06:50    27  ********
    222  423    2017-10-06 06:51    26  *******
    223  ...    ..(  4 skipped).    ..  *******
    224  428    2017-10-06 06:56    26  *******
    225  429    2017-10-06 06:57    25  ******
    226  ...    ..( 23 skipped).    ..  ******
    227  453    2017-10-06 07:21    25  ******
    228  454    2017-10-06 07:22    26  *******
    229  455    2017-10-06 07:23    26  *******
    230  456    2017-10-06 07:24    26  *******
    231  457    2017-10-06 07:25    27  ********
    232  ...    ..( 13 skipped).    ..  ********
    233  471    2017-10-06 07:39    27  ********
    234  472    2017-10-06 07:40    26  *******
    235  ...    ..(  3 skipped).    ..  *******
    236  476    2017-10-06 07:44    26  *******
    237  477    2017-10-06 07:45    25  ******
    238  ...    ..( 33 skipped).    ..  ******
    239   33    2017-10-06 08:19    25  ******
    240   34    2017-10-06 08:20    26  *******
    241  ...    ..(  3 skipped).    ..  *******
    242   38    2017-10-06 08:24    26  *******
    243   39    2017-10-06 08:25    27  ********
    244  ...    ..( 13 skipped).    ..  ********
    245   53    2017-10-06 08:39    27  ********
    246   54    2017-10-06 08:40    26  *******
    247  ...    ..(  5 skipped).    ..  *******
    248   60    2017-10-06 08:46    26  *******
    249   61    2017-10-06 08:47    25  ******
    250  ...    ..( 20 skipped).    ..  ******
    251   82    2017-10-06 09:08    25  ******
    252   83    2017-10-06 09:09    26  *******
    253  ...    ..(  2 skipped).    ..  *******
    254   86    2017-10-06 09:12    26  *******
    255   87    2017-10-06 09:13    27  ********
    256  ...    ..( 12 skipped).    ..  ********
    257  100    2017-10-06 09:26    27  ********
    258  101    2017-10-06 09:27    26  *******
    259  ...    ..(  4 skipped).    ..  *******
    260  106    2017-10-06 09:32    26  *******
    261  107    2017-10-06 09:33    25  ******
    262  ...    ..( 21 skipped).    ..  ******
    263  129    2017-10-06 09:55    25  ******
    264  130    2017-10-06 09:56    26  *******
    265  131    2017-10-06 09:57    26  *******
    266  132    2017-10-06 09:58    26  *******
    267  133    2017-10-06 09:59    27  ********
    268  ...    ..( 13 skipped).    ..  ********
    269  147    2017-10-06 10:13    27  ********
    270  148    2017-10-06 10:14    26  *******
    271  ...    ..(  5 skipped).    ..  *******
    272  154    2017-10-06 10:20    26  *******
    273  155    2017-10-06 10:21    25  ******
    274  ...    ..( 23 skipped).    ..  ******
    275  179    2017-10-06 10:45    25  ******
    276 
    277 SCT Error Recovery Control:
    278            Read:     70 (7.0 seconds)
    279           Write:     70 (7.0 seconds)
     154Write SCT Data Table failed: Operation not supported
     155Read SCT Temperature History failed
     156
     157Write SCT (Get) Error Recovery Control Command failed: Operation not supported
     158SCT (Get) Error Recovery Control command failed
    280159
    281160Device Statistics (GP/SMART Log 0x04) not supported
    282161
    … …  
    2951740x000b  2            0  CRC errors within host-to-device FIS
    2961750x000f  2            0  R_ERR response for host-to-device data FIS, CRC
    2971760x0012  2            0  R_ERR response for host-to-device non-data FIS, CRC
    298 0x8000  4     91210322  Vendor specific
     1770x8000  4     91210329  Vendor specific

comment:8 by Alex Samorukov, 7 years ago

Related to #93

comment:9 by Alex Samorukov, 7 years ago

Okay, problem found.

Line memset(&io_hdr, 0, sizeof(struct sg_io_hdr)); should be memset(&io_hdr, 0, sizeof(struct sg_io_v4));. This fixing the issue and it works as expected.

I will add updated patch. One of the question i have - do we need to use it only for the /dev/bsgX or we can also try to enable it for the normal sdX devices? I think it will not be included to the upcoming 6.6 release, but is a good candidate for the 6.7.

comment:10 by Alexander K Austin, 7 years ago

The new code should only happen on /dev/bsg/* devices, because the sg_io autodetect attempts both access modes. sg_io_v4 only works on /dev/bsg/*, and sg_io_hdr only works on /dev/sg* devices.

by Alex Samorukov, 7 years ago

Attachment: sgio.patch​ added

Fixed patch against r4523

comment:11 by Alex Samorukov, 7 years ago

I tested the patch and it works good for me. I think we can accept it before new release.

I would like to include your name in the ChangeLog and NEWS file if you are ok with it.

comment:12 by Alex Samorukov, 7 years ago

I think i will refactor the patch to use sg_io_cmnd_io in the both cases but with respect to the SG_IO_PRESENT_V3/SG_IO_PRESENT_V4 global. This will allow to avoid code duplication caused by patch

in reply to:  12 comment:13 by Alexander K Austin, 7 years ago

I don't think trying to do both in one function is sensible; there are enough differences between the structures that I think doing so will result in more confusion, rather than less.

comment:14 by Christian Franke, 7 years ago

Milestone: undecided → Release 6.6

The combined code would require thorough regression tests of old functionality. Therefore I would suggest to leave possible refactoring for later and include the current patch into upcoming smartmontools 6.6.

comment:15 by Alex Samorukov, 7 years ago

Christian, i am working on updated patch. I think we can include it in 6.6, but will ask you for review. I will attach new patch version now, it works fine on my devices and i think it is preferable, because allows to avoid code duplication.

by Alex Samorukov, 7 years ago

Attachment: sgio.2.patch​ added

Updated SG_IO patch to use v3 and v4 api from the same function

comment:16 by Alex Samorukov, 7 years ago

Testing i did with a patch:

  1. Normal /dev/sdX for scsi and sata disks, no difference, V3 API is in use, as expected.
  2. /dev/sgX for scsi and sata disks, no difference, V3 API is in use, as expected.
  3. /deb/bsg/x:x:x:x devices (same systems, SATA and SCSI) - API V4 is in use, works as expected, no difference with V3 found. Tested -x, -t short, reading some logs, etc.

After all tests nothing was found in the kernel log, so fallback to the old ioctl was never happens

Why i think its better to use combined function:

  • Internally both ioctl-s are ending with a same kernel functions, for us difference is mostly about variable mapping.
  • This way we can avoid unnecessary code duplication and simplify debugging
Last edited 7 years ago by Alex Samorukov (previous) (diff)

comment:17 by Christian Franke, 7 years ago

Owner: set to Alex Samorukov
Status: new → assigned

OK. Patch looks good so far. Feel free to commit.

Circuitsoft: Please test ASAP, thanks.

comment:18 by Alex Samorukov, 7 years ago

Thank you for review, committed in r4554.

Circuitsoft: please test if it works for you and thank you for the patch.

comment:19 by Alex Samorukov, 7 years ago

Resolution: → fixed
Status: assigned → closed

in reply to:  18 comment:20 by Alexander K Austin, 7 years ago

Replying to samm2:

Thank you for review, committed in r4554.

Circuitsoft: please test if it works for you and thank you for the patch.

Unfortunately, I have changed jobs and no longer have access to the need for it. Based on your testing above, I believe it is good and would accept it.

Note: See TracTickets for help on using tickets.