DragonFlyBSD

Johannes Hofmann wrote:

In which context are sysctl callbacks run? How would I avoid races
when modifying flags or accessing registers (to disable the PRCE intr
for example)?

In the context of the calling thread, with the mp lock held.

Maybe setting flags with with atomic operations and do the real work
in the per port thread?

You could send a message to the port thread which in turn does all the work, without changing anything from within the sysctl.

cheers
simon

:Here is a first patch to add support for AHCI HBA initiated power
:management to ahci(4):
:http://leaf.dragonflybsd.org/~hofmann/ahci_pwrmgmt.patch
:
:To enable it you need to set
: hint.ahci/powermanagement=1
:for medium or
: hint.ahci/powermanagement=2
:for aggressive power management in /boot/loader.conf.
:Setting it to 2 reduces performance quite a bit!
:On the positive side I see the 1W reduction in power consumption as it is
:reported for Linux.
:
:Obviously this should be tunable at runtime, but I have some
:questions:
:
:In which context are sysctl callbacks run? How would I avoid races
:when modifying flags or accessing registers (to disable the PRCE intr
:for example)?
:Maybe setting flags with with atomic operations and do the real work
:in the per port thread?
:
:Cheers,
:Johannes

If you set up a SYSCTL_PROC (you can probably find examples
    in various places in the kernel) then whenever the sysctl is
    read or written the procedure callback is made.

In order to safely modify the feature while AHCI is operational
    you probably just need to get the AHCI port lock.  There are
    numerous examples in the ahci code of calls to e.g. 
    ahci_os_lock_port(ap).  So you'd get the lock, modify the
    feature, and then release the lock.

The patch looks pretty good, but needs one or two more passes
    before comitting.  There are capability bits for AHCI features
    and the power management feature must be conditionalized on the
    capability existing.  I think the one that applies is
    AHCI_REG_CAP_SALP.  See other AHCI_REG_CAP_* uses in the ahci
    code for examples.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

Matthew Dillon <dillon@apollo.backplane.com> wrote:

If you set up a SYSCTL_PROC (you can probably find examples
in various places in the kernel) then whenever the sysctl is
read or written the procedure callback is made.

In order to safely modify the feature while AHCI is operational
you probably just need to get the AHCI port lock. There are
numerous examples in the ahci code of calls to e.g.
ahci_os_lock_port(ap). So you'd get the lock, modify the
feature, and then release the lock.

The patch looks pretty good, but needs one or two more passes
before comitting. There are capability bits for AHCI features
and the power management feature must be conditionalized on the
capability existing. I think the one that applies is
AHCI_REG_CAP_SALP. See other AHCI_REG_CAP_* uses in the ahci
code for examples.

I have updated the patch at
http://leaf.dragonflybsd.org/~hofmann/ahci_pwrmgmt.patch
accordingly.
Link power management can now be switched on and off with a sysctl.
On my system it is "hw.ahci0.0.link_pwr_mgmt"
(0 = disabled, 1 = medium, 2 = aggressive).

Thanks for the help on the ML and on irc,
Johannes

:I have updated the patch at
:http://leaf.dragonflybsd.org/~hofmann/ahci_pwrmgmt.patch
:accordingly.
:Link power management can now be switched on and off with a sysctl.
:On my system it is "hw.ahci0.0.link_pwr_mgmt"
:(0 = disabled, 1 = medium, 2 = aggressive).
:
:Thanks for the help on the ML and on irc,
: Johannes

This is looking very good.  Just one more thing and I can commit it
    for this release.  The AHCI driver can be unloaded... the sysctl
    nodes have to be destroyed when it is.  It can be done in
    ahci_port_free().  Keep in mind that the function can be called
    on a partially initialized port so conditionalize the destruction of
    the sysctl node on it having been installed.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

Matthew Dillon <dillon@apollo.backplane.com> wrote:

:I have updated the patch at
:http://leaf.dragonflybsd.org/~hofmann/ahci_pwrmgmt.patch
:accordingly.
:Link power management can now be switched on and off with a sysctl.
:On my system it is "hw.ahci0.0.link_pwr_mgmt"
:(0 = disabled, 1 = medium, 2 = aggressive).
:
:Thanks for the help on the ML and on irc,
: Johannes

This is looking very good. Just one more thing and I can commit it
for this release. The AHCI driver can be unloaded... the sysctl
nodes have to be destroyed when it is. It can be done in
ahci_port_free(). Keep in mind that the function can be called
on a partially initialized port so conditionalize the destruction of
the sysctl node on it having been installed.

Ah right. Patch updated.

Johannes

Ok, its committed!

-Matt

Matthew Dillon <dillon@apollo.backplane.com> wrote:

Ok, its committed!

Cool! Thanks,

Johannes

I don't see it in the commits; someone who does, please post the commit id here
and set the status to resolved.

Cheers,
Alex Hornung

Commited in f5caeaa06f8d843ee0e550a55bb06b4e67ac52bc ;)

Cheers,
Alex Hornung

:Alex Hornung <ahornung@gmail.com> added the comment:
:
:Commited in f5caeaa06f8d843ee0e550a55bb06b4e67ac52bc ;)
:
:Cheers,

Yah, sorry, I mistakenly merged that with another commit.  I have
    made a followup commit to ahci.c to give proper credit.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>