* [lm-sensors] Memory usage of sysfs files
@ 2006-03-08 22:36 Hartmut Rick
2006-03-09 19:41 ` Jean Delvare
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: Hartmut Rick @ 2006-03-08 22:36 UTC (permalink / raw)
To: lm-sensors
Hi everybody,
On Wed, 8 Mar 2006, Jean Delvare wrote:
> The only data I am missing now is the memory used by each additional
> sysfs file we create. We need to know, as Hans objected that too many
> sysfs files could have a negative impact on memory consumption. I dug
I've just created a couple sysfs files in order to find out, and the
conclusion is that each additional sysfs file reduces the remaining free
memory by something on the order of 90-95 bytes.
It turns out that the memory reported by 'free' varies quite a bit between
calls, which disturbs somewhat the precision of this measurement. I had to
create 10000 sysfs files in order to get a statistically significant
result. Now this result is correct on the 10% level, which I think is good
enough for our discussion.
For this experiment, I used the same callback function for all sysfs
files, so each additional file has just one SENSOR_DEVICE_ATTR struct
definition and a device_create_file() call, but no extra supporting code.
This is the typical situation for the individual alarm files, I guess.
With current DRAM prices, 100 extra bytes correspond to extra costs of
around 1e-5 EUR or USD for each additional file, which I think is
tolerable.
Best regards,
Hartmut
^ permalink raw reply [flat|nested] 11+ messages in thread
* [lm-sensors] Memory usage of sysfs files
2006-03-08 22:36 [lm-sensors] Memory usage of sysfs files Hartmut Rick
@ 2006-03-09 19:41 ` Jean Delvare
2006-03-09 20:05 ` Hans de Goede
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Jean Delvare @ 2006-03-09 19:41 UTC (permalink / raw)
To: lm-sensors
Hi Hartmut,
> I've just created a couple sysfs files in order to find out, and the
> conclusion is that each additional sysfs file reduces the remaining free
> memory by something on the order of 90-95 bytes.
> It turns out that the memory reported by 'free' varies quite a bit between
> calls, which disturbs somewhat the precision of this measurement. I had to
> create 10000 sysfs files in order to get a statistically significant
> result. Now this result is correct on the 10% level, which I think is good
> enough for our discussion.
> For this experiment, I used the same callback function for all sysfs
> files, so each additional file has just one SENSOR_DEVICE_ATTR struct
> definition and a device_create_file() call, but no extra supporting code.
> This is the typical situation for the individual alarm files, I guess.
Yes, you tested the right thing, thanks for reporting your results.
I have been conducting a similar investigation but on the theoretical
front. It seems that the amount of memory required by each additional
sysfs file (not counting the callbacks and creation) is the size of
stuct sysfs_dirent, plus 4 bytes (not sure why, most probably an
internal slab implementation detail.) This is 40 + 4 = 44 bytes on x86,
72 + 4 = 76 bytes on x86_64.
I don't know why your measurement suggests larger allocations. But even
with your value of 90-95 bytes it's still a rather small value.
> With current DRAM prices, 100 extra bytes correspond to extra costs of
> around 1e-5 EUR or USD for each additional file, which I think is
> tolerable.
I don't think this is a matter of price, but more of a matter of
consumed memory on already designed/deployed systems, especially on
embedded systems. Other systems typically have over 3000 sysfs files,
so a dozen more or less really doesn't change anything.
If we consider the w83627hf driver as an example (being one of the
drivers with the higher number of alarm+beep files in my individual
files model) the additional memory consumption is (on x86) 44 * 33 1452 bytes. This really isn't much, even for an embedded system. Such
systems will typically have between 600 and 1600 sysfs files already,
33 more or less (worse case) still aren't that much.
Last, here are the figures I promised some days ago:
Before my individual alarm files patches:
text data bss dec hex filename
6101 1344 4 7449 1d19 drivers/hwmon/f71805f.o
4848 1052 16 5916 171c drivers/hwmon/lm63.o
5861 1892 36 7789 1e6d drivers/hwmon/lm90.o
15024 1356 16 16396 400c drivers/hwmon/w83627hf.o
-rw-r--r-- 1 khali users 24129 2006-03-09 18:52 drivers/hwmon/f71805f.c
-rw-r--r-- 1 khali users 18909 2006-03-09 15:23 drivers/hwmon/lm63.c
-rw-r--r-- 1 khali users 22806 2006-03-09 15:23 drivers/hwmon/lm90.c
-rw-r--r-- 1 khali users 45891 2006-03-09 15:23 drivers/hwmon/w83627hf.c
After the patches:
text data bss dec hex filename
6357 1696 4 8057 1f79 drivers/hwmon/f71805f.o
5122 1196 16 6334 18be drivers/hwmon/lm63.o
6185 2060 36 8281 2059 drivers/hwmon/lm90.o
16126 2128 16 18270 475e drivers/hwmon/w83627hf.o
-rw-r--r-- 1 khali users 25316 2006-03-09 19:04 drivers/hwmon/f71805f.c
-rw-r--r-- 1 khali users 20174 2006-03-09 19:04 drivers/hwmon/lm63.c
-rw-r--r-- 1 khali users 24235 2006-03-09 19:04 drivers/hwmon/lm90.c
-rw-r--r-- 1 khali users 50068 2006-03-09 19:18 drivers/hwmon/w83627hf.c
Summary:
f71805f: +608 binary ( +8.1%), +1187 source ( +4.9%)
lm63: +418 binary ( +7.0%), +1265 source ( +6.6%)
lm90: +492 binary ( +6.3%), +1429 source ( +6.2%)
w83627hf: +1874 binary (+11.4%), +4177 source ( +9.1%)
So the size increase is admittedly not neglectable and greater than I
would like, but it isn't (IMHO at least) unaccpeptable either,
especially when hardware monitoring drivers are not amongst the largest
drivers in the first place. Also note that the number of individual
alarm files depends on the number of channels and/or limits the device
has, so this approach won't suddenly turn a small driver into a large
one. The size increment had to be somehow proportional to the original
driver size.
If anyone has similar figures for Hans de Goede's proposal, for
comparison, I'll be happy to take a look.
Thanks,
--
Jean Delvare
^ permalink raw reply [flat|nested] 11+ messages in thread
* [lm-sensors] Memory usage of sysfs files
2006-03-08 22:36 [lm-sensors] Memory usage of sysfs files Hartmut Rick
2006-03-09 19:41 ` Jean Delvare
@ 2006-03-09 20:05 ` Hans de Goede
2006-03-09 20:06 ` Hans de Goede
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2006-03-09 20:05 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> Hi Hartmut,
>
>> I've just created a couple sysfs files in order to find out, and the
>> conclusion is that each additional sysfs file reduces the remaining free
>> memory by something on the order of 90-95 bytes.
>> It turns out that the memory reported by 'free' varies quite a bit between
>> calls, which disturbs somewhat the precision of this measurement. I had to
>> create 10000 sysfs files in order to get a statistically significant
>> result. Now this result is correct on the 10% level, which I think is good
>> enough for our discussion.
>> For this experiment, I used the same callback function for all sysfs
>> files, so each additional file has just one SENSOR_DEVICE_ATTR struct
>> definition and a device_create_file() call, but no extra supporting code.
>> This is the typical situation for the individual alarm files, I guess.
>
> Yes, you tested the right thing, thanks for reporting your results.
>
> I have been conducting a similar investigation but on the theoretical
> front. It seems that the amount of memory required by each additional
> sysfs file (not counting the callbacks and creation) is the size of
> stuct sysfs_dirent, plus 4 bytes (not sure why, most probably an
> internal slab implementation detail.) This is 40 + 4 = 44 bytes on x86,
> 72 + 4 = 76 bytes on x86_64.
>
> I don't know why your measurement suggests larger allocations. But even
> with your value of 90-95 bytes it's still a rather small value.
>
AFAIK each sysfs file has a inode behind it which has been fixated in
the inode-cache. This is also one of the more worry some parts IMHO, if
we completly fill the inode-cache (which can be configured to be quite
small on embedded systems) this will cause a performace impact. This is
all AFAIK / IMHO.
> Also note that the number of individual
> alarm files depends on the number of channels and/or limits the device
> has, so this approach won't suddenly turn a small driver into a large
> one. The size increment had to be somehow proportional to the original
> driver size.
>
Well, the uguru driver would have 100 alarm (mask/beep/shutdown) files
on my mb instead of the current 14!
(this number could vary depending on the exact config of the uguru on a mb)
> If anyone has similar figures for Hans de Goede's proposal, for
> comparison, I'll be happy to take a look.
>
To get an idea of the extra code size for the mask creation I've removed
the loops doing the masking in the uguru driver and replaced them with a
simple function which gets/sets values directly from the data using
attr->nr and/or attr->index .
Results:
With loops:
[hans at shalem uguru]$ ls -l abituguru.o abituguru.ko
-rw-rw-r-- 1 hans hans 232871 Mar 8 15:14 abituguru.ko
-rw-rw-r-- 1 hans hans 154472 Mar 8 15:14 abituguru.o
Without loops:
[hans at shalem uguru]$ ls -l abituguru.o abituguru.ko
-rw-rw-r-- 1 hans hans 231191 Mar 9 21:12 abituguru.ko
-rw-rw-r-- 1 hans hans 152816 Mar 9 21:11 abituguru.o
This would also spare one array with the sensor masks in show to user
order a 16 bytes which is malloced.
So this saves around 1.8k code and would cost around 9k malloced (+
possible fragmented page size loss) data.
Also think about the number of extra file handles and / or open/closes a
userspace program would need to have / do with this interface.
Regards,
Hans
^ permalink raw reply [flat|nested] 11+ messages in thread
* [lm-sensors] Memory usage of sysfs files
2006-03-08 22:36 [lm-sensors] Memory usage of sysfs files Hartmut Rick
2006-03-09 19:41 ` Jean Delvare
2006-03-09 20:05 ` Hans de Goede
@ 2006-03-09 20:06 ` Hans de Goede
2006-03-09 21:05 ` Jean Delvare
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2006-03-09 20:06 UTC (permalink / raw)
To: lm-sensors
p.s.
I'm on x86_64 so the code would be even smaller on i386
Regards,
Hans
^ permalink raw reply [flat|nested] 11+ messages in thread
* [lm-sensors] Memory usage of sysfs files
2006-03-08 22:36 [lm-sensors] Memory usage of sysfs files Hartmut Rick
` (2 preceding siblings ...)
2006-03-09 20:06 ` Hans de Goede
@ 2006-03-09 21:05 ` Jean Delvare
2006-03-09 21:41 ` Hans de Goede
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Jean Delvare @ 2006-03-09 21:05 UTC (permalink / raw)
To: lm-sensors
Hi Hans,
> AFAIK each sysfs file has a inode behind it which has been fixated in
> the inode-cache. This is also one of the more worry some parts IMHO, if
> we completly fill the inode-cache (which can be configured to be quite
> small on embedded systems) this will cause a performace impact. This is
> all AFAIK / IMHO.
Well, if a given embedded system wants to support hardware monitoring,
it gets to be configured with such an inode-cache size that it'll work
properly. If not this is a configuration issue, and I don't think we
can take this into account in our design decision.
> > Also note that the number of individual
> > alarm files depends on the number of channels and/or limits the device
> > has, so this approach won't suddenly turn a small driver into a large
> > one. The size increment had to be somehow proportional to the original
> > driver size.
>
> Well, the uguru driver would have 100 alarm (mask/beep/shutdown) files
> on my mb instead of the current 14!
> (this number could vary depending on the exact config of the uguru on a mb)
Ah, I guess this explains your reluctance to individual files ;) But the
uguru really is more of an exception than the rule here. For the other
drivers, I'd expect no more than 50 files for the largest ones, and
rather half that value for the vast majority of our "complete hardware
monitoring" drivers. Temperature-only drivers have even less, of course.
How many sysfs files does the uguru driver have, not counting the
alarm/beep/shutdown entries?
> To get an idea of the extra code size for the mask creation I've removed
> the loops doing the masking in the uguru driver and replaced them with a
> simple function which gets/sets values directly from the data using
> attr->nr and/or attr->index .
>
> Results:
> With loops:
> [hans at shalem uguru]$ ls -l abituguru.o abituguru.ko
> -rw-rw-r-- 1 hans hans 232871 Mar 8 15:14 abituguru.ko
> -rw-rw-r-- 1 hans hans 154472 Mar 8 15:14 abituguru.o
>
> Without loops:
> [hans at shalem uguru]$ ls -l abituguru.o abituguru.ko
> -rw-rw-r-- 1 hans hans 231191 Mar 9 21:12 abituguru.ko
> -rw-rw-r-- 1 hans hans 152816 Mar 9 21:11 abituguru.o
These numbers are really too high, you must have some debugging option
enabled. Please provide the same numbers without debugging so that the
comparisons make more sense.
> This would also spare one array with the sensor masks in show to user
> order a 16 bytes which is malloced.
Hm?
> So this saves around 1.8k code and would cost around 9k malloced (+
> possible fragmented page size loss) data.
Nothing to be afraid of, if you want my opinion.
> Also think about the number of extra file handles and / or open/closes a
> userspace program would need to have / do with this interface.
That's right, this is an interesting point, maybe the more serious in
your series of objections. That being said, I wouldn't expect a
user-space program to use several file handles, that's really the kind
of process where you serialize the reads. I'm also not sure how costly
an open/close on a ram-based fs is. Given that there is no seek time
involved, I'd expect it to be quite fast. But if you have some figures
to throw in the battle, they'll be welcome.
I'd really like to see other numbers for your propsal on a driver
different from the uguru, as this one really isn't representative of
the rest of the drivers. If you could pick one (or more) on which I did
my own tests, even better (these are f71805f, lm63, lm90, w83627hf).
Lastly, I have been thinking of another advantage of my proposal over
yours: it makes it possible for user-space to know exactly which
features a given chip has. With your proposal, there is no way for the
user-space to know if a given channel supports alarm or beep until this
alarm or beep is activated; it basically assumes that all channels
support alarm or beep, or none does.
A concrete example can be found in the new smsc47m192 driver: temp2 and
temp3 have a diode fault bit, but temp1 doesn't. Same for temp1 vs
temp2 in the lm63 and lm90 drivers. I guess we can find similar
examples in other drivers. But I would agree that this probably is a
rare case, so it doesn't justify my proposal on its own - it's just
another item on the scales.
Thanks,
--
Jean Delvare
^ permalink raw reply [flat|nested] 11+ messages in thread
* [lm-sensors] Memory usage of sysfs files
2006-03-08 22:36 [lm-sensors] Memory usage of sysfs files Hartmut Rick
` (3 preceding siblings ...)
2006-03-09 21:05 ` Jean Delvare
@ 2006-03-09 21:41 ` Hans de Goede
2006-03-09 23:16 ` Hans de Goede
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2006-03-09 21:41 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> Hi Hans,
>
>> Well, the uguru driver would have 100 alarm (mask/beep/shutdown) files
>> on my mb instead of the current 14!
>> (this number could vary depending on the exact config of the uguru on a mb)
>
> Ah, I guess this explains your reluctance to individual files ;) But the
> uguru really is more of an exception than the rule here. For the other
> drivers, I'd expect no more than 50 files for the largest ones, and
> rather half that value for the vast majority of our "complete hardware
> monitoring" drivers. Temperature-only drivers have even less, of course.
>
> How many sysfs files does the uguru driver have, not counting the
> alarm/beep/shutdown entries?
>
72 without pwm 54
>> To get an idea of the extra code size for the mask creation I've removed
>> the loops doing the masking in the uguru driver and replaced them with a
>> simple function which gets/sets values directly from the data using
>> attr->nr and/or attr->index .
>>
>> Results:
>> With loops:
>> [hans at shalem uguru]$ ls -l abituguru.o abituguru.ko
>> -rw-rw-r-- 1 hans hans 232871 Mar 8 15:14 abituguru.ko
>> -rw-rw-r-- 1 hans hans 154472 Mar 8 15:14 abituguru.o
>>
>> Without loops:
>> [hans at shalem uguru]$ ls -l abituguru.o abituguru.ko
>> -rw-rw-r-- 1 hans hans 231191 Mar 9 21:12 abituguru.ko
>> -rw-rw-r-- 1 hans hans 152816 Mar 9 21:11 abituguru.o
>
> These numbers are really too high, you must have some debugging option
> enabled. Please provide the same numbers without debugging so that the
> comparisons make more sense.
>
Erm, how do I do that? Also they might be correct this is x86_64 which
comes with almost twice as big code and: "wc -l abituguru.c" gives 1473.
Remeber currently I'm building this against a kernel as provided by
Fedora using the out of tree module building tech 2.6 has (which btw is
great).
>> This would also spare one array with the sensor masks in show to user
>> order a 16 bytes which is malloced.
>
> Hm?
>
Currently I need an array wich maps a sensor id to an alarm-reg-mask for
that sensor, this array is 16 bytes big.
>> Also think about the number of extra file handles and / or open/closes a
>> userspace program would need to have / do with this interface.
>
> That's right, this is an interesting point, maybe the more serious in
> your series of objections. That being said, I wouldn't expect a
> user-space program to use several file handles, that's really the kind
> of process where you serialize the reads. I'm also not sure how costly
> an open/close on a ram-based fs is. Given that there is no seek time
> involved, I'd expect it to be quite fast. But if you have some figures
> to throw in the battle, they'll be welcome.
>
Sorry no figures, it just doesn't feel right todo all this opens / close
if we can pass it all in one file. Also be aware that each open and each
read and each close will cause a userspace <-> kernel space context
switch syscalls are expensive. Doing this the lots of files way just
doesn't feel right for me (I've learned programming on a 8051, so 9k
additional data is _huge_ to me).
> I'd really like to see other numbers for your propsal on a driver
> different from the uguru, as this one really isn't representative of
> the rest of the drivers. If you could pick one (or more) on which I did
> my own tests, even better (these are f71805f, lm63, lm90, w83627hf).
>
I'll see what I can do. I've just finised making all the requested
changes to the uguru driver, but untill this is sorted submitting it
doesn't make much sense.
> Lastly, I have been thinking of another advantage of my proposal over
> yours: it makes it possible for user-space to know exactly which
> features a given chip has. With your proposal, there is no way for the
> user-space to know if a given channel supports alarm or beep until this
> alarm or beep is activated; it basically assumes that all channels
> support alarm or beep, or none does.
>
> A concrete example can be found in the new smsc47m192 driver: temp2 and
> temp3 have a diode fault bit, but temp1 doesn't. Same for temp1 vs
> temp2 in the lm63 and lm90 drivers. I guess we can find similar
> examples in other drivers. But I would agree that this probably is a
> rare case, so it doesn't justify my proposal on its own - it's just
> another item on the scales.
>
Yes, that is a real argument in favor of the one file method.
Regards,
Hans
^ permalink raw reply [flat|nested] 11+ messages in thread
* [lm-sensors] Memory usage of sysfs files
2006-03-08 22:36 [lm-sensors] Memory usage of sysfs files Hartmut Rick
` (4 preceding siblings ...)
2006-03-09 21:41 ` Hans de Goede
@ 2006-03-09 23:16 ` Hans de Goede
2006-03-10 11:11 ` Hans de Goede
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2006-03-09 23:16 UTC (permalink / raw)
To: lm-sensors
Hi,
I've given this some more thought and I've decided against implementing
my way for a couple of other drivers to be able to really measure the
difference. I believe we agree my way is somewhat more efficient, but
that the differences aren't big enough to swing the scales to my way
only on the efficiency reasons.
Instead of spending more time coding test-cases I'd rather just see a
decision soon and (if nescessarry) I'll update the uguru driver to match .
There are 2 arguments I would still like to make in favor of my way though:
1) you initially did it this way too for atleast one driver. Remember,
usually your first hunch is a good one.
2) I've also written support for libsensors and the sensors application
when doing things my way and that fits nicely. I'm afraid that the many
files way will be a problem with the current libsensors. I don't want to
now wath the lib and the progs chips.* will look like. 100+ additional
entries for the uguru alone! And I don't think the chip specific reading
and report code in the sensors application will get any pretter too.
Now I know that libsensors is destined to be replaced, but it will be
around for a while I think.
I've attached a patch adding support for the uguru to both lib and prog
with alarm reporting to give you an idea how this will look like when
doing things my way. Please note that this patch needs updating for
2.10 / cvs and thus won't apply.
Regards,
Hans
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lm_sensors-2.9.1-uguru.patch
Type: text/x-patch
Size: 15184 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20060310/73c52b90/lm_sensors-2.9.1-uguru-0001.bin
^ permalink raw reply [flat|nested] 11+ messages in thread
* [lm-sensors] Memory usage of sysfs files
2006-03-08 22:36 [lm-sensors] Memory usage of sysfs files Hartmut Rick
` (5 preceding siblings ...)
2006-03-09 23:16 ` Hans de Goede
@ 2006-03-10 11:11 ` Hans de Goede
2006-03-10 13:46 ` Jean Delvare
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2006-03-10 11:11 UTC (permalink / raw)
To: lm-sensors
Jean,
Can you take a decision on this please and then write a bit of
specification? Then I can make the uGuru driver implement the
specification and submit it for merging.
What I need for specification if you decide for the one file per sensor
is the names to use for:
-alarm
-alarm enable/disable
-beep on alarm enable/disable
-shutdown on alarm enable/disable
And also the names for alarm and alarm enable/disable for sensors with
more then 1 alarm, specifically with a volt low and a volt high alarm.
Thanks & Regards,
Hans
^ permalink raw reply [flat|nested] 11+ messages in thread
* [lm-sensors] Memory usage of sysfs files
2006-03-08 22:36 [lm-sensors] Memory usage of sysfs files Hartmut Rick
` (6 preceding siblings ...)
2006-03-10 11:11 ` Hans de Goede
@ 2006-03-10 13:46 ` Jean Delvare
2006-03-10 14:09 ` Jean Delvare
2006-03-10 15:28 ` Jim Cromie
9 siblings, 0 replies; 11+ messages in thread
From: Jean Delvare @ 2006-03-10 13:46 UTC (permalink / raw)
To: lm-sensors
Hi Hans,
> > > With loops:
> > > [hans at shalem uguru]$ ls -l abituguru.o abituguru.ko
> > > -rw-rw-r-- 1 hans hans 232871 Mar 8 15:14 abituguru.ko
> > > -rw-rw-r-- 1 hans hans 154472 Mar 8 15:14 abituguru.o
> > >
> > > Without loops:
> > > [hans at shalem uguru]$ ls -l abituguru.o abituguru.ko
> > > -rw-rw-r-- 1 hans hans 231191 Mar 9 21:12 abituguru.ko
> > > -rw-rw-r-- 1 hans hans 152816 Mar 9 21:11 abituguru.o
> >
> > These numbers are really too high, you must have some debugging option
> > enabled. Please provide the same numbers without debugging so that the
> > comparisons make more sense.
>
> Erm, how do I do that? Also they might be correct this is x86_64 which
> comes with almost twice as big code and: "wc -l abituguru.c" gives 1473.
Granted, drivers are a bit larger on x86_64 but not to that extent. I
have w83627hf.o around 35 kB on x86 and 51 kB on x86_64, for example.
Please check in your kernel configuration if you have some kernel-wide
debugging option set. These options are under Kernel hacking > Kernel
debugging (most of them are named CONFIG_DEBUG_*, but not all).
I remember that Jim Cromie reported similarly incredible sizes when he
was working on the pc87360 driver. I don't remember what the cause was
(if I ever knew it).
> Sorry no figures, it just doesn't feel right todo all this opens / close
> if we can pass it all in one file. Also be aware that each open and each
> read and each close will cause a userspace <-> kernel space context
> switch syscalls are expensive. Doing this the lots of files way just
> doesn't feel right for me (I've learned programming on a 8051, so 9k
> additional data is _huge_ to me).
Well we're not talking about 8051 here (whatever it is) so we have to
think in terms of hardware which actually runs Linux, and 9 additional
kB really isn't much in that respect (or at least not enough to
invalidate my proposed interface.)
However, your point about context switching is interesting. I have to
admit I did not think about it at all until now. Greg, can you please
comment on this point? Imagine that we have 200 sysfs files total for a
driver, and an application which is polling half of them every two
seconds, this means an average 50 such context switches per second.
Does it sound bad?
(As a side question, I am wondering if inotify could be used instead of
polling here? Does inotify work on sysfs? If so maybe this addresses
Hans' objection altogether.)
Greg, you pointed our that there were large amounts of sysfs files in
other areas of the kernel, however do these other areas repeatedly poll
the values found in these sysfs files, like we do to the hwmon drivers?
Thanks,
--
Jean Delvare
^ permalink raw reply [flat|nested] 11+ messages in thread
* [lm-sensors] Memory usage of sysfs files
2006-03-08 22:36 [lm-sensors] Memory usage of sysfs files Hartmut Rick
` (7 preceding siblings ...)
2006-03-10 13:46 ` Jean Delvare
@ 2006-03-10 14:09 ` Jean Delvare
2006-03-10 15:28 ` Jim Cromie
9 siblings, 0 replies; 11+ messages in thread
From: Jean Delvare @ 2006-03-10 14:09 UTC (permalink / raw)
To: lm-sensors
Hi Hans,
> I've given this some more thought and I've decided against implementing
> my way for a couple of other drivers to be able to really measure the
> difference. I believe we agree my way is somewhat more efficient, but
> that the differences aren't big enough to swing the scales to my way
> only on the efficiency reasons.
>
> Instead of spending more time coding test-cases I'd rather just see a
> decision soon and (if nescessarry) I'll update the uguru driver to match .
>
> There are 2 arguments I would still like to make in favor of my way though:
>
> 1) you initially did it this way too for atleast one driver. Remember,
> usually your first hunch is a good one.
Doesn't work for me, it seems. See my automatic fan speed control
interface :(
If I did it that way for the f71805f, this was mostly suggested by the
design of the chip itself. If you look at the code you'll see that it's
really simple. This one chip was almost meant for your proposal. But
for many other chips it wouldn't be that nice at all.
Also, I did this without really thinking of all the other features
(separate high/low alarms, input faults, beeps, etc...) that other
drivers would need, and ideas cannot always be generalized.
> 2) I've also written support for libsensors and the sensors application
> when doing things my way and that fits nicely. I'm afraid that the many
> files way will be a problem with the current libsensors. I don't want to
> now wath the lib and the progs chips.* will look like. 100+ additional
> entries for the uguru alone! And I don't think the chip specific reading
> and report code in the sensors application will get any pretter too.
This will look fat and ugly, no matter which alarms interface we go
with. This is due to the broken design of libsensors, there's not much
we can do until then. And this isn't specific to the uguru either.
Having seperate alarm files would simply cause you to pass feature IDs
for alarms instead of pre-computed booleans to print_abituguru_in and
friends. It really doesn't change much in terms of number of lines of
code, just try it.
> Now I know that libsensors is destined to be replaced, but it will be
> around for a while I think.
True, but the new interface is clearly meant for the future library, so
I really don't care if it looks less than optimal for the old library.
> I've attached a patch adding support for the uguru to both lib and prog
> with alarm reporting to give you an idea how this will look like when
> doing things my way. Please note that this patch needs updating for
> 2.10 / cvs and thus won't apply.
You may spare some lines in lib/chips.h by defining feature IDs as
parametrized macros rather than constants. Given that this is how you
end up using these values, this would be cleaner too. That's what I
did for the f71805f, if you want to take a look.
Also, I invite you to leave some room after each set of values. Having
SENSORS_ABITUGURU_IN10 = 11 and SENSORS_ABITUGURU_IN0_MIN = 12 doesn't
look good at all - what will you do if the next version of the uguru
has a 12th voltage input?
Thanks,
--
Jean Delvare
^ permalink raw reply [flat|nested] 11+ messages in thread
* [lm-sensors] Memory usage of sysfs files
2006-03-08 22:36 [lm-sensors] Memory usage of sysfs files Hartmut Rick
` (8 preceding siblings ...)
2006-03-10 14:09 ` Jean Delvare
@ 2006-03-10 15:28 ` Jim Cromie
9 siblings, 0 replies; 11+ messages in thread
From: Jim Cromie @ 2006-03-10 15:28 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
>
> Please check in your kernel configuration if you have some kernel-wide
> debugging option set. These options are under Kernel hacking > Kernel
> debugging (most of them are named CONFIG_DEBUG_*, but not all).
>
> I remember that Jim Cromie reported similarly incredible sizes when he
> was working on the pc87360 driver. I don't remember what the cause was
> (if I ever knew it).
>
It never came up that they were 'incredible', but you did comment on
DEBUG and
the validity of size comparisons. Ive got various kernels lying around,
the sizes of
the .ko files is either ~30kb or ~135kb.
Various DEBUG flags are strong determining factors, but Ive not turned
the flags off,
as they dont interfere with checking sizes before & after other patches.
FWIW, /usr/bin/size gives more info than wc or ls -l, and it seems to
eliminate
some of the 'noise' in the results, esp those due to CONFIG diffs
soekris:/lib/modules# size `find . -name pc87360.ko `
text data bss dec hex filename
14893 3400 16 18309 4785
./2.6.16-rc5-mm2/kernel/drivers/hwmon/pc87360.ko
13370 2780 16 16166 3f26
./2.6.15-sk/kernel/drivers/hwmon/pc87360.ko
12029 2972 16 15017 3aa9
./2.6.15.1-ipipe-103-sony/kernel/drivers/hwmon/pc87360.ko
13876 3085 16 16977 4251
./2.6.14-1.1656_FC4/kernel/drivers/hwmon/pc87360.ko
12029 2972 16 15017 3aa9
./2.6.15-ipipe-103-sony/kernel/drivers/hwmon/pc87360.ko
13800 3085 16 16901 4205
./2.6.15-1.1831_FC4/kernel/drivers/hwmon/pc87360.ko
13806 2740 16 16562 40b2
./2.6.16-rc5-sk/kernel/drivers/hwmon/pc87360.ko
11894 3276 16 15186 3b52
./2.6.16-rc1-sony/kernel/drivers/hwmon/pc87360.ko
13800 3085 16 16901 4205
./2.6.15-1.1830_FC4/kernel/drivers/hwmon/pc87360.ko
13869 3368 16 17253 4365
./2.6.15-mm4-sony/kernel/drivers/hwmon/pc87360.ko
13117 2780 16 15913 3e29
./2.6.15.6-sk/kernel/drivers/hwmon/pc87360.ko
13117 2780 16 15913 3e29
./2.6.15.6-ipipe-121-sk/kernel/drivers/hwmon/pc87360.ko
14572 3176 16 17764 4564
./2.6.16-rc5-mm2-sk/kernel/drivers/hwmon/pc87360.ko
BTW, I'll take this (on-topic) opportunity to remind you of 2 patches I
sent on 1/26:
1. hwmon-pc87360-use-sensor-attr-2.patch
This converts SENSOR_ATTRs to SENSOR_ATTR_2s,
and add a bunch of #defines for the new member's values.
2. hwmon-pc87360-sysfs-combo-callbacks.patch
This combines individual (show|set)_Sensor_Attr callbacks into
(show|set)_Sensor callbacks that handle all that Sensor's Attrs.
this results in a non-trivial size reduction (b4, after)
14588 3224 16 17828 45a4 A-2/drivers/hwmon/pc87360.ko
13124 3224 16 16364 3fec A-3/drivers/hwmon/pc87360.ko
ie about 10%
Combining all show_X (for all X) together is possible, but would
compromise clarity,
and would save only ~ 1/5 th additional space.
hth ;-)
-jimc
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2006-03-10 15:28 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-08 22:36 [lm-sensors] Memory usage of sysfs files Hartmut Rick
2006-03-09 19:41 ` Jean Delvare
2006-03-09 20:05 ` Hans de Goede
2006-03-09 20:06 ` Hans de Goede
2006-03-09 21:05 ` Jean Delvare
2006-03-09 21:41 ` Hans de Goede
2006-03-09 23:16 ` Hans de Goede
2006-03-10 11:11 ` Hans de Goede
2006-03-10 13:46 ` Jean Delvare
2006-03-10 14:09 ` Jean Delvare
2006-03-10 15:28 ` Jim Cromie
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.