Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-10 Thread Dale R. Worley
Seymour, Shane M shane.seym...@hp.com writes:
 Both of those things would have to be futures and require discussion -
 the very original version cleared stats on a device open but I got
 asked to keep it the stats cumulative so they would be more similar to
 disk stats.

Yes, this is a futures question.  But in a perfect world, there'd be two
sets of statistics, one cumulative since the last reboot and one that
was cleared every time a tape was unloaded.

Dale
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-09 Thread Kai Mäkisara (Kolumbus)

 On 9.2.2015, at 8.00, Seymour, Shane M shane.seym...@hp.com wrote:
 
 Kai - see last 3 paragraphs for question about if something is a bug or not.
 
 BTW I had a look - I couldn't quickly find out if there was a way to tell if 
 the medium has changed in a tape drive (there could be something device 
 specific). At the device level there's a record of I/O errors:
 
 [root@tapedrive device]# pwd
 /sys/class/scsi_tape/st0/device
 [root@tapedrive device]# cat ioerr_cnt
 0x5
  
 That doesn't distinguish between read or write or any other kind of error 
 though - it doesn't even really have to mean an error since reading with a 
 block size too small also increments the error count:

The counts in the device directory are not specific to tapes. From 
linux/scsi/scsi_lib.c one can see that ierr_cnt is incremented every time the 
scsi call returns any kind of error, including when the tape drive wants to 
return some information with sense data.

 [root@tapedrive tape-stats]# ./tape_exercise write /dev/st0 /dev/urandom 
 100
 About to write from /dev/urandom to /dev/st0, max size = 100, blksize = 
 4096
 Write complete on /dev/st0 after 1003520 bytes
 ./tape_[root@tapedrive tape-stats]# ./tape_exercise read /dev/st0
 About to read from /dev/st0 blksize = 256
 Failed to read from /dev/st0 using current blksize, will try using a bigger 
 blksize
 About to read from /dev/st0 blksize = 512
 Failed to read from /dev/st0 using current blksize, will try using a bigger 
 blksize
 About to read from /dev/st0 blksize = 1024
 Failed to read from /dev/st0 using current blksize, will try using a bigger 
 blksize
 About to read from /dev/st0 blksize = 2048
 Failed to read from /dev/st0 using current blksize, will try using a bigger 
 blksize
 About to read from /dev/st0 blksize = 4096
 Reading complete for /dev/st0, 987136 bytes read
 
 [root@tapedrive tape-stats]# cd /sys/class/scsi_tape/st0/device
 [root@tapedrive device]# cat ioerr_cnt
 0xa
 
 It would seem that ioerr_cnt is probably a count of SCSI check conditions 
 encountered?
 
 Unfortunately for the MTIOCGET ioctl the value of mt_resid is the partition 
 number of the tape not what I would have expected it to be - the residual 
 left after the last read or write that returned an error (and 0 if the last 
 read/write didn't return an error).
 
 Kai - is that a bug? Shouldn't mt_resid be the residual from the last failed 
 read or write indicating how much data didn't make it to the device and 0 on 
 a successful read or write? I've used mt_resid from MTIOCGET on HP-UX 
 previously when issuing reads and repositioning and retrying tape reads when 
 starting with too low a block size to try and automatically determine the 
 block size in use (it's never a good idea to under or overread tape blocks 
 because it always results in check conditions and in the st driver under 
 reading the block size always creates messages in dmesg).
 
This is not a bug. man st documents that mt_resid does return the partition 
number. In the different unix systems the field did not consistently return the 
residual count. The Linux SCSI drivers did not at that time return the 
residual. These are reasons why the field is used for partition number.

For writes in any real situation (drive in buffered mode) you don’t know when 
the write() returns whether the data can be written to tape. All writes are on 
tape when write file marks returns successfully of the device is successfully 
closed. I am not sure that the amount of data successfully written is really 
useful. If I see a write error, I want to rewrite at least the latest file.

For reads, there are other ways to determine the block size. (Use a large 
enough byte count and see what you get.)

The st driver does not write to the log if the block is shorter than requested. 
Short read is logged (together with the real block size). If you don’t want the 
overhead of returning the sense data for short reads, you can set the SILI flag.

 If you don't have time to look at it I may look at if it's possible to 
 provide the correct mt_resid for MTIOCGET - assuming that a long time if 
 misuse prevents us from correcting it. If there's no way to export a 
 partition number for the devices that support it I can add a new sysfs file 
 (call it partition) to export it that way and see if I can get the correct 
 value into mt_resid.
 
Changing the definition of a field should not be done. There is software that 
is correctly using the field as it is documented.

Kai

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-08 Thread Dale R. Worley
One feature of tape statistics is that they're as much about the *tape*
as they are about the *drive*, which is uncommon for block devices.  It
might be useful to have a set of counters which is cleared every time a
new tape is inserted into the drive.  In particular, bad reads since
this tape was inserted would be very useful for monitoring the quality
of tapes.

Dale
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] st: implement sysfs based tape statistics v2

2015-02-08 Thread Seymour, Shane M
Kai - see last 3 paragraphs for question about if something is a bug or not.

