[Toybox] [PATCH] acpi: change when to recurse

[Rob Landley]

On 10/06/14 17:23, Isaac Dunham wrote:
> On Mon, Oct 06, 2014 at 12:36:08AM -0500, Rob Landley wrote:
>> On 10/05/14 23:13, Isaac Dunham wrote:
>>> On Sun, Oct 05, 2014 at 04:28:21PM -0500, Rob Landley wrote:
>>>> On 10/04/14 15:52, Isaac Dunham wrote:
>>>>> acpi: never follow symlinks, always recurse in dirs.
>>>>>
>>>>> /sys/class contains several directories, each containing symlinks to
>>>>> devices of a given type.
>>>>> The symlinks are where we want to end recursion.
>>>>> This makes for a simpler, cleaner, and more correct check, avoids needless
>>>>> recursion, and shrinks the text segment by 25 bytes (at least here).
>>>>
>>>> I _totally_ got bit by this doing mdev back on busybox... repeatedly:
>>>>
>>>> http://lists.busybox.net/pipermail/busybox/2006-February/052297.html
>>>> http://lists.busybox.net/pipermail/busybox/2008-July/066210.html
>>>>
>>>> I.E. things that were directories can become symlinks, and vice versa.
>>>> I'm reluctant to trust it.
>>>
>>> Ok, thanks for the warning.
>>> Is there a simple function to check absolute depth?
>>> (ie, how many dirs down from / the current dirtree entry is)
>>
>> I just recurse on the node's parent pointer are count up until I hit the
>> null at the top. (Haven't bothered to make a function for it, it's a
>> simple for loop.)
>>
>> This doesn't tell you how far you are from /, it tells you how far you
>> are from the top directory you started recursing from. (Which could be a
>> path arbitrarily deep from root, but is generally a known quantity.)
> 
> In other words, "No, but relative depth should be fine."
> Thanks, I can probably make it work.

If you really want to xabspath(dirtree_path(node, 0)) and then count
slashes, it's possible. Just not recommended.

