linux-kernel - Re: [PATCH] ata: ahci: power off unused ports

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20080603094903.280775ae@appleyard>
Date:	Tue, 3 Jun 2008 09:49:03 -0700
From:	Kristen Carlson Accardi <kristen.c.accardi@...el.com>
To:	Jeff Garzik <jeff@...zik.org>
Cc:	Mark Lord <liml@....ca>, Alan Cox <alan@...rguk.ukuu.org.uk>,
	Pavel Machek <pavel@....cz>, Theodore Tso <tytso@....edu>,
	linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ata: ahci: power off unused ports

On Mon, 02 Jun 2008 14:38:55 -0400
Jeff Garzik <jeff@...zik.org> wrote:

> Kristen Carlson Accardi wrote:
> > On Mon, 02 Jun 2008 14:15:27 -0400
> > Jeff Garzik <jeff@...zik.org> wrote:
> > 
> >> Kristen Carlson Accardi wrote:
> >>> Here you can see that it would be very easy for another driver
> >>> to just add code to set the NO_HOTPLUG flag and then this code
> >>> will work for them as well, since we power off the phy using
> >>> DET which is specified by SATA.
> >>> here's that code:
> >> Agreed.  The main discussion in this sub-thread is more about user 
> >> interface.  The user interface in this case, a module option, is 
> >> specific to AHCI.
> >>
> >> Surely you can see how it is a bit silly to force each driver writer to 
> >> create the same user interface in each driver, to support a generic concept.
> >>
> >> 	Jeff
> >>
> >>
> > Not all drivers will need a user interface to turn off hotplug
> > I would think.  At any rate - I would think it'd be better to let
> > driver writers decide how they want their drivers to behave wrt
> > hotplug and power instead of forcing a generic policy on everyone.
> > 
> > This patch would provide users of AHCI controllers a way to save
> > power now, while you work on the grand scheme for polling/turning on off
> > hotplug via sysfs.  It's an interim solution that impacts nobody but
> > ahci users and is can be easily integrated into whatever solution you
> > eventually work out.
> 
> It's a one-off driver-specific interface that will have to be supported 
> even after the same feature is available via libata core...  that's not 
> the path to scalable, sustainable engineering.

Jeff - I think you are misunderstanding what the module parameter 
is doing.  It is not a substitute for the sysfs interface that you
proposed.  You would not want to get rid of it ever.  What it does
is set the default of the AHCI driver to favor the BIOS settings
for which ports are hotpluggable.  the parameter is there to allow
users to override the BIOS settings at module load time.  Once you
get your sysfs interface in, you would then be able to override
the default at run time.  

> 
> I know user interfaces are annoying because you have to think about 
> chips other than your own, but that's life.  Other hardware vendors have 
> to do it too.

User interfaces are annoying because it takes 2 maintainers to agree
on them, that is all.  Surely you remember how long it took AN to
get in?  It was about 9 months.  This patch can go in now while we
spend 9 months deciding how to do the ultimate sysfs interface.

> 
> Letting each driver have a different user interface is /unfriendly/ to 
> both developers users.  It's easiest for Intel kernel developers, but 
> that is not our target audience :)
> 
> The biggest power savings for the largest amount of users can be had if 
> you take a moment and figure out what's best for Linux, rather than what 
> is best for Intel.
> 
> Because you can be damned sure SATA users with non-AHCI chips want this 
> power savings too...  let's not put roadblocks to that in place in the 
> beginning (by adding one-off interfaces).

As I've been trying to explain to you.  a) this isn't a one off interface.
b) this patch doesn't prevent you at all from adding a generic interface
later.  Perhaps you have a non-technical reason for not wanting this
patch, but I'd like to make sure there's nothing here that you are
just not understanding or something that I can fix.  I obviously can't
fix your idea that I only care about Intel chips, not matter how utterly
wrong and not supported by the facts that is.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/