BTW I had a look - I couldn't quickly find out if there was a way to tell if 
the medium has changed in a tape drive (there could be something device 
specific). At the device level there's a record of I/O errors:

[root@tapedrive device]# pwd
/sys/class/scsi_tape/st0/device
[root@tapedrive device]# cat ioerr_cnt
0x5

That doesn't distinguish between read or write or any other kind of error 
though - it doesn't even really have to mean an error since reading with a 
block size too small also increments the error count:

[root@tapedrive tape-stats]# ./tape_exercise write /dev/st0 /dev/urandom 100
About to write from /dev/urandom to /dev/st0, max size = 100, blksize = 4096
Write complete on /dev/st0 after 1003520 bytes
./tape_[root@tapedrive tape-stats]# ./tape_exercise read /dev/st0
About to read from /dev/st0 blksize = 256
Failed to read from /dev/st0 using current blksize, will try using a bigger 
blksize
About to read from /dev/st0 blksize = 512
Failed to read from /dev/st0 using current blksize, will try using a bigger 
blksize
About to read from /dev/st0 blksize = 1024
Failed to read from /dev/st0 using current blksize, will try using a bigger 
blksize
About to read from /dev/st0 blksize = 2048
Failed to read from /dev/st0 using current blksize, will try using a bigger 
blksize
About to read from /dev/st0 blksize = 4096
Reading complete for /dev/st0, 987136 bytes read

[root@tapedrive tape-stats]# cd /sys/class/scsi_tape/st0/device
[root@tapedrive device]# cat ioerr_cnt
0xa

It would seem that ioerr_cnt is probably a count of SCSI check conditions 
encountered?

Unfortunately for the MTIOCGET ioctl the value of mt_resid is the partition 
number of the tape not what I would have expected it to be - the residual left 
after the last read or write that returned an error (and 0 if the last 
read/write didn't return an error).

Kai - is that a bug? Shouldn't mt_resid be the residual from the last failed 
read or write indicating how much data didn't make it to the device and 0 on a 
successful read or write? I've used mt_resid from MTIOCGET on HP-UX previously 
when issuing reads and repositioning and retrying tape reads when starting with 
too low a block size to try and automatically determine the block size in use 
(it's never a good idea to under or overread tape blocks because it always 
results in check conditions and in the st driver under reading the block size 
always creates messages in dmesg).

If you don't have time to look at it I may look at if it's possible to provide 
the correct mt_resid for MTIOCGET - assuming that a long time if misuse 
prevents us from correcting it. If there's no way to export a partition number 
for the devices that support it I can add a new sysfs file (call it partition) 
to export it that way and see if I can get the correct value into mt_resid.

-Original Message-
From: Seymour, Shane M 
Sent: Monday, February 09, 2015 10:19 AM
To: 'Dale R. Worley'
Cc: linux-scsi@vger.kernel.org
Subject: RE: [PATCH] st: implement sysfs based tape statistics v2

Both of those things would have to be futures and require discussion - the very 
original version cleared stats on a device open but I got asked to keep it the 
stats cumulative so they would be more similar to disk stats. I'm not sure 
about the bad reads idea I'd have to look and see if some other layer provided 
device error information. I've got some changes to make and don't want to 
change it into a moving target that needs more discussion at this point.

-Original Message-
From: Dale R. Worley [mailto:wor...@alum.mit.edu] 
Sent: Monday, February 09, 2015 4:08 AM
To: Seymour, Shane M
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH] st: implement sysfs based tape statistics v2

One feature of tape statistics is that they're as much about the *tape* as they 
are about the *drive*, which is uncommon for block devices.  It might be useful 
to have a set of counters which is cleared every time a new tape is inserted 
into the drive.  In particular, bad reads since this tape was inserted would 
be very useful for monitoring the quality of tapes.

Dale
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] st: implement sysfs based tape statistics v2

2015-02-08 Thread Seymour, Shane M
Both of those things would have to be futures and require discussion - the very 
original version cleared stats on a device open but I got asked to keep it the 
stats cumulative so they would be more similar to disk stats. I'm not sure 
about the bad reads idea I'd have to look and see if some other layer provided 
device error information. I've got some changes to make and don't want to 
change it into a moving target that needs more discussion at this point.

-Original Message-
From: Dale R. Worley [mailto:wor...@alum.mit.edu] 
Sent: Monday, February 09, 2015 4:08 AM
To: Seymour, Shane M
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH] st: implement sysfs based tape statistics v2

One feature of tape statistics is that they're as much about the *tape* as they 
are about the *drive*, which is uncommon for block devices.  It might be useful 
to have a set of counters which is cleared every time a new tape is inserted 
into the drive.  In particular, bad reads since this tape was inserted would 
be very useful for monitoring the quality of tapes.

Dale
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-06 Thread Jeremy Linton
On 1/26/2015 6:11 PM, Seymour, Shane M wrote:
 I was wondering if anyone had any feedback or had any chance to review the 
 changes?

Per the other discussion about having the same stat format forever. It 
seems to
me that you might want to preemptively add a few additional counters.