>>> I *really* hate the first way I wrote; it's not even a magic number, it's
>>> a disgusting hack that *will* break if it ever gets called outside 
>>> /sys/class/power_supply/.  And it certainly can't be copied for if we
>>> recurse through /sys/class/thermal and /sys/class/power_supply.
>>
>> What are the actual rules? What are you looking for in the filesystem?
> 
> The actual rules:
> /sys/class/$CURRENT_CLASS/* should be the final nodes we visit.

Easy enough to implement with, as you say, relative depth. :)

>>> recurse through /sys/class/power_supply and read a few different files.
>>
>> My LCD has a cooling device? Really?
>>
>> Ok, it's got a max state of 9 and a cur state of 0. Meaning...?
> 
> /sys/class/thermal contains fans, and all the things that can *readily* be
> throttled back to cut power (ie, LCDs and CPUs).
> 
> For a CPU, the CPU speed steps are numbered, with 0 as "slow as possible"
> and max_state corresponding to the maximum speed.
> 
> Ostensibly, LCD max brightness is max_state; dimmest (or is it off?)
> is cur_state=0.
> In reality, many laptops are wired backwards, so max_state actually means
> the dimmest state possible for them.
> 
> ...Do you have anything in /sys/class/pwm?

$ ls /sys/class/pwm
ls: cannot access /sys/class/pwm: No such file or directory

Same on both boxes.

>> (Devices 0 and 1 are both "Processor" and cur_state 0, one has max state
>> of 3 the other 10.)
>>
>> So... yay thermal. That's easy to recurse on, but all we do is print out
>> the numbers?
> 
> Believe it or not, that's all we do.
> After all, any interpretation *will* be wrong somewhere.

Lovely.

>> (And yes, give it its own dirtree callback function. If !parent then
>> recurse following symlinks, otherwise openat() cur_state max_state and
>> type relative to dirtree_parentfd().)
> 
> For temperature, we'll need to recurse through more of /sys/class;
> my phone has a temperature sensor on its battery, and there's no telling
> where else we might find one.
> That means
>   if (!tree->parent || !tree->parent->parent)
>     return DIRTREE_RECURSE|DIRTREE_SYMFOLLOW;

Reasonable.

>>>>> The above method does ignore quite a few sensors;
>>>>> there's no obvious way to get HDD temps besides these commands:
>>>>>  smartctl -A -d ata /dev/sda |grep Temperature
>>>>> # or (Hitachi-only, but no spin-up)
>>>>>  hdparm -H /dev/sda
>>>>
>>>> strace, the breakfast of champions.
>>>
>>> Yes, I've tried that and ltrace.
>>> hdparm calls ioctl(fd, SG_IO, ...) with a very large struct that I haven't
>>> figured out.
>>
>> SG_IO sounds like it's sending its own scsi command. Possibly:
>>
>> http://www.t10.org/lists/2op.htm
>>
>> I can dig down into the kernel and try to reverse engineer it... Ah,
>> there's docs:
>>
>> http://tldp.org/HOWTO/SCSI-Generic-HOWTO/sg_io.html
>> http://tldp.org/HOWTO/SCSI-Generic-HOWTO/pexample.html
>>
>> Lots of docs:
>>
>> http://sg.danny.cz/sg/sg_io.html
> <snip> 
>> There are standard scsi commands for basic status info, which are
>> repeated in a bunch of places (such as the wikipedia page about scsi).
> 
> hdparm -H is supposedly a Hitachi-specific extension, which is 
> preferable to the standard way because it doesn't spin up the disk to read
> temperature.

Indeed, but the hitachi-specific extension is probably just a custom
scsi command packet. (It has to send a packet to talk to the onboard
electronics. Whether they decide to spin up the disk is their call.)

> I'm inclined to avoid reading the disk temperature via scsi commands.
> It would make for a bit of a mess and could have "surprising" effects on
> time you can run on battery, depending on the disk vendor.

I happily leave this to you, dunno enough about it to have an opinion.

> Now if someone decided to write a driver that can export it to sysfs,
> I would be fine with reading that.

That sounds like a question for James Bottomley.

>> That would actually be a fun thing to try thermal sensor stuff on,
>> because the reason I know it has an overheating problem is dmesg says
>> "thermal limit reached, processors throttled" when I leave it doing a 3
>> hour build.
> 
> Much better than my Thinkpad.
> Back in ~2009-2010 (late netbook craze era) AMD decided to put one of its
> processors in the "mobile" market; it was higher power use than the Atom,
> but it was (a) 64-bit, (b) AMD, (c) faster in terms of real performance,
> and (d) had virtualization support.

Yes. AMD-C60. that's what's in my netbook.

> Besides that, it came with a Radeon GPU, instead of Poulsbo.
> So when I found that out that Lenovo had made a cheap 11.6" laptop with that,
> I went for it.
> And I went for the dual-core version.

It's craptacular, isn't it?

Acer Aspire one 722 here. It manages to be _slower_ than the Atom (even
before you take the 2 way vs hyperthreaded 4-way in that my D255E-1802
had) but it can take 8 gigs of ram and the atom maxes at 2.

> The sole problem is that the fan wasn't quite enough for a single-core;
> it would frequently shut down when I tried to do something like build
> a kernel.

Acer manages to cool it just fine. The fan seldom even comes on.
Possibly mine's lower GHz.

> (If I would blow out the fan, lock it in "powersave" mode at 800 MHz,
> set the fan to full-speed, and run it with make -j1 at nice 19 or 20,
> it would "only" get to the mid-80 degree C range. Thermal shutdown is
> 92 C.)

That's... sad.

>> Hmmm...
>>
>> $ ls /sys/class/thermal
>> cooling_device0  cooling_device3  cooling_device6  thermal_zone0
>> cooling_device1  cooling_device4  cooling_device7
>> cooling_device2  cooling_device5  cooling_device8
>>
>> $ ls /sys/class/thermal/thermal_zone0
>> device  passive  power      temp               trip_point_0_type  uevent
>> mode    policy   subsystem  trip_point_0_temp  type
>  
> thermal_zone0 is probably the fan.
> Speaking of which, do you have a fan driver that allows setting fan speed?

Maybe? The entire bottom back strip of this netbook seems to be a giant
heat sink (gets a bit warm on the knees in shorts when doing
cpu-intensive stuff) so the fan doesn't come up much. If I listen at the
ethernet port it's got one running, though. Dunno what would make it go
faster.

(The System76 meanwhile is hissing like an annoyed cat. I have sometimes
forced all the processors to 1.7ghz and locked there to get the fans to
shut up.)

> See previous comment about backlights being wired backwards.

Indeed. My main concern here is not knowing what the requirements are
for this command (there being no spec), but if "spit out the raw data
from /sys" is enough of a mandate, by all means...

> Incidentally...
> I was thinking that I might need to use glob() in the callback for the
> Thinkpad temperatures.
> But it seems that I could just do 
>   err = glob("/sys/class/*/*/temp", 0, NULL, &pglob);
>   glob("/sys/class/*/*/temp*_input", err?GLOB_APPEND:0, NULL, &pglob);
> 
> And check if there are any matches.

I'm not sure what you're trying to do here, but it seems unlikely that
you'd need dirtree _and_ glob here?

> Thanks,
> Isaac Dunham

Rob

 1412638096.0