On Fri, Feb 06, 2015 at 04:59:16AM -0800, Greg KH wrote:
On Fri, Feb 06, 2015 at 12:20:53AM +0000, Seymour, Shane M wrote:
quoted
The current patch that implements tape statistics is here:
http://marc.info/?l=linux-scsi&m=142112067313723&w=2
Aside from the "do we want to do this all in a single file" issue that I
will say more on below, this patch has issues. Please don't use a
kobject for _ANYTHING_ in sysfs that has a struct device as a parent.
If you do that, it can't be seen by userspace tools very well, if at
all.
I can't speak for Shane but wouldn't spend too much time looking at the
current v2 patch: it's the result of a pretty ugly compromise suggested
on linux-scsi.
This thread was really to try to settle the discussion on the structure
of the stats files.
quoted
Recently there was was another discussion here about one file vs a collection of files for tape statistics:
http://marc.info/?l=linux-scsi&m=142316255501550&w=2
The result was that I should ask here what method I should use. I
would like to get feedback in relation to tape statistics and one file
vs multi-file in sysfs. I'm happy to keep the existing code or change
to a single file approach.
One of the primary reasons we created sysfs and the "one value per file"
rule is that multi-value files just do not work well. Yes, you get an
atomic snapshot, and you save some open/read/close syscall roundtrips,
but you do so at the expense of forcing userspace to "know" what the
format of the file is. And once you create it, you can NEVER CHANGE IT
AGAIN.
I am not convinced this is a concern for tape statistics: they are pretty
much a solved problem. The commercial *nixes have had this for decades.
Likewise for disk stats: although fluff like maj:min/name etc. has been
shuffled a few times the basic fields have remained unchanged for a very
long time and sysfs already removes the need to include an identity
field.
Yes, that's right, if you come up with some new statistic in the future,
or realize that one of the ones you have now is wrong, you can't change
it, you have to make a whole new file, otherwise you could break
userspace tools.
I understand the fact that you can't change them; I just don't think it's
a big problem in this specific case (and much less than some of the
more imaginative sysfs content - 2d int arrays with column headers
anyone?).
And yes, open/read/close does take take a few extra cycles, but you
can't really measure it for a virtual filesystem like this on any modern
system.
I'll try to get some numbers when I get back home next week - Shane is
talking about use cases involving tens of thousands of tape devices. I
am not certain that the overhead would be unmeasurable in that case: the
additional context switching & TLB flushes alone seem like they would
add up.
Hope this helps explain why we have the sysfs rule, and why you should
continue to follow it as well.
Yes, it's not always followed, but that's usually because people forgot
why we had this rule, and no one noticed or pointed it out to me that it
was wrong.
Perhaps sysfs.txt should be updated to make the position more clear? The
current wording seems rather more liberal than this thread would
suggest. Maybe something like the patch below?
This would help people who are trying to dtrt by reading the documentation.
Regards,
Bryn.
From 3081aad4cc4d19b68f39499dbeb3837f0642f70e Mon Sep 17 00:00:00 2001
From: "Bryn M. Reeves" [off-list ref]
Date: Fri, 6 Feb 2015 15:19:39 +0000
Subject: [PATCH] docs/sysfs: Specify array valued attribute review
requirements
Although the linux-api position that one-value-per-file is a strong rule
is very clear in mailing list discussions the sysfs.txt documentation
suggests a rather more liberal stance:
"... it is socially acceptable to express an array of values of the same
type."
Fix the documentation to make it clear that such uses should be
discussed on linux-api first.
Signed-off-by: Bryn M. Reeves <redacted>
---
Documentation/filesystems/sysfs.txt | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.txt
index b35a64b..494fa78 100644
--- a/Documentation/filesystems/sysfs.txt
+++ b/Documentation/filesystems/sysfs.txt
@@ -57,8 +57,15 @@ 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.
+value per file, so it may be socially acceptable to express an array of
+values of the same type.
+
+If you are considering adding such an array attribute to sysfs please
+discuss it via the linux-api mailing list first to ensure that your
+proposed use is acceptable:
+
+ https://www.kernel.org/doc/man-pages/linux-api-ml.html
+ linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Mixing types, expressing multiple lines of data, and doing fancy
formatting of data is heavily frowned upon. Doing these things may get
--
1.9.3
On Fri, Feb 06, 2015 at 03:41:58PM +0000, Bryn M. Reeves wrote:
On Fri, Feb 06, 2015 at 04:59:16AM -0800, Greg KH wrote:
quoted
On Fri, Feb 06, 2015 at 12:20:53AM +0000, Seymour, Shane M wrote:
quoted
The current patch that implements tape statistics is here:
http://marc.info/?l=linux-scsi&m=142112067313723&w=2
Aside from the "do we want to do this all in a single file" issue that I
will say more on below, this patch has issues. Please don't use a
kobject for _ANYTHING_ in sysfs that has a struct device as a parent.
If you do that, it can't be seen by userspace tools very well, if at
all.
I can't speak for Shane but wouldn't spend too much time looking at the
current v2 patch: it's the result of a pretty ugly compromise suggested
on linux-scsi.
Fair enough, but please feel free to cc: me on the patch that you do
feel is correct to get a sysfs-related review.
quoted
quoted
Recently there was was another discussion here about one file vs a collection of files for tape statistics:
http://marc.info/?l=linux-scsi&m=142316255501550&w=2
The result was that I should ask here what method I should use. I
would like to get feedback in relation to tape statistics and one file
vs multi-file in sysfs. I'm happy to keep the existing code or change
to a single file approach.
One of the primary reasons we created sysfs and the "one value per file"
rule is that multi-value files just do not work well. Yes, you get an
atomic snapshot, and you save some open/read/close syscall roundtrips,
but you do so at the expense of forcing userspace to "know" what the
format of the file is. And once you create it, you can NEVER CHANGE IT
AGAIN.
I am not convinced this is a concern for tape statistics: they are pretty
much a solved problem. The commercial *nixes have had this for decades.
Likewise for disk stats: although fluff like maj:min/name etc. has been
shuffled a few times the basic fields have remained unchanged for a very
long time and sysfs already removes the need to include an identity
field.
We already handle i/o stats just fine, why create a special sysfs
interface for just a tape device interface? What makes them so special?
quoted
Yes, that's right, if you come up with some new statistic in the future,
or realize that one of the ones you have now is wrong, you can't change
it, you have to make a whole new file, otherwise you could break
userspace tools.
I understand the fact that you can't change them; I just don't think it's
a big problem in this specific case (and much less than some of the
more imaginative sysfs content - 2d int arrays with column headers
anyone?).
What sysfs file is a 2d int array? I'll be glad to fix it.
Also, everyone doesn't think their solution will ever need to be
changed. Until later when it does :)
quoted
And yes, open/read/close does take take a few extra cycles, but you
can't really measure it for a virtual filesystem like this on any modern
system.
I'll try to get some numbers when I get back home next week - Shane is
talking about use cases involving tens of thousands of tape devices. I
am not certain that the overhead would be unmeasurable in that case: the
additional context switching & TLB flushes alone seem like they would
add up.
If you want to measure tens of thousands of tape devices then you
shouldn't be using sysfs in the first place as it is not designed for
"speed" at all. Use the existing i/o rate interfaces instead, don't try
to cram something into sysfs that doesn't belong there.
thanks,
greg k-h
Hello
Its not going to be tens of thousands of devices. That count was an
aggregate based on 1000's of servers.
In reality its unlikely to ever be more than 100 tapes drives per
individual Linux kernel instance.
Therefore sysfs will be the valid way to do this and make the data
available to user space.
Thanks
Laurence
On Feb 6, 2015, at 11:07 PM, Greg KH [off-list ref] wrote:
quoted
On Fri, Feb 06, 2015 at 03:41:58PM +0000, Bryn M. Reeves wrote:
quoted
On Fri, Feb 06, 2015 at 04:59:16AM -0800, Greg KH wrote:
quoted
On Fri, Feb 06, 2015 at 12:20:53AM +0000, Seymour, Shane M wrote:
The current patch that implements tape statistics is here:
http://marc.info/?l=linux-scsi&m=142112067313723&w=2
Aside from the "do we want to do this all in a single file" issue that I
will say more on below, this patch has issues. Please don't use a
kobject for _ANYTHING_ in sysfs that has a struct device as a parent.
If you do that, it can't be seen by userspace tools very well, if at
all.
I can't speak for Shane but wouldn't spend too much time looking at the
current v2 patch: it's the result of a pretty ugly compromise suggested
on linux-scsi.
Fair enough, but please feel free to cc: me on the patch that you do
feel is correct to get a sysfs-related review.
quoted
quoted
quoted
Recently there was was another discussion here about one file vs a collection of files for tape statistics:
http://marc.info/?l=linux-scsi&m=142316255501550&w=2
The result was that I should ask here what method I should use. I
would like to get feedback in relation to tape statistics and one file
vs multi-file in sysfs. I'm happy to keep the existing code or change
to a single file approach.
One of the primary reasons we created sysfs and the "one value per file"
rule is that multi-value files just do not work well. Yes, you get an
atomic snapshot, and you save some open/read/close syscall roundtrips,
but you do so at the expense of forcing userspace to "know" what the
format of the file is. And once you create it, you can NEVER CHANGE IT
AGAIN.
I am not convinced this is a concern for tape statistics: they are pretty
much a solved problem. The commercial *nixes have had this for decades.
Likewise for disk stats: although fluff like maj:min/name etc. has been
shuffled a few times the basic fields have remained unchanged for a very
long time and sysfs already removes the need to include an identity
field.
We already handle i/o stats just fine, why create a special sysfs
interface for just a tape device interface? What makes them so special?
quoted
quoted
Yes, that's right, if you come up with some new statistic in the future,
or realize that one of the ones you have now is wrong, you can't change
it, you have to make a whole new file, otherwise you could break
userspace tools.
I understand the fact that you can't change them; I just don't think it's
a big problem in this specific case (and much less than some of the
more imaginative sysfs content - 2d int arrays with column headers
anyone?).
What sysfs file is a 2d int array? I'll be glad to fix it.
Also, everyone doesn't think their solution will ever need to be
changed. Until later when it does :)
quoted
quoted
And yes, open/read/close does take take a few extra cycles, but you
can't really measure it for a virtual filesystem like this on any modern
system.
I'll try to get some numbers when I get back home next week - Shane is
talking about use cases involving tens of thousands of tape devices. I
am not certain that the overhead would be unmeasurable in that case: the
additional context switching & TLB flushes alone seem like they would
add up.
If you want to measure tens of thousands of tape devices then you
shouldn't be using sysfs in the first place as it is not designed for
"speed" at all. Use the existing i/o rate interfaces instead, don't try
to cram something into sysfs that doesn't belong there.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 07, 2015 at 09:27:05PM -0500, Laurence Oberman wrote:
Hello
Its not going to be tens of thousands of devices. That count was an
aggregate based on 1000's of servers.
In reality its unlikely to ever be more than 100 tapes drives per
individual Linux kernel instance.
Therefore sysfs will be the valid way to do this and make the data
available to user space.
Even if it is only 2 tape drives, again, what's wrong with using the
existing i/o statistic interfaces that all block devices have? Don't go
making special one-off interfaces for one type of device if at all
possible.
thanks,
greg k-h
On 8.2.2015, at 4.45, Greg KH [off-list ref] wrote:
On Sat, Feb 07, 2015 at 09:27:05PM -0500, Laurence Oberman wrote:
quoted
Hello
Its not going to be tens of thousands of devices. That count was an
aggregate based on 1000's of servers.
In reality its unlikely to ever be more than 100 tapes drives per
individual Linux kernel instance.
Therefore sysfs will be the valid way to do this and make the data
available to user space.
Even if it is only 2 tape drives, again, what's wrong with using the
existing i/o statistic interfaces that all block devices have? Don't go
making special one-off interfaces for one type of device if at all
possible.
The tape driver does not have access to the block device i/o statistics because the tapes are not block devices but character devices. They use the Linux block subsystem enough to do i/o but this does not include access to the statistics counters.
The purpose of the suggested text vector format patch is to create statistics that have the same format as the existing i/o statistics that the block devices so that the existing tools can be used with minimal modifications. One alternative is, of course, to tie the st driver more into the Linux block device system. I have looked into this several times but have never seen how to do this in a simple enough way.
Thanks,
Kai