A counter for WRITE_FILEMARKS, particularly non immediate count=0 ones, which
are often used to flush the drive write buffer. A counter for movement related
commands like SPACE/LOCATE/REWIND would also be helpful. Finally, abnormal read
conditions like, ILI's, and hit FMs should have their own stat. Those three
should provide a better view into how the drive is being used and why
performance may not be what is expected.

There may be others, but those three are high on my list of things I want to
know about a tape stream that is not performing up to expectations.





--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-05 Thread Kai Mäkisara (Kolumbus)

 On 2.2.2015, at 17.16, Laurence Oberman oberma...@gmail.com wrote:
 
 I pulled this this morning and will be testing. The prior version was
 stable for me on the upstream and RHEL 6.5 kernel without exhaustive
 testing.
 We also just received more requests to get this into RHEL from HP /
 Red Hat customers.
 
 Kai, what are your thoughts. I realize this is a large amount of
 additional code. I am not keen to create a driver just for stats as we
 would have to keep the rest of the st driver changes always in sync.
 

I still think that the tape statistics should be exported like the statistics 
of “real” block devices, i.e., one sysfs file exporting on a single line the 
statistics that temporally belong together. James rejected this approach. I am 
leaving the decision about this code to him. I will neither ack nor nak this 
code.

Thanks,
Kai

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-05 Thread Laurence Oberman
- Original Message -
From: Kai Mäkisara (Kolumbus) kai.makis...@kolumbus.fi
To: Laurence Oberman oberma...@gmail.com
Cc: Shane M Seymour shane.seym...@hp.com, lober...@redhat.com, 
linux-scsi@vger.kernel.org, James E.J. Bottomley (jbottom...@parallels.com) 
jbottom...@parallels.com, je...@suse.com
Sent: Thursday, February 5, 2015 12:03:29 PM
Subject: Re: [PATCH] st: implement sysfs based tape statistics v2


 On 2.2.2015, at 17.16, Laurence Oberman oberma...@gmail.com wrote:
 
 I pulled this this morning and will be testing. The prior version was
 stable for me on the upstream and RHEL 6.5 kernel without exhaustive
 testing.
 We also just received more requests to get this into RHEL from HP /
 Red Hat customers.
 
 Kai, what are your thoughts. I realize this is a large amount of
 additional code. I am not keen to create a driver just for stats as we
 would have to keep the rest of the st driver changes always in sync.
 

I still think that the tape statistics should be exported like the statistics 
of “real” block devices, i.e., one sysfs file exporting on a single line the 
statistics that temporally belong together. James rejected this approach. I am 
leaving the decision about this code to him. I will neither ack nor nak this 
code.

Thanks,
Kai

Hello Kai,

I missed the earlier conversations with James, I will go search for them.
Do you mean add them so they are similar to the /proc/diskstats

cat /proc/diskstats
..
   8   0 sda 2258346 152801 291907067 5263795 388817 1518048 15013833 
4542062 0 4794931 9803495
   8   1 sda1 717 102 26154 1179 8 2 80 76 0 1172 1254
   8   2 sda2 328 31 2872 1554 0 0 0 0 0 1554 1554
   8   3 sda3 2195205 151617 290898283 5203627 355053 1518046 15009528 
4370598 0 4594137 9571937
   8   4 sda4 61921 1050 978350 57218 18 0 4225 34 0 56384 57185
  11   0 sr0 0 0 0 0 0 0 0 0 0 0 0
..


Laurence Oberman
Red Hat Global Support Service
SEG Team

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-05 Thread Kai Mäkisara (Kolumbus)

 On 5.2.2015, at 19.40, Laurence Oberman lober...@redhat.com wrote:
 
 - Original Message -
 From: Kai Mäkisara (Kolumbus) kai.makis...@kolumbus.fi
 To: Laurence Oberman oberma...@gmail.com
 Cc: Shane M Seymour shane.seym...@hp.com, lober...@redhat.com, 
 linux-scsi@vger.kernel.org, James E.J. Bottomley (jbottom...@parallels.com) 
 jbottom...@parallels.com, je...@suse.com
 Sent: Thursday, February 5, 2015 12:03:29 PM
 Subject: Re: [PATCH] st: implement sysfs based tape statistics v2
 
 
 On 2.2.2015, at 17.16, Laurence Oberman oberma...@gmail.com wrote:
 
 I pulled this this morning and will be testing. The prior version was
 stable for me on the upstream and RHEL 6.5 kernel without exhaustive
 testing.
 We also just received more requests to get this into RHEL from HP /
 Red Hat customers.
 
 Kai, what are your thoughts. I realize this is a large amount of
 additional code. I am not keen to create a driver just for stats as we
 would have to keep the rest of the st driver changes always in sync.
 
 
 I still think that the tape statistics should be exported like the statistics 
 of “real” block devices, i.e., one sysfs file exporting on a single line the 
 statistics that temporally belong together. James rejected this approach. I 
 am leaving the decision about this code to him. I will neither ack nor nak 
 this code.
 
 Thanks,
 Kai
 
 Hello Kai,
 
 I missed the earlier conversations with James, I will go search for them.
 Do you mean add them so they are similar to the /proc/diskstats
 
 cat /proc/diskstats
 ..
   8   0 sda 2258346 152801 291907067 5263795 388817 1518048 15013833 
 4542062 0 4794931 9803495
   8   1 sda1 717 102 26154 1179 8 2 80 76 0 1172 1254
   8   2 sda2 328 31 2872 1554 0 0 0 0 0 1554 1554
   8   3 sda3 2195205 151617 290898283 5203627 355053 1518046 15009528 
 4370598 0 4594137 9571937
   8   4 sda4 61921 1050 978350 57218 18 0 4225 34 0 56384 57185
  11   0 sr0 0 0 0 0 0 0 0 0 0 0 0
 ..
 
