RE: [Patch][RFC] st: provide tape statistics via sysfs
-Original Message- From: Kai Mäkisara [mailto:kai.makis...@kolumbus.fi] Sent: Wednesday, February 27, 2013 4:38 AM To: James Bottomley Cc: Seymour, Shane M; linux-scsi@vger.kernel.org Subject: Re: [Patch][RFC] st: provide tape statistics via sysfs > The sysfs files in the patch export the statistics in the same way as the > disk statistics are exported > (cf. linux/Documentation/iostats.txt). This would enable the tools parson > iostats easily analyze > statistics also for tapes. Can I make a counter proposal that doesn't introduce any more ABI compatibility issues than already exist in sysfs? I'll create a statistics directory as James suggested and expose all of the individual statistics in their own files one value per file. I'll remove the extra field I wanted to add from the stat file and make it identical to the file provided for disk statistics and move the file down into the statistics directory under the device (with a warning in the code that this file must provide exactly the same information as the disks stat file and cannot provide more than that). To get the statistics I want to get I'll use the stat file plus one of the individual files. I'd still like to keep the file with the hint about how many tape drives there will be as a maximum. > The statistics represent the status at one time point and must be read > together (within reason). > This means that splitting the values to several files is not a proper way to > export the information. One file would be better than two, but two is a lot better than having to open and read all of them individually and get stats that really are an approximation. -- 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][RFC] st: provide tape statistics via sysfs
On Feb 22, 2013, at 12:50 PM, James Bottomley wrote: > On Fri, 2013-02-22 at 02:11 +, Seymour, Shane M wrote: >> First forgive me for using outlook for this, if there are any issues with >> what I sent let me know and I'll send it again from gmail. This is also my >> first attempt at a kernel patch so please be gentle. >> >> This patch was written to enable tape statistics via sysfs for the dt >> driver based on kernel 3.8.0-rc6. It creates two new files in sysfs >> and is based on work done previously in 2005 by Kai Mäkisara. Any >> feedback would be greatly appreciated. > > 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. > The sysfs files in the patch export the statistics in the same way as the disk statistics are exported (cf. linux/Documentation/iostats.txt). This would enable the tools parson iostats easily analyze statistics also for tapes. The statistics represent the status at one time point and must be read together (within reason). This means that splitting the values to several files is not a proper way to export the information. What then is the proper way? Debugfs? 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][RFC] st: provide tape statistics via sysfs
.. 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. That was initially my understanding, but when I looked at this the precedent had already been set, see for example the block subsys: [root@darwin ~]# grep . /sys/block/sd*/stat /sys/block/sda/stat: 27351 6890 609272 22812936810 920727 7660304 13339500 556889 1562009 /sys/block/sdb/stat:2369 6762188903900300 000 405939002 [root@darwin ~]# I can only assume it was implemented this way for the sake of efficiency, eg avoid a huge amount of file open/read/close calls in sar/iostat. It's not unusual for us to see a thousand block devices on enterprise servers, multiply that by the number of above entries and you would be talking about 11 x block-dev-count per iostat read iteration. Okay for tapes we typically don't see anything like this number but the patch just follows the precedent set with the block device. Nevertheless, if opinion is really strong against this we could split each stat out in to a single file. Actually, to be a little more thorough I checked the kernel Docs: [root@darwin Documentation]# less ./filesystems/sysfs.txt Attributes ~~ Attributes can be exported for kobjects in the form of regular files in the filesystem. Sysfs forwards file I/O operations to methods defined for the attributes, providing a means to read and write kernel attributes. 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. Technically speaking we are not providing an array of the same types, so again I'd say if opinion is really strong then we can break this out. It's really then a question of file-io efficiency. The biggest tape library I've seen has 2700 drives installed, however I think devices of that number are few and far between Regards Darren -- 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][RFC] st: provide tape statistics via sysfs
Hi James On 22/02/13 10:50, James Bottomley wrote: On Fri, 2013-02-22 at 02:11 +, Seymour, Shane M wrote: First forgive me for using outlook for this, if there are any issues with what I sent let me know and I'll send it again from gmail. This is also my first attempt at a kernel patch so please be gentle. This patch was written to enable tape statistics via sysfs for the dt driver based on kernel 3.8.0-rc6. It creates two new files in sysfs and is based on work done previously in 2005 by Kai Mäkisara. Any feedback would be greatly appreciated. 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. That was initially my understanding, but when I looked at this the precedent had already been set, see for example the block subsys: [root@darwin ~]# grep . /sys/block/sd*/stat /sys/block/sda/stat: 27351 6890 609272 22812936810 920727 7660304 13339500 556889 1562009 /sys/block/sdb/stat:2369 6762188903900300 000 405939002 [root@darwin ~]# I can only assume it was implemented this way for the sake of efficiency, eg avoid a huge amount of file open/read/close calls in sar/iostat. It's not unusual for us to see a thousand block devices on enterprise servers, multiply that by the number of above entries and you would be talking about 11 x block-dev-count per iostat read iteration. Okay for tapes we typically don't see anything like this number but the patch just follows the precedent set with the block device. Nevertheless, if opinion is really strong against this we could split each stat out in to a single file. Assuming sysfs is mounted at /sys the first file is /sys/bus/scsi/drivers/st/drives which gives a single number indicating what the largest tape drive instance assigned by st_probe in the st module is. If it's 4 it possible that st0, st1, st2, and st3 exist on the system. Since tape drives can later be disconnected they don't have to exist, the count is a hint so it's possible to gather statistics in a loop with an upper bound. This makes it easier in iostat to gather statistcs. I don't really get why you need this. What's wrong with getting the exact information from /sys/class/scsi_tape? Again efficiency. The sys/class/scsi_tape is a per device entry whilst /sys/bus/scsi/drivers/st is essentially a global, it seems to make more sense to place a number-of-attached devices here than on a repeated per device basis, especially if for example st0 later disappears, perfectly possible with SAN based devices. If we don't have some hint as to the number of devices currently attached then you don't have any clue as to the device iteration range. Darren -- 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][RFC] st: provide tape statistics via sysfs
On Fri, 2013-02-22 at 02:11 +, Seymour, Shane M wrote: > First forgive me for using outlook for this, if there are any issues with > what I sent let me know and I'll send it again from gmail. This is also my > first attempt at a kernel patch so please be gentle. > > This patch was written to enable tape statistics via sysfs for the dt > driver based on kernel 3.8.0-rc6. It creates two new files in sysfs > and is based on work done previously in 2005 by Kai Mäkisara. Any > feedback would be greatly appreciated. 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. > Assuming sysfs is mounted at /sys the first file > is /sys/bus/scsi/drivers/st/drives which gives a single number > indicating what the largest tape drive instance assigned by st_probe > in the st module is. If it's 4 it possible that st0, st1, st2, and st3 > exist on the system. Since tape drives can later be disconnected they > don't have to exist, the count is a hint so it's possible to gather > statistics in a loop with an upper bound. This makes it easier in > iostat to gather statistcs. I don't really get why you need this. What's wrong with getting the exact information from /sys/class/scsi_tape? 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