Re: [PATCH] st: implement sysfs based tape statistics v2
"Seymour, Shane M" 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
> On 9.2.2015, at 8.00, Seymour, Shane M 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
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
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
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
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
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
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 wrote: > > > From: "Kai Mäkisara (Kolumbus)" > > > 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
On Thu, Feb 05, 2015 at 07:46:32PM +0200, "Kai Mäkisara (Kolumbus)" wrote: > > On 5.2.2015, at 19.40, Laurence Oberman wrote: > > From: "Kai Mäkisara (Kolumbus)" > > 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
On Thu, 2015-02-05 at 19:46 +0200, "Kai Mäkisara (Kolumbus)" wrote: > > On 5.2.2015, at 19.40, Laurence Oberman 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
- Original Message - From: "Kai Mäkisara (Kolumbus)" To: "Laurence Oberman" Cc: "Laurence Oberman" , "Shane M Seymour" , linux-scsi@vger.kernel.org, "James E.J. Bottomley (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 wrote: > > - Original Message - > From: "Kai Mäkisara (Kolumbus)" > To: "Laurence Oberman" > Cc: "Shane M Seymour" , lober...@redhat.com, > linux-scsi@vger.kernel.org, "James E.J. Bottomley (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 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
> On 5.2.2015, at 19.40, Laurence Oberman wrote: > > - Original Message - > From: "Kai Mäkisara (Kolumbus)" > To: "Laurence Oberman" > Cc: "Shane M Seymour" , lober...@redhat.com, > linux-scsi@vger.kernel.org, "James E.J. Bottomley (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 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
- Original Message - From: "Kai Mäkisara (Kolumbus)" To: "Laurence Oberman" Cc: "Shane M Seymour" , lober...@redhat.com, linux-scsi@vger.kernel.org, "James E.J. Bottomley (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 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
> On 2.2.2015, at 17.16, Laurence Oberman 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
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 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 AT&T 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
> +#include
>
> #include
> #include
> @@ -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 s
RE: [PATCH] st: implement sysfs based tape statistics v2
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
AT&T 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
+#include
#include
#include
@@ -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 !
[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
AT&T 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
+#include
#include
#include
@@ -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,