Not exactly. I mean the data exported in sysfs, for example:

 cat /sys/block/sda/sda1/stat
  159740 9006  594150664461   12472455907 12772208  3598677
0   299875  3663235

Kai

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-05 Thread James Bottomley
On Thu, 2015-02-05 at 19:46 +0200, Kai Mäkisara (Kolumbus) wrote:
  On 5.2.2015, at 19.40, Laurence Oberman lober...@redhat.com wrote:
  I missed the earlier conversations with James, I will go search for them.
  Do you mean add them so they are similar to the /proc/diskstats
  
  cat /proc/diskstats
  ..
8   0 sda 2258346 152801 291907067 5263795 388817 1518048 15013833 
  4542062 0 4794931 9803495
8   1 sda1 717 102 26154 1179 8 2 80 76 0 1172 1254
8   2 sda2 328 31 2872 1554 0 0 0 0 0 1554 1554
8   3 sda3 2195205 151617 290898283 5203627 355053 1518046 15009528 
  4370598 0 4594137 9571937
8   4 sda4 61921 1050 978350 57218 18 0 4225 34 0 56384 57185
   11   0 sr0 0 0 0 0 0 0 0 0 0 0 0
  ..
  
 Not exactly. I mean the data exported in sysfs, for example:
 
  cat /sys/block/sda/sda1/stat
   159740 9006  594150664461   12472455907 12772208  3598677   
  0   299875  3663235

The problem is we're going to be attacked by the sysfs one value per
file crowd if we do something like this.  Block gets away with it
because it was grandfathered in.  It's only 12 files ... if there's a
good reason for having the fight with the sysfs people, we can; however,
I haven't seen a good reason yet, so I'd rather avoid creating a fuss
over something we're going to lose.

James


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-05 Thread Bryn M. Reeves
On Thu, Feb 05, 2015 at 07:46:32PM +0200, Kai Mäkisara (Kolumbus) wrote:
  On 5.2.2015, at 19.40, Laurence Oberman lober...@redhat.com wrote:
  From: Kai Mäkisara (Kolumbus) kai.makis...@kolumbus.fi
  I still think that the tape statistics should be exported like the 
  statistics of “real” block devices, i.e., one sysfs file exporting on a 
  single line the statistics that temporally belong together. James rejected 
  this approach. I am leaving the decision about this code to him. I will 
  neither ack nor nak this code.
  
  I missed the earlier conversations with James, I will go search for them.
  Do you mean add them so they are similar to the /proc/diskstats

http://comments.gmane.org/gmane.linux.scsi/80497

On Fri, Feb 22 2013 James Bottomley wrote:
 I'm afraid we can't do it the way you're proposing.  files in sysfs must
 conform to the one value per file rule (so we avoid the ABI nastiness
 that plagues /proc).  You can create a stat directory with a bunch of
 files, but not a single file that gives all values.

Documentation/filesystems/sysfs.txt does not agree:

  Attributes should be ASCII text files, preferably with only one value
  per file. It is noted that it may not be efficient to contain only one
  value per file, so it is socially acceptable to express an array of
  values of the same type.

There's also ample precedent for this: sysfs disk and partition stats,
SELinux cache and hash table stats (which have a pretty yucky 2d int
array with column headers and a name: val format respectively).

There's also a bunch of multivariate name=value format stats files in
the cgroups sysfs tree.

 Not exactly. I mean the data exported in sysfs, for example:
 
  cat /sys/block/sda/sda1/stat
   159740 9006  594150664461   12472455907 12772208  3598677   
  0   299875  3663235

I'd prefer to consume tape stats in this format too; it follows the
principle of least surprise since it's shared with every other IO stats
source (including device-mapper statistics) and it simplifies handling
the counters in user space.

