Re: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-11 Thread Bryn M. Reeves
On Wed, Feb 11, 2015 at 06:30:27AM +0800, Greg KH wrote:
> On Tue, Feb 10, 2015 at 02:27:20PM +, Bryn M. Reeves wrote:
> > On Sat, Feb 07, 2015 at 12:07:43PM +0800, Greg KH wrote:
> > 
> > $ cat /sys/fs/selinux/avc/cache_stats 
> > lookups hits misses allocations reclaims frees
> > 18938916 18921707 17209 17209 17328 22215
> > 38164283 38146514 17769 17769 16800 19049
> > 18078108 18056991 21117 21117 21344 19305
> > 15168204 15150079 18125 18125 17776 13149
> > 0 0 0 0 0 0
> > 0 0 0 0 0 0
> > 0 0 0 0 0 0
> > 0 0 0 0 0 0
> > 
> > $ cat /sys/fs/selinux/avc/hash_stats
> > entries: 506
> > buckets used: 290/512
> > longest chain: 5
> 
> Ugh, those look like they should be debugfs interfaces.  Thanks, I'll
> add them to my list of things to nag people about...

Actually looking properly these are outside sysfs:

$ mount | grep selinux
selinuxfs on /sys/fs/selinux type selinuxfs (rw,relatime)

So everything below the mount point is in their own vfstype.

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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-10 Thread Greg KH
On Tue, Feb 10, 2015 at 02:27:20PM +, Bryn M. Reeves wrote:
> On Sat, Feb 07, 2015 at 12:07:43PM +0800, Greg KH wrote:
> 
> $ cat /sys/fs/selinux/avc/cache_stats 
> lookups hits misses allocations reclaims frees
> 18938916 18921707 17209 17209 17328 22215
> 38164283 38146514 17769 17769 16800 19049
> 18078108 18056991 21117 21117 21344 19305
> 15168204 15150079 18125 18125 17776 13149
> 0 0 0 0 0 0
> 0 0 0 0 0 0
> 0 0 0 0 0 0
> 0 0 0 0 0 0
> 
> $ cat /sys/fs/selinux/avc/hash_stats
> entries: 506
> buckets used: 290/512
> longest chain: 5

Ugh, those look like they should be debugfs interfaces.  Thanks, I'll
add them to my list of things to nag people about...

greg k-h
--
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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-10 Thread Bryn M. Reeves
On Sat, Feb 07, 2015 at 12:07:43PM +0800, Greg KH wrote:
> On Fri, Feb 06, 2015 at 03:41:58PM +, Bryn M. Reeves wrote:
> > 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.

Will do; I'm back from travels this week & will have some time to look at
this.
 
> > 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?

But the iostats use exactly the sort of array file we're talking about:

$ cat /sys/block/sda/stat 
  12764420869  4320505  2305697   15404530056  3834036  9065092
0   931842 11371357

And we can't simply extend these to tapes as they are not block devices.
 
> > 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.

$ cat /sys/fs/selinux/avc/cache_stats 
lookups hits misses allocations reclaims frees
18938916 18921707 17209 17209 17328 22215
38164283 38146514 17769 17769 16800 19049
18078108 18056991 21117 21117 21344 19305
15168204 15150079 18125 18125 17776 13149
0 0 0 0 0 0
0 0 0 0 0 0
0 0 0 0 0 0
0 0 0 0 0 0

$ cat /sys/fs/selinux/avc/hash_stats
entries: 506
buckets used: 290/512
longest chain: 5

> 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.

So far as I'm aware there is no other way to obtain performance data
for the SCSI tape subsystem (without resorting to ftrace/systemtap).

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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-08 Thread James Bottomley
On Sun, 2015-02-08 at 10:45 +0800, Greg KH wrote:
> 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?

Tape is a character device.  It only uses block via SCSI (SCSI uses
block to give an issue queue for every device).  One of the problems
with this model is that the block kobj, where all the statistics hang,
is actually never exposed for these devices because they don't have a
block name.  Even granted that we could alter block to give names to the
nameless queues and expose them in /sys/block, we'd still have the
problem, the queue statistics are the property of the pluggable I/O
scheduler, so there's a disconnect between the SCSI upper layer drivers
and the block scheduler (since the latter is embedded by design).
Pulling that apart would get us into a fairly nasty layering violation
(drivers aren't supposed to care about the scheulders).

>   Don't go
> making special one-off interfaces for one type of device if at all
> possible.

I don't really see any way around this.  The statistics the block
schedulers collect are relevant to I/O load balancing; that's not at all
the same class of statistics as the users of tape are interested in.
This problem is equivalent to the fibrechannel one where we collect the
fc_host_statistics in the scsi_transport_fc.c class as an attribute
group (block doesn't want to see or know any of the information because
it's all relevant to the transport, not the block abstraction).

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: [RFC] implementing tape statistics single file vs multi-file in sysfs

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

> On 8.2.2015, at 4.45, Greg KH  wrote:
> 
> 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.
> 
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

--
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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-07 Thread Greg KH
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
--
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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-07 Thread Laurence Oberman
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  wrote:
> 
>> On Fri, Feb 06, 2015 at 03:41:58PM +, Bryn M. Reeves wrote:
>>> On Fri, Feb 06, 2015 at 04:59:16AM -0800, Greg KH wrote:
 On Fri, Feb 06, 2015 at 12:20:53AM +, 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.
> 
 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?
> 
>>> 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 :)
> 
>>> 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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-07 Thread Greg KH
On Fri, Feb 06, 2015 at 03:41:58PM +, Bryn M. Reeves wrote:
> On Fri, Feb 06, 2015 at 04:59:16AM -0800, Greg KH wrote:
> > On Fri, Feb 06, 2015 at 12:20:53AM +, 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.

> > > 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?

> > 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 :)

