On Wed, 01 Aug 2007 02:48:42 +0900
Tejun Heo [off-list ref] wrote:

Hello, Kristen.

Kristen Carlson Accardi wrote:

quoted

On Tue, 31 Jul 2007 23:45:25 +0900
Tejun Heo [off-list ref] wrote:

quoted

Anyways, I don't really think this attribute belongs to SCSI sysfs
hierarchy.  There currently isn't any alternative but sysfs is part of
userland visible interface and putting something into SCSI sysfs
hierarchy just because libata doesn't have one doesn't look like a good
idea.

sysfs isn't far from being detached from kobject and driver model.  I
think it would be best to wait a bit and build proper libata sysfs
hierarchy which won't have to be changed later when libata departs from
SCSI.  Well, it isn't really a good way but IMHO it's better than
sticking ATA power saving node into SCSI sysfs hierarchy.

"Wait a bit" could be a very long time.  Who is working on building this
new libata sysfs support now?

I happen to be working on sysfs these days and guess why I started
working on sysfs. :-)

Disassociating sysfs from driver model is probably one or two patchsets
away.  It could have happened earlier but I wanted to pace things a bit
so that the changes receive some testing through release cycles.  Check
how many sysfs related changes went through .23-rc1 merge window and
expect about the same influx during the next cycle; with some luck, it
can be complete before .24-rc1 window.

So at current rate of development and kernel release schedule, the best
possible scenario is still 6 months away - whereas this patchset is already 
tested and ready for merging now.

quoted

If the answer is "no one", which I think it 
may be, do you want to hold up a feature that actually helps many people
for possibly 6 months or more just because we have to go through scsi
right now for our sysfs interface?

I don't necessarily want to but delaying it might be the right thing to
do.  Anyways, if we're going to do an interim solution, I don't think
mainline SCSI sysfs hierarchy is the right place.  Do it with module
parameter which carries large "to be deprecated sign" or maintain out of
tree patches.

Out of tree patches don't work.  Nobody tests them.  A module parameter
will not work - we need to be able to expose the sysfs interface so that
users may chose to turn the feature off and on at will - mainly according
to their usage.  For example, at boot time - you want ALPM off to maximize
performance.  Lets say when you are plugged into the wall socket, you leave
it off, but then when you go on battery power you would want to enable it.

quoted

on top of that, the last mail I 
got from James on this subject indicated that if we kept our granularity
large with the power savings levels, SCSI can actually take advantage of
this as well.  Sure, we may have to tweak things around later, but isn't
this what we do routinely?  Holding up valuable features from the kernel 
because things aren't perfect yet seems really broken.

As far as your complaints about broken hardware go, keep in mind that
the patch set does provide a method of adding these disks to a blacklist,
so I don't see that as a problem.  And, the default for this feature is
"off", and user space would have to explicitly enable it.

This is gonna be a bit long.  Please be patient.

Due to the generosity of the ATA committee, we have, by default, at
least two ways to achieve link PS - HIPS and DIPS.  I dunno why but
someone thought we needed two.  And then, ahci people thought automatic

They thought we needed two because sometimes the device knows when it
will be idle faster than the host can. this is why ALPM can determine
idleness faster than any software algorithm on the host cpu can.  You
can use ALPM without having HIPM.  You can also use it without having 
DIPM.

HIPS would be nice, which I fully agree, and added ALPM.  Unfortunately,
they mandated ALPM to kick in the moment command engine becomes idle, so
most current implementations suffer unnecessary performance hit when
ALPM is active.

"unnecessary" is subjective and at the moment, untrue.  You 
have to make performance/power tradeoffs with anything, whether it's 
CPU or your AHCI controller.  It's the current cost of getting out of these 
sleep states even if you aren't using ALPM but just doing HIPM or DIPM 
manually.  But, this is why it's so critical to allow the user to 
control when ALPM is enabled dynamically - which this patchset does.
The medium power setting is provided for users who wish to not go to
SLUMBER and get the higher latency cost but still have some power savings.

We have this crazy number of combinations.  HIPS, DIPS,
not-so-hot-looking ALPM and probably some number of broken devices which
may be happy with some method but not with others.  Some might be happy
with PARTIAL but vomit on SLUMBER.  I might be being too pessimistic but
my last two years in IDE/ATA land taught me to be pessimistic about
hardware quality.  Heck, I bet you some of non-intel ahci
implementations which claim to support ALPM will crap themselves when
actually told to do so.

If we're gonna do this like NCQ and gonna put knowledge about all the
combinations into the driver.  The suggested interface is good enough.
Heck, we probably don't even need three levels.  On and off should be
enough if things are done right as link PS doesn't have to incur
noticeable performance degradation.  But to do that, we'll need to test
a lot of combinations and it's gonna be much harder than NCQ (some ahci
implementations don't seem to actually enter PS mode when instructed to
do so via SControl but turning off the controller saves a lot of power.
 Maybe ALPM works better on such cases) and the blacklist will probably
be longer.

I understand you are being cautious based on your prior experience, but
again we still have no data to show that we are going to have a lot of
problems.  Perhaps we should proceed optimistically and deal with problems
when they actually occur rather than block something on a bet.

The alternate way is to export flexible interface to userland and let
userland decide and if we're gonna do that.  SCSI sysfs just isn't the
place.

How about a patch? I'd love to have a flexible interface to userland,
it was my goal to provide this with the sysfs files.  The requirement
is that the users be able to turn ALPM off and on dynamically, and to
be able to chose the power savings level you want - i.e. SLUMBER vs.
PARTIAL - perferrably not using those terms since users really shouldn't
need to know AHCI terminology just to chose a power management level.