Regards,
Bryn.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-05 Thread James Bottomley
On Thu, 2015-02-05 at 18:50 +, Bryn M. Reeves wrote:
 On Thu, Feb 05, 2015 at 07:46:32PM +0200, Kai Mäkisara (Kolumbus) wrote:
   On 5.2.2015, at 19.40, Laurence Oberman lober...@redhat.com wrote:
   From: Kai Mäkisara (Kolumbus) kai.makis...@kolumbus.fi
   I still think that the tape statistics should be exported like the 
   statistics of “real” block devices, i.e., one sysfs file exporting on a 
   single line the statistics that temporally belong together. James 
   rejected this approach. I am leaving the decision about this code to him. 
   I will neither ack nor nak this code.
   
   I missed the earlier conversations with James, I will go search for them.
   Do you mean add them so they are similar to the /proc/diskstats
 
 http://comments.gmane.org/gmane.linux.scsi/80497
 
 On Fri, Feb 22 2013 James Bottomley wrote:
  I'm afraid we can't do it the way you're proposing.  files in sysfs must
  conform to the one value per file rule (so we avoid the ABI nastiness
  that plagues /proc).  You can create a stat directory with a bunch of
  files, but not a single file that gives all values.
 
 Documentation/filesystems/sysfs.txt does not agree:
 
   Attributes should be ASCII text files, preferably with only one value
   per file. It is noted that it may not be efficient to contain only one
   value per file, so it is socially acceptable to express an array of
   values of the same type.
 
 There's also ample precedent for this: sysfs disk and partition stats,
 SELinux cache and hash table stats (which have a pretty yucky 2d int
 array with column headers and a name: val format respectively).
 
 There's also a bunch of multivariate name=value format stats files in
 the cgroups sysfs tree.
 
  Not exactly. I mean the data exported in sysfs, for example:
  
   cat /sys/block/sda/sda1/stat
159740 9006  594150664461   12472455907 12772208  3598677 
 0   299875  3663235
 
 I'd prefer to consume tape stats in this format too; it follows the
 principle of least surprise since it's shared with every other IO stats
 source (including device-mapper statistics) and it simplifies handling
 the counters in user space.

OK, the sysfs bikeshedders hang out on linux-api

https://www.kernel.org/doc/man-pages/linux-api-ml.html

If you can convince them, we'll do the single file approach.

James


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-05 Thread Bryn M. Reeves
On Thu, Feb 05, 2015 at 10:55:50AM -0800, James Bottomley wrote:
 OK, the sysfs bikeshedders hang out on linux-api
 
 https://www.kernel.org/doc/man-pages/linux-api-ml.html
 
 If you can convince them, we'll do the single file approach.

Will do - I've got a couple of stats projects on the go at the moment so
this ties in well with that.

I'll sync up with Shane and see if he's interested in running the int array
version via the linux-api folks.

Regards,
Bryn.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-05 Thread Laurence Oberman
- Original Message -
From: Kai Mäkisara (Kolumbus) kai.makis...@kolumbus.fi
To: Laurence Oberman lober...@redhat.com
Cc: Laurence Oberman oberma...@gmail.com, Shane M Seymour 
shane.seym...@hp.com, linux-scsi@vger.kernel.org, James E.J. Bottomley 
(jbottom...@parallels.com) jbottom...@parallels.com, je...@suse.com
Sent: Thursday, February 5, 2015 12:46:32 PM
Subject: Re: [PATCH] st: implement sysfs based tape statistics v2


 On 5.2.2015, at 19.40, Laurence Oberman lober...@redhat.com wrote:
 
 - Original Message -
 From: Kai Mäkisara (Kolumbus) kai.makis...@kolumbus.fi
 To: Laurence Oberman oberma...@gmail.com
 Cc: Shane M Seymour shane.seym...@hp.com, lober...@redhat.com, 
 linux-scsi@vger.kernel.org, James E.J. Bottomley (jbottom...@parallels.com) 
 jbottom...@parallels.com, je...@suse.com
 Sent: Thursday, February 5, 2015 12:03:29 PM
 Subject: Re: [PATCH] st: implement sysfs based tape statistics v2
 
 
 On 2.2.2015, at 17.16, Laurence Oberman oberma...@gmail.com wrote:
 
 I pulled this this morning and will be testing. The prior version was
 stable for me on the upstream and RHEL 6.5 kernel without exhaustive
 testing.
 We also just received more requests to get this into RHEL from HP /
 Red Hat customers.
 
 Kai, what are your thoughts. I realize this is a large amount of
 additional code. I am not keen to create a driver just for stats as we
 would have to keep the rest of the st driver changes always in sync.
 
 
 I still think that the tape statistics should be exported like the statistics 
 of “real” block devices, i.e., one sysfs file exporting on a single line the 
 statistics that temporally belong together. James rejected this approach. I 
 am leaving the decision about this code to him. I will neither ack nor nak 
 this code.
 
 Thanks,
 Kai
 
 Hello Kai,
 
 I missed the earlier conversations with James, I will go search for them.
 Do you mean add them so they are similar to the /proc/diskstats
 
 cat /proc/diskstats
 ..
   8   0 sda 2258346 152801 291907067 5263795 388817 1518048 15013833 
 4542062 0 4794931 9803495
   8   1 sda1 717 102 26154 1179 8 2 80 76 0 1172 1254
   8   2 sda2 328 31 2872 1554 0 0 0 0 0 1554 1554
   8   3 sda3 2195205 151617 290898283 5203627 355053 1518046 15009528 
 4370598 0 4594137 9571937
   8   4 sda4 61921 1050 978350 57218 18 0 4225 34 0 56384 57185
  11   0 sr0 0 0 0 0 0 0 0 0 0 0 0
 ..
 
Not exactly. I mean the data exported in sysfs, for example:

 cat /sys/block/sda/sda1/stat
  159740 9006  594150664461   12472455907 12772208  3598677
0   299875  3663235

Kai

Ok, Thanks, got it now. Let me circle back with Shane

Laurence Oberman
Red Hat Global Support Service
SEG Team
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: implement sysfs based tape statistics v2