> > 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-06 Thread Bryn M. Reeves
On Fri, Feb 06, 2015 at 04:59:16AM -0800, Greg KH wrote:
> On Fri, Feb 06, 2015 at 12:20:53AM +, 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.

This thread was really to try to settle the discussion on the structure
of the stats files.

> > 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" 
  Date: Fri, 6 Feb 2015 15:19:39 +
  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 
---
 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-...@

Re: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-06 Thread Greg KH
On Fri, Feb 06, 2015 at 09:13:55AM +, Bryn M. Reeves wrote:
> > The sysfs documentation says that files should contain one item per
> > file (with some small exceptions):
> > 
> > > "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."
> 
> Right: I think there's good precedent for the array file style when
> dealing with counter sets.

See my previous reply for why I strongly feel this is incorrect, sorry.

greg k-h
--
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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-06 Thread Greg KH
On Fri, Feb 06, 2015 at 12:20:53AM +, Seymour, Shane M wrote:
> Hello linux-api'ers
> 
> There has been some ongoing discussion about the best way to implement tape 
> statistics. The original method suggested a long time ago used a single file 
> in sysfs similar to block statistics in sysfs. That lead to an impass about 
> the code on the linux-scsi mailing list.
> 
> The sysfs documentation says that files should contain one item per file 
> (with some small exceptions):
> 
> > "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."
> 
> 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.

Instead, if you want to create a directory, just use an attribute group,
which will be created at the proper time (before the device is announced
to userspace), and then userspace can see it with the tools it is used
to using (i.e. libudev and friends.)

That should simplify your code a lot, please make that change no matter
what happens here with the content of the files.

> 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.

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.

Instead, a one-value-per-file rule forces userspace to know that if the
file is present, it can be opened and read from for a single value.  If
it isn't there, it should fail properly and move on to the next
statistic.  If you want to add a new statistic, great, just add a new
file and you are set.

You aren't dealing with performance-sensitive numbers here that "have"
to have an atomic snapshot of the state at a specific point in time in
order to work properly, so just have a bunch of files, all one value per
file, then all userspace tools will "just work" (i.e. libudev), and
everyone is happy.

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.

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.

thanks,

greg k-h
--
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: [RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-06 Thread Bryn M. Reeves
On Fri, Feb 06, 2015 at 12:20:53AM +, Seymour, Shane M wrote:
> There has been some ongoing discussion about the best way to implement
> tape statistics. The original method suggested a long time ago used a
> single file in sysfs similar to block statistics in sysfs. That lead to
> an impass about the code on the linux-scsi mailing list.

I would have a strong preference for a single file containing an array
of integer counters. This is in keeping with other statistics attributes
in sysfs and follows the principle of least surprise: it's essentially
the same general format as /proc/diskstats and sysfs disk and partition
stats (dm statistics also follow this convention via the @print_stats
message).

This simplifies userspace code to read and parse the counters and avoids
additional sample jitter when reading stats for very large numbers of
devices; each device would require at least eleven open()/read()/close()
cycles.

For a small number of devices this shouldn't matter too much but
eventually the additional syscall overhead could become significant (I
think you mentioned users with ~20k devices?).

The sync file mechanism in the v2 patch that addresses this problem is
kinda cute but also significantly more complex than a plain old array
and as you pointed out adds hundreds of lines to the patch..

Sticking to arrays also allows existing tools like sysstat to be easily
adapted to the new data source.

> The sysfs documentation says that files should contain one item per file 
> (with some small exceptions):
> 
> > "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."

Right: I think there's good precedent for the array file style when
dealing with counter sets.

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


[RFC] implementing tape statistics single file vs multi-file in sysfs

2015-02-05 Thread Seymour, Shane M
Hello linux-api'ers

There has been some ongoing discussion about the best way to implement tape 
statistics. The original method suggested a long time ago used a single file in 
sysfs similar to block statistics in sysfs. That lead to an impass about the 
code on the linux-scsi mailing list.

The sysfs documentation says that files should contain one item per file (with 
some small exceptions):

> "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."

The current patch that implements tape statistics is here:

http://marc.info/?l=linux-scsi&m=142112067313723&w=2

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.

Thanks
Shane
--
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