Hi,
Jeff Garzik wrote:
NAK.
Module parameters in libata are largely for turning off/on major
features, during a transitional testing period. They largely provide
users with workaround solutions to "it won't boot" type problems.
I also think you have been seduced by the relative ease of adding this
control to libata, as opposed to the preferred alternative: digging
through sysfs objects to find the right one to which attributes should
be added.
As an aside, in addition to per-controller (or per-port) sysfs powersave
controls, libata should [eventually] recognize the external "laptop
mode" setting. When laptop mode == ON, program powersave aggresively.
* I agree that cross-port synchronization is not a particularly nice
thing to implement, but it's a nice thing to have from user's POV.
Also, it is necessary to implement any kind of global control like
laptop_mode. I think it's a necessary evil.
* I want to export per-port and eventually per-link powersave interface
but the problem is that libata doesn't have any sysfs representation
other than what's supplied by SCSI, which means empty links don't have
anything to represent them in sysfs. Unfortunately, sysfs is tied to
driver implementation ATM, so we can't start building sysfs tree on
as-needed basis.
So, until sysfs representation is detached from internal driver data
structure or ATA moves out of SCSI, I don't really see how we can add
those fine-grained knobs. Also, although having fine knobs is a good
thing, I think powersave with global control goes a long way by itself
and global control is needed with or without fine knobs.
However, one thing I can think of is to implement (yet) bogus ATA bus
and add global controls there. e.g. /sys/bus/ata/powersave. What do
you think about this?
* For global control, overloading laptop_mode doesn't seem to be a good
idea. That's a VM-specific knob with very specific role. I think we
should add system-wide sysfs node - maybe /sys/power/powersave? - which
advises the kernel about power consumption. It can propagate powersave
requests using notifier chain and both VM and libata can hook into it to
control laptop_mode and libata powersaving respectively.
In general, though, we should peek to minimize power usage for the
general case, where feasible. People with huge server closets like
Google need every ounce of power savings possible.
I'm not too sure about enabling powersave by default, if that's what you
mean. Yeah, people with large server farms will definitely benefit from
powersave. That's why I'm thinking of implementing full powersave
including both HIPS and DIPS on PMP. But powersave also has the
possibility of causing malfunctions on devices which used to work okay.
I've seen sil3112 family of controllers malfunction when powersave kicks
in and won't be surprised to see other controllers and drives show
erratic behavior on powersave.
Another problem is that, however minute, powersave does introduce extra
delay when waking up and thus can negatively affect performance. The
performance drop should be negligible on most non-SSD devices but it's
still there.
With regards to the patch content: other than the control interface,
the implementation looks OK.
Nice, thanks.
--
tejun
-
: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