2015-02-02 Thread Laurence Oberman
I pulled this this morning and will be testing. The prior version was
stable for me on the upstream and RHEL 6.5 kernel without exhaustive
testing.
We also just received more requests to get this into RHEL from HP /
Red Hat customers.

Kai, what are your thoughts. I realize this is a large amount of
additional code. I am not keen to create a driver just for stats as we
would have to keep the rest of the st driver changes always in sync.

Thanks
Laurence

On Mon, Jan 12, 2015 at 10:43 PM, Seymour, Shane M shane.seym...@hp.com wrote:
 Some small changes since the last version - this version removes two files 
 from sysfs compared to the last version (read and write block counts since 
 they're derived from the byte counts they can be calculated in user space) 
 but that's the only change. This version has been rebased to 
 3.19.0-rc3-next-20150108.

 Since posting the last version an email was received by Kai and myself from 
 an ATT employee who has a need for tape statistics to be implemented (who 
 gave permission to quote their email), I've included part of the email here:

 There are over 20,000 tape devices managed by our operations group zoned to 
 tens of thousands of servers.

 My challenge is that I cannot provide operations a solution that gives them 
 visibility into the tape drive performance metrics when that platform is 
 linux. Our legacy platforms (Solaris,HPUX,AIX) provide facilities to use 
 iostat and sar to determine the write speed of the tape drives. We took for 
 granted that this would be available in linux and its absence has been very 
 troublesome. Because operations cannot measure tape drive performance in this 
 way they cannot easily determine when there is a tape drive performance 
 problem and whether the change improved or worsened the performance problem.
 ...
 I have followed the debate https://lkml.org/lkml/2013/3/20/696 and from a 
 service provide perspective we would expect the same maturity and 
 functionality that we have from the traditional unix platform in regards to 
 iostat/sar. This issue is important and urgent because tape drive performance 
 issues are common and I am unable to provide standards and processes to 
 identify and remediate these issues.

 Another HP customer has also requested the same functionality (but hasn't 
 given permission to be named), they own tape drives numbering in the 1000s 
 and also need the ability to investigate performance issues.

 Signed-off-by: shane.seym...@hp.com
 Tested-by: shane.seym...@hp.com
 ---
 diff -uprN a/drivers/scsi/st.c b/drivers/scsi/st.c
 --- a/drivers/scsi/st.c 2015-01-11 14:46:00.243814755 -0600
 +++ b/drivers/scsi/st.c 2015-01-12 13:54:52.549117333 -0600
 @@ -20,6 +20,7 @@
  static const char *verstr = 20101219;

  #include linux/module.h
 +#include linux/kobject.h

  #include linux/fs.h
  #include linux/kernel.h
 @@ -226,6 +227,20 @@ static DEFINE_SPINLOCK(st_index_lock);
  static DEFINE_SPINLOCK(st_use_lock);
  static DEFINE_IDR(st_index_idr);

 +static inline void st_stats_remove_files(struct scsi_tape *);
 +static inline void st_stats_create_files(struct scsi_tape *);
 +static ssize_t st_tape_attr_show(struct kobject *, struct attribute *, char 
 *);
 +static ssize_t st_tape_attr_store(struct kobject *, struct attribute *,
 +   const char *, size_t);
 +static void st_release_stats_kobj(struct kobject *);
 +static const struct sysfs_ops st_stats_sysfs_ops = {
 +   .show   = st_tape_attr_show,
 +   .store  = st_tape_attr_store,
 +};
 +static struct kobj_type st_stats_ktype = {
 +   .release = st_release_stats_kobj,
 +   .sysfs_ops = st_stats_sysfs_ops,
 +};



  #include osst_detect.h
 @@ -476,10 +491,22 @@ static void st_scsi_execute_end(struct r
 struct st_request *SRpnt = req-end_io_data;
 struct scsi_tape *STp = SRpnt-stp;
 struct bio *tmp;
 +   u64 ticks;

 STp-buffer-cmdstat.midlevel_result = SRpnt-result = req-errors;
 STp-buffer-cmdstat.residual = req-resid_len;

 +   if (STp-stats != NULL) {
 +   ticks = get_jiffies_64();
 +   STp-stats-in_flight--;
 +   ticks -= STp-stats-stamp;
 +   STp-stats-io_ticks += ticks;
 +   if (req-cmd[0] == WRITE_6)
 +   STp-stats-write_ticks += ticks;
 +   else if (req-cmd[0] == READ_6)
 +   STp-stats-read_ticks += ticks;
 +   }
 +
 tmp = SRpnt-bio;
 if (SRpnt-waiting)
 complete(SRpnt-waiting);
 @@ -496,6 +523,7 @@ static int st_scsi_execute(struct st_req
 struct rq_map_data *mdata = SRpnt-stp-buffer-map_data;
 int err = 0;
 int write = (data_direction == DMA_TO_DEVICE);
 +   struct scsi_tape *STp = SRpnt-stp;

 req = blk_get_request(SRpnt-stp-device-request_queue, write,
   GFP_KERNEL);
 @@ -516,6 +544,20 @@ static int st_scsi_execute(struct st_req
 }
 }

 +   

RE: [PATCH] st: implement sysfs based tape statistics v2

2015-01-26 Thread Seymour, Shane M
I was wondering if anyone had any feedback or had any chance to review the 
changes?

-Original Message-
From: linux-scsi-ow...@vger.kernel.org 
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Seymour, Shane M
Sent: Tuesday, January 13, 2015 2:43 PM
To: linux-scsi@vger.kernel.org
Cc: kai.makis...@kolumbus.fi; James E.J. Bottomley (jbottom...@parallels.com); 
je...@suse.com
Subject: [PATCH] st: implement sysfs based tape statistics v2

Some small changes since the last version - this version removes two files from 
sysfs compared to the last version (read and write block counts since they're 
derived from the byte counts they can be calculated in user space) but that's 
the only change. This version has been rebased to 3.19.0-rc3-next-20150108.

Since posting the last version an email was received by Kai and myself from an 
ATT employee who has a need for tape statistics to be implemented (who gave 
permission to quote their email), I've included part of the email here:

There are over 20,000 tape devices managed by our operations group zoned to 
tens of thousands of servers.

My challenge is that I cannot provide operations a solution that gives them 
visibility into the tape drive performance metrics when that platform is linux. 
Our legacy platforms (Solaris,HPUX,AIX) provide facilities to use iostat and 
sar to determine the write speed of the tape drives. We took for granted that 
this would be available in linux and its absence has been very troublesome. 
Because operations cannot measure tape drive performance in this way they 
cannot easily determine when there is a tape drive performance problem and 
whether the change improved or worsened the performance problem.
...
I have followed the debate https://lkml.org/lkml/2013/3/20/696 and from a 
service provide perspective we would expect the same maturity and functionality 
that we have from the traditional unix platform in regards to iostat/sar. This 
issue is important and urgent because tape drive performance issues are common 
and I am unable to provide standards and processes to identify and remediate 
these issues.

Another HP customer has also requested the same functionality (but hasn't given 
permission to be named), they own tape drives numbering in the 1000s and also 
need the ability to investigate performance issues.

Signed-off-by: shane.seym...@hp.com
Tested-by: shane.seym...@hp.com
---
diff -uprN a/drivers/scsi/st.c b/drivers/scsi/st.c
--- a/drivers/scsi/st.c 2015-01-11 14:46:00.243814755 -0600
+++ b/drivers/scsi/st.c 2015-01-12 13:54:52.549117333 -0600
@@ -20,6 +20,7 @@
 static const char *verstr = 20101219;
 
 #include linux/module.h
+#include linux/kobject.h
 
 #include linux/fs.h
 #include linux/kernel.h
@@ -226,6 +227,20 @@ static DEFINE_SPINLOCK(st_index_lock);
 static DEFINE_SPINLOCK(st_use_lock);
 static DEFINE_IDR(st_index_idr);
 
+static inline void st_stats_remove_files(struct scsi_tape *);
+static inline void st_stats_create_files(struct scsi_tape *);
+static ssize_t st_tape_attr_show(struct kobject *, struct attribute *, char *);
+static ssize_t st_tape_attr_store(struct kobject *, struct attribute *,
+   const char *, size_t);
+static void st_release_stats_kobj(struct kobject *);
+static const struct sysfs_ops st_stats_sysfs_ops = {
+   .show   = st_tape_attr_show,
+   .store  = st_tape_attr_store,
+};
+static struct kobj_type st_stats_ktype = {
+   .release = st_release_stats_kobj,
+   .sysfs_ops = st_stats_sysfs_ops,
+};
 
 

 #include osst_detect.h
@@ -476,10 +491,22 @@ static void st_scsi_execute_end(struct r
struct st_request *SRpnt = req-end_io_data;
struct scsi_tape *STp = SRpnt-stp;
struct bio *tmp;
+   u64 ticks;
 
STp-buffer-cmdstat.midlevel_result = SRpnt-result = req-errors;
STp-buffer-cmdstat.residual = req-resid_len;
 
+   if (STp-stats != NULL) {
+   ticks = get_jiffies_64();
+   STp-stats-in_flight--;
+   ticks -= STp-stats-stamp;
+   STp-stats-io_ticks += ticks;
+   if (req-cmd[0] == WRITE_6)
+   STp-stats-write_ticks += ticks;
+   else if (req-cmd[0] == READ_6)
+   STp-stats-read_ticks += ticks;
+   }
+
tmp = SRpnt-bio;
if (SRpnt-waiting)
complete(SRpnt-waiting);
@@ -496,6 +523,7 @@ static int st_scsi_execute(struct st_req
struct rq_map_data *mdata = SRpnt-stp-buffer-map_data;
int err = 0;
int write = (data_direction == DMA_TO_DEVICE);
+   struct scsi_tape *STp = SRpnt-stp;
 
req = blk_get_request(SRpnt-stp-device-request_queue, write,
  GFP_KERNEL);
@@ -516,6 +544,20 @@ static int st_scsi_execute(struct st_req
}
}
 
+   if (STp-stats != NULL) {
+   if (cmd[0] == WRITE_6) {
+   STp-stats-write_cnt++;
+   STp-stats-write_byte_cnt

[PATCH] st: implement sysfs based tape statistics v2

2015-01-12 Thread Seymour, Shane M
Some small changes since the last version - this version removes two files from 
sysfs compared to the last version (read and write block counts since they're 
derived from the byte counts they can be calculated in user space) but that's 
the only change. This version has been rebased to 3.19.0-rc3-next-20150108.

Since posting the last version an email was received by Kai and myself from an 
ATT employee who has a need for tape statistics to be implemented (who gave 
permission to quote their email), I've included part of the email here:

There are over 20,000 tape devices managed by our operations group zoned to 
tens of thousands of servers.

My challenge is that I cannot provide operations a solution that gives them 
visibility into the tape drive performance metrics when that platform is linux. 
Our legacy platforms (Solaris,HPUX,AIX) provide facilities to use iostat and 
sar to determine the write speed of the tape drives. We took for granted that 
this would be available in linux and its absence has been very troublesome. 
Because operations cannot measure tape drive performance in this way they 
cannot easily determine when there is a tape drive performance problem and 
whether the change improved or worsened the performance problem.
...
I have followed the debate https://lkml.org/lkml/2013/3/20/696 and from a 
service provide perspective we would expect the same maturity and functionality 
that we have from the traditional unix platform in regards to iostat/sar. This 
issue is important and urgent because tape drive performance issues are common 
and I am unable to provide standards and processes to identify and remediate 
these issues.

Another HP customer has also requested the same functionality (but hasn't given 
permission to be named), they own tape drives numbering in the 1000s and also 
need the ability to investigate performance issues.

Signed-off-by: shane.seym...@hp.com
Tested-by: shane.seym...@hp.com
---
diff -uprN a/drivers/scsi/st.c b/drivers/scsi/st.c
--- a/drivers/scsi/st.c 2015-01-11 14:46:00.243814755 -0600
+++ b/drivers/scsi/st.c 2015-01-12 13:54:52.549117333 -0600
@@ -20,6 +20,7 @@
 static const char *verstr = 20101219;
 
 #include linux/module.h
+#include linux/kobject.h
 
 #include linux/fs.h
 #include linux/kernel.h
@@ -226,6 +227,20 @@ static DEFINE_SPINLOCK(st_index_lock);
 static DEFINE_SPINLOCK(st_use_lock);
 static DEFINE_IDR(st_index_idr);
 
+static inline void st_stats_remove_files(struct scsi_tape *);
+static inline void st_stats_create_files(struct scsi_tape *);
+static ssize_t st_tape_attr_show(struct kobject *, struct attribute *, char *);
+static ssize_t st_tape_attr_store(struct kobject *, struct attribute *,
+   const char *, size_t);
+static void st_release_stats_kobj(struct kobject *);
+static const struct sysfs_ops st_stats_sysfs_ops = {
+   .show   = st_tape_attr_show,
+   .store  = st_tape_attr_store,
+};
+static struct kobj_type st_stats_ktype = {
+   .release = st_release_stats_kobj,
+   .sysfs_ops = st_stats_sysfs_ops,
+};
 
 

 #include osst_detect.h
@@ -476,10 +491,22 @@ static void st_scsi_execute_end(struct r
struct st_request *SRpnt = req-end_io_data;
struct scsi_tape *STp = SRpnt-stp;
struct bio *tmp;
+   u64 ticks;
 
STp-buffer-cmdstat.midlevel_result = SRpnt-result = req-errors;
STp-buffer-cmdstat.residual = req-resid_len;
 
+   if (STp-stats != NULL) {
+   ticks = get_jiffies_64();
+   STp-stats-in_flight--;
+   ticks -= STp-stats-stamp;
+   STp-stats-io_ticks += ticks;
+   if (req-cmd[0] == WRITE_6)
+   STp-stats-write_ticks += ticks;
+   else if (req-cmd[0] == READ_6)
+   STp-stats-read_ticks += ticks;
+   }
+
tmp = SRpnt-bio;
if (SRpnt-waiting)
complete(SRpnt-waiting);
@@ -496,6 +523,7 @@ static int st_scsi_execute(struct st_req
struct rq_map_data *mdata = SRpnt-stp-buffer-map_data;
int err = 0;
int write = (data_direction == DMA_TO_DEVICE);
+   struct scsi_tape *STp = SRpnt-stp;
 
req = blk_get_request(SRpnt-stp-device-request_queue, write,
  GFP_KERNEL);
@@ -516,6 +544,20 @@ static int st_scsi_execute(struct st_req
}
}
 
+   if (STp-stats != NULL) {
+   if (cmd[0] == WRITE_6) {
+   STp-stats-write_cnt++;
+   STp-stats-write_byte_cnt += bufflen;
+   } else if (cmd[0] == READ_6) {
+   STp-stats-read_cnt++;
+   STp-stats-read_byte_cnt += bufflen;
+   } else {
+   STp-stats-other_cnt++;
+   }
+   STp-stats-stamp = get_jiffies_64();
+   STp-stats-in_flight++;
+   }
+
SRpnt-bio = req-bio;
req-cmd_len = COMMAND_SIZE(cmd[0]);
memset(req-cmd, 0,