[ugly patch] Save .15W-.5W by AHCI powersaving
Hi! This is a patch (very ugly, assumes you have just one disk) to bring powersaving to AHCI. You need Alan's SCSI autosuspend (attached) patch as a base. It saves .5W compared to config with disk spinning, and even .15W compared to hdparm -y... on my thinkpad x60 anyway. It is also mandatory first step towards sleepy linux ;-). Pavel diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 29e71bd..0197b1f 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -259,8 +260,8 @@ static void ahci_fill_cmd_slot(struct ah u32 opts); #ifdef CONFIG_PM static int ahci_port_suspend(struct ata_port *ap, pm_message_t mesg); -static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg); -static int ahci_pci_device_resume(struct pci_dev *pdev); +int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg); +int ahci_pci_device_resume(struct pci_dev *pdev); #endif static struct class_device_attribute *ahci_shost_attrs[] = { @@ -268,6 +269,35 @@ static struct class_device_attribute *ah NULL }; +struct pci_dev *my_pdev; +int autosuspend_enabled; + +/* The host and its devices are all idle so we can autosuspend */ +static int autosuspend(struct Scsi_Host *host) +{ + if (my_pdev autosuspend_enabled) { + printk(ahci: should autosuspend\n); + ahci_pci_device_suspend(my_pdev, PMSG_SUSPEND); + return 0; + } + printk(ahci: autosuspend disabled\n); + return -EINVAL; +} + +/* The host needs to be autoresumed */ +static int autoresume(struct Scsi_Host *host) +{ + if (my_pdev autosuspend_enabled) { + printk(ahci: should autoresume\n); + ahci_pci_device_resume(my_pdev); + return 0; + } + printk(ahci: autoresume disabled\n); + return -EINVAL; +} + + + static struct scsi_host_template ahci_sht = { .module = THIS_MODULE, .name = DRV_NAME, @@ -286,6 +322,8 @@ static struct scsi_host_template ahci_sh .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, .shost_attrs= ahci_shost_attrs, + .autosuspend= autosuspend, + .autoresume = autoresume, }; static const struct ata_port_operations ahci_ops = { @@ -2300,8 +2356,12 @@ static int ahci_init_one(struct pci_dev ahci_print_info(host); pci_set_master(pdev); - return ata_host_activate(host, pdev-irq, ahci_interrupt, IRQF_SHARED, + + rc = ata_host_activate(host, pdev-irq, ahci_interrupt, IRQF_SHARED, ahci_sht); + pci_save_state(pdev); + my_pdev = pdev; + return rc; } static int __init ahci_init(void) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 4e31071..5c40ac2 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -380,7 +381,7 @@ enum scsi_eh_timer_return ata_scsi_timed * Inherited from SCSI layer (none, can sleep) * * RETURNS: - * Zero. + * Nothing. */ void ata_scsi_error(struct Scsi_Host *host) { diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index c838e65..0edc25e 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -484,6 +484,8 @@ static int scsi_try_host_reset(struct sc if (scsi_autoresume_host(shost) != 0) return FAILED; + rtn = shost-hostt-eh_host_reset_handler(scmd); + if (rtn == SUCCESS) { if (!shost-hostt-skip_settle_delay) ssleep(HOST_RESET_SETTLE_TIME); @@ -1577,7 +1579,11 @@ int scsi_error_handler(void *data) * what we need to do to get it up and online again (if we can). * If we fail, we end up taking the thing offline. */ +#if 0 + /* libata uses scsi_error_handler to suspend its parts; we deadlock + if we try to autoresume here */ autoresume_rc = scsi_autoresume_host(shost); +#endif if (shost-transportt-eh_strategy_handler) shost-transportt-eh_strategy_handler(shost); else @@ -1591,8 +1597,10 @@ int scsi_error_handler(void *data) * which are still online. */ scsi_restart_operations(shost); +#if 0 if (autoresume_rc == 0) scsi_autosuspend_host(shost); +#endif set_current_state(TASK_INTERRUPTIBLE); } __set_current_state(TASK_RUNNING); diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 233feee..3c598e0 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -67,6 +67,8 @@ #undef SP static struct kmem_cache *scsi_bidi_sdb_cache; +void scsi_run_queue(struct
Re: [ugly patch] Save .15W-.5W by AHCI powersaving
Hi! This is a patch (very ugly, assumes you have just one disk) to bring powersaving to AHCI. You need Alan's SCSI autosuspend (attached) patch as a base. It saves .5W compared to config with disk spinning, and even .15W compared to hdparm -y... on my thinkpad x60 anyway. .. There was a discussion of this here today. Real-life discussion, or something I could read? :-). It makes good use of AHCI-specific features. Has it been tested with a Port-Multiplier yet? I do not know what port-multiplier is, sorry. But it was not really tested. It is not expected to work on any other config than notebook very similar to mine. This is cool enough that we really ought to do a hardware-independent version, so that all SATA interfaces could benefit. Especially ata_piix, but others too. Well, it seems like it is 10 lines per driver once Alan's SCSI autosuspend patches are in... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new regression in 2.6.25-rc3: can't resume from suspend to ram, ata1 errors
On Mon 2008-02-25 16:04:08, Jeff Garzik wrote: Michael S. Tsirkin wrote: git bisect points at this commit: commit 559bbe6cbd0d8c68d40076a5f7dc98e3bf5864b2. power_state: get rid of write-only variable in SATA Hello Pavel -- It looks like this not a write-only variable after all... Yep, right and sorry. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.25-rc2 + smartd = hang
Hi! So that's using the old IDE drivers. And the network and USB are sharing IRQ#11 with each other. If you are going to be using newer kernels like this (2.6.23+), then you might consider shifting those drives over to libata drivers. Yes, that will probably fix it for him, but but... maybe we should fix that regression? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] enclosure: add support for enclosure services
On Wed 2008-02-13 09:45:02, Kristen Carlson Accardi wrote: On Tue, 12 Feb 2008 13:28:15 -0600 James Bottomley [EMAIL PROTECTED] wrote: On Tue, 2008-02-12 at 11:07 -0800, Kristen Carlson Accardi wrote: I understand what you are trying to do - I guess I just doubt the value you've added by doing this. I think that there's going to be so much customization that system vendors will want to add, that they are going to wind up adding a custom library regardless, so standardising those few things won't buy us anything. It depends ... if you actually have a use for the customisations, yes. If you just want the basics of who (what's in the enclousure), what (activity) and where (locate) then I think it solves your problem almost entirely. So, entirely as a straw horse, tell me what else your enclosures provide that I haven't listed in the four points. The SES standards too provide a huge range of things that no-one ever seems to implement (temperature, power, fan speeds etc). I think the users of enclosures fall int these categories 85% just want to know where their device actually is (i.e. that sdc is in enclosure slot 5) 50% like watching the activity lights 30% want to be able to have a visual locate function 20% want a visual failure indication (the other 80% rely on some OS notification instead) When you add up the overlapping needs, you get about 90% of people happy with the basics that the enclosure services provide. Could there be more ... sure; should there be more ... I don't think so ... that's what value add the user libraries can provide. James I don't think I'm arguing whether or not your solution may work, what I am arguing is really a more philosophical point. Not can we do it this way, but should we do it way. I am of the opinion that Hw abstraction is still kernel's job. That's why we have leds exported in sysfs... let vendors have their libraries, but lets put the 'everyone does these' stuff in kernel. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.26-git0: IDE oops during boot
On Wed 2008-02-06 11:53:34, Pavel Machek wrote: Hi! Trying to boot 2.6.25-git0 (few days old), I get BUG: unable to handle kernel paging request at ..ffb0 IP at init_irq+0x42e Call trace: ide_device_add_all ide_generic_init kernel_init child_rip vgacon_cursor kernel_init child_rip Excerpt from config: CONFIG_IDE=y CONFIG_BLK_DEV_IDE=y Disabling CONFIG_IDE made my machine boot, as it was using libata anyway. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ata: ahci: Enclosure Management via LED
Hi! +static int ahci_em_messages = 1; +module_param(ahci_em_messages, int, 0444); +/* add other LED protocol types when they become supported */ +MODULE_PARM_DESC(ahci_em_messages, + Set AHCI Enclosure Management Message type (0 = disabled, 1 = LED); Should you add line in Doc* somewhere? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possibly SATA related freeze killed networking and RAID
On Fri 2007-11-30 10:00:55, Mark Lord wrote: Pavel Machek wrote: On Fri 2007-11-30 13:13:44, Alan Cox wrote: Why does a single spurious interrupt cause it to be shut down? I can It doesn't. see if the interrupt is stuck on and keeps interrupting constantly, but if it's just the occasional spurious interrupt, why not just ignore it and move on? The interrupt is usually level triggered so it continues to create interrupts until you silence it. The thresholds are about 10,000 interrupt events and on newer kernels we also reset the count if we don't see any for a while. That works for most stuff except the thinkpad bluetooth problem. Which is confirmed hw problem now, btw. ... What problem is that, exactly? Spurious interrupt, interrupt link is disabled after ~15 minutes. It seems pretty unique to t61. My Dell has an internal USB BT adapter that briefly appears and then disappears again on resume (or stays if I have enabled it via the BIOS key). I wonder if that has anything to do with the (new in) 2.6.23 pauses that machine has on resume (about every 10th time). No idea, but t61 problem seems different. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possibly SATA related freeze killed networking and RAID
On Fri 2007-11-30 13:13:44, Alan Cox wrote: Why does a single spurious interrupt cause it to be shut down? I can It doesn't. see if the interrupt is stuck on and keeps interrupting constantly, but if it's just the occasional spurious interrupt, why not just ignore it and move on? The interrupt is usually level triggered so it continues to create interrupts until you silence it. The thresholds are about 10,000 interrupt events and on newer kernels we also reset the count if we don't see any for a while. That works for most stuff except the thinkpad bluetooth problem. Which is confirmed hw problem now, btw. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possibly SATA related freeze killed networking and RAID
Hi! kernel: [734344.717844] irq 21: nobody cared (try booting with the irqpoll option) kernel: [734344.717866] Your machine decided to emit interrupt 21 without an apparent reason. Whatever caused that made the kernel shut down IRQ 21 at which point the disk drives on that IRQ were no longer being serviced. Everything on IRQ 21 would have died - which may be why your networking failed too. Hmm, perhaps that 'nobody cared' message should be worded more strongly, and printed and KERN_CRIT? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
size of git repository (was Re: [BUG] New Kernel Bugs)
On Tue 2007-11-13 12:50:08, Mark Lord wrote: Ingo Molnar wrote: for example git-bisect was godsent. I remember that years ago bisection of a bug was a very laborous task so that it was only used as a final, last-ditch approach for really nasty bugs. Today we can autonomouly bisect build bugs via a simple shell command around git-bisect run, without any human interaction! This freed up testing resources .. It's only a godsend for the few people who happen to be kernel developers and who happen to already use git. It's a 540MByte download over a slow link for everyone else. Hmmm, clean-cg is 7.7G on my machine, and yes I tried git-prune-packed. What am I doing wrong? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] New Kernel Bugs
Hi! Suspend to RAM resume hangs on a tickless (NO_HZ) kernel http://bugzilla.kernel.org/show_bug.cgi?id=9275 Kernel: 2.6.23 This is HP notebook nc6320 T2400 945GM No response from developers Maybe I'm optimistic, but I expected Ingo/Thomas to look after nohz problems. nohz=off highres=off fixes more than one suspend problem... ...stuff I've seen with NOHZ even without suspend (cursor blinking irregulary) make me think that nohz perhaps should not be used in production just yet... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Disk spin down issue on shut down/suspend to disk
Hi! Oh... crap, so acpi wants to sync cache on shutdown. I wonder whether it spins down the disk correctly. Does emergency unload count increase after each power down? Also, please post the result of 'dmidecode'. I know that my Compaq X1000-series laptop does do some kind of ACPI games with the disk on ACPI power off (I assume it is putting the disk in standby before power-off at least). It also does this if you boot into DOS, GRUB, etc. and then hit the power button. Could be if the disk is dumb enough to spin up for sync cache and standby when there is nothing to flush, and the kernel does its own standby, this could cause an extra spinup/down.. Yeah, that seems to be what's going on. I don't think we have any other choice than blacklisting those notebooks. This is a mess. How does the other OS cope with this? I'm thinking about using DMI vendor/product match to detect the affected systems but I think it would be better to match the ACPI implementation directly. Is there a way to match specific ACPI implementation? Well.. unless they use some SMM trick, it is ACPI AML code telling kernel to spin the disk down. I guess we could detect that, and simply ignore the request. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Disk spin down issue on shut down/suspend to disk
Hi! firmwarekit-discuss [EMAIL PROTECTED] (added to CC list) see: http://linuxfirmwarekit.org/ But if I understand this problem right, this won't be easy. The ACPI tables are just parsed with system (iasl ...) and syntactical errors/warnings are printed out. I also thought about a test, interpreting the DSDT and read out values of cpufreq tables and sanity check them. AFAIK the linuxfirmwarekit is not designed for that atm. You need to compile in most parts of the acpica code and parse and interpret DSDT/SSDT code yourself in the firmwarekit core or inside a plugin, then do a walk_namespace call or whatever to find the functions/parts you like to examine. This is a lot work and needs a proper design (providing an interface to plugins to let them easily check specific AML/ASL code). Furthermore, we don't really know what we're looking for. How can you tell a given write to an ioport is issuing STANDBYNOW to an ATA disk or trying to power the machine off? Adding to the fun, many modern ATA controller have more than one way to issue a command. Maybe we can match accesses inside regions specified by PCI BARs :-( Hmmm... perhaps we should do it the other way. ACPI is allowed to touch the embedded controller, what else? Maybe we should warn as soon as API touches non-EC I/O port? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/3] AHCI Link Power Management
Hi! I'm not sure about this. We need better PM framework to support powersaving in other controllers and some ahcis don't save much when only link power management is used, do you have data to support this? Yeah, it was some Lenovo notebook. Pavel is more familiar with the hardware. Pavel, what was the notebook which didn't save much power with standard SATA power save but needed port to be completely turned off? Thinkpad x60. Some one Kristen probably used while developing the patch :-). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/3] AHCI Link Power Management
Hi! Yeah, it was some Lenovo notebook. Pavel is more familiar with the hardware. Pavel, what was the notebook which didn't save much power with standard SATA power save but needed port to be completely turned off? Pavel, if you have time, could you measure this with Kristen's patch? Kristen has same machine as me, and I have seen similar '1W' saving with previous version of the patch. I'd trust her results. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/3] AHCI Link Power Management
Hi! I'm not sure about this. We need better PM framework to support powersaving in other controllers and some ahcis don't save much when only link power management is used, do you have data to support this? Yeah, it was some Lenovo notebook. Pavel is more familiar with the hardware. Pavel, what was the notebook which didn't save much power with standard SATA power save but needed port to be completely turned off? Uhuh, now I understand why Arjan wanted me to test. But I have same hw as Kristen, so I assume there must have been something wrong with the old tests. Sorry for confusion. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Loud pop coming from hard drive on reboot
Hi! When I reboot my notebook, it powers off and powers back on. On poweroff a loud snapping noise seems to be coming from the hard drive. Today I noticed there is no shutdown: hda on the console when I reboot. Whne I do a normal poweroff the message is displayed and there is no noise. Should the IDE code be changed so it always shuts down the drive? Well... most machines have reboot handling where they do not cut power to disk drive. That means that shutting down their hdds would unneccessarily wear their drives. ...while we unneccessarily wear *your* drive :-(. How common are notebooks that cut power to disks during reboot? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ATA ACPI (was Re: Linux 2.6.21-rc5)
Hi! So if you have reported a regression in the 2.6.21-rc series, please check 2.6.21-rc5, and update your report as appropriate (whether fixed or still problems with xyzzy). [just got back from vacation, or would have sent this earlier] FWIW, I'm still leaning towards disabling libata ACPI support by default for 2.6.21. Upstream has Alan's fix for the worst PATA problems, but for different reasons, I think PATA ACPI and SATA ACPI support in libata does not feel quite ready for prime time in 2.6.21. Scream now, or hold your peace until 2.6.22... :) I second disabling ACPI for 2.6.21. Ugh.. does that mean we'll have 'regression reports' as in 'it worked ok in -rc5, broken in final? Well, suspend is currently so broken that we'll be flooded by reports, anyway, but could we get at least define in code so that we can tell users to flip it? Or maybe it is enough to make libata dependend on EXPERIMETAL? ...making it dependend on BROKEN should be definitely enough... -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [5/6] 2.6.21-rc3: known regressions
Hi! This email lists some known regressions in Linus' tree compared to 2.6.20. If you find your name in the Cc header, you are either submitter of one of the bugs, maintainer of an affectected subsystem or driver, a patch of you caused a breakage or I'm considering you in any other way possibly involved with one or more of these issues. Due to the huge amount of recipients, please trim the Cc when answering. Subject: resume: slab error in verify_redzone_free(): cache `size-512': memory outside object was overwritten References : http://lkml.org/lkml/2007/2/24/41 Submitter : Pavel Machek [EMAIL PROTECTED] Status : unknown Subject: beeps get longer after suspend References : http://lkml.org/lkml/2007/2/26/276 Submitter : Pavel Machek [EMAIL PROTECTED] Status : unknown Seems fixed in -rc3. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] implement __attribute_discard_text/data__ and use it to leave out PM functions if !CONFIG_PM
Hi! This patch is the result from the following discussion. http://thread.gmane.org/gmane.linux.ide/16475 The problem is that CONFIG_PM affects a lot of low level drivers and scattering CONFIG_PM all over the place is too ugly. This patch... * implements __attribute_discard_text__ and __attribute_discard_data__ (not implemented for modules yet, only for built-ins) * uses them to implement __pm and __pmdata markers * convert libata midlayer and ahci to use it instead of CONFIG_PM __attribute_discard_text/data__ puts the marked symbols in separate sections which are located from VMA 0 and discarded when generating the final image. It's similar to putting those into /DISCARD/ section but won't complain even if the discarded symbols are referenced by active sections. As the discard sections are located from VMA 0, actually dereferencing such symbols will result in OOPS. This trick certainly makes LLDs cleaner but there are also some downsides, so here are the cons. * Cannot depend on the compiler to detect illegal dereferences to discarded symbols. We probably can do this using sparse. * Cannot use the compiler to detect unused but unmarked symbols. This also probably can be done with sparse. * EXPORT_SYMBOL() is nullified, so it will take extra bytes for each symbol marked with __pm. (insignificant) * Implementation involves modifying kernel image, module build process and possibly sparse. It might be too expensive to achieve device driver prettiness, but there are a lot of device drivers out there and a lot of CONFIG_PM's to be added. Also, discard attributes can be used for other purposes too. Any better ideas? Comments? Seems ok to me. Certainly better than config_pm... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] libata: add missing PM callbacks
Hi! [cc'ing Pavel and linux-kernel, hello] Original thread can be read from http://thread.gmane.org/gmane.linux.ide/16475 Jeff Garzik wrote: Tejun Heo wrote: Some LLDs were missing scsi device PM callbacks while having host/port suspend support. Add missing ones. Signed-off-by: Tejun Heo [EMAIL PROTECTED] applied 1-3, though I agree with Alan that a non-ifdef solution should be sought (by the PM PCI people?), where possible Agreed, CONFIG_PM ifdefs are all over low level drivers, libata or not, and ugly as hell. Maybe use separate section, mark functions with __power and drop them at link time is a better idea. With linker tricks, we can make references to __power symbols NULL. How does it sound? Much complexity for little gain. Who is running _without_ CONFIG_PM these days? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] libata: add missing PM callbacks
Hi! Much complexity for little gain. Who is running _without_ CONFIG_PM these days? Embedded people, I guess. The problem here is that if we are gonna support !CONFIG_PM configuration and try to reduce the kernel/module images size for such case, we end up sprinkling #ifdef's all over huge number of device drivers. Ok, in separate mail I see you already did the work... If we determine to drop !CONFIG_PM configuration, I'm happy with that too but we need to determine something here. Alternatives... 1. drop !CONFIG_PM configuration 2. continue to sprinkle #ifdef's over device drivers 3. find out prettier way to mark PM functions ...of 3., and code is nicely generic and fairly simple. I thought it would be too much work to implemente it, and it would get too messy. Given that it is so simple, I believe 3. is the best option. We can still do 1. in future if we feel like so... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pcim_enable_device BUGs for libata devices in 2.6.20-git6
Hi! I'm seeing BUGs like these on all libata-driven controllers when suspending to disk on 2.6.20-git6: sata_nv :00:07.0: resuming BUG: at drivers/pci/pci.c:817 pcim_enable_device() Call Trace: [80337d21] pcim_enable_device+0x8a/0xa5 [88099d18] :libata:ata_pci_device_do_resume+0x20/0x59 [880bb731] :sata_nv:nv_pci_device_resume+0x1d/0x100 [8039d2bf] resume_device+0xcb/0x12c [8039d3ac] dpm_resume+0x8c/0xec [8039d456] device_resume+0x4a/0x5d [802a0a33] pm_suspend_disk+0x160/0x170 [8029f4b6] enter_state+0x52/0x1da [8029f69c] state_store+0x5e/0x79 [802f2b20] sysfs_write_file+0xe4/0x118 [80214b58] vfs_write+0xce/0x177 [8021553e] sys_write+0x45/0x6e [8025711e] system_call+0x7e/0x83 It looks like what's happening is that during the freezing stage, we suspend and then resume the controllers. ata_pci_device_do_suspend only calls pci_disable_device if the event is PM_EVENT_SUSPEND but ata_pci_device_do_resume calls pcim_enable_device unconditionally. If the event was something else, then pcim_enable_device complains because the device was previously enabled and never disabled. Not sure what the best way to fix this is? I think what should happen is either one of the followings. 1. Don't restore power state and re-enable PCI device on resume from freeze just as we don't do the opposite when freezing. 2. Unconditionally disable and power down PCI device on suspend whether it's freeze or not. #2 would be simpler but I'm a bit worried about it. There might be controllers which choke after such sequence (save state, disable, power down, no actual power removal, power on, restore state, re-enable). I'd just go for #2. #1 can be easily done by taking a look at the current device power state (gendev-power.power_state). The problem is that no one in suspend/resume path seems to be setting that variable except for runtime No, that variable is probably going to go away. If you want to remember that you are resuming from freeze, just store that info in private data structure. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH: ide: ide-disk freeze support for hdaps
Hi! Please make the interface accept number of seconds (as suggested by Jens) and remove this module parameter. This way interface will be more flexible and cleaner. I really don't see any advantage in doing echo 1 ... instead of echo x ... (Pavel, please explain). Either way is pretty easy enough to implement. Note though that I'd expect the userspace app should thaw the device when danger is out of the way (the timeout is mainly there to ensure that the queue isn't frozen forever, and should probably be higher). Personally I don't have too much of an opinion either way though... what's the consensus? :). Yes please, I don't understand why you would want a 0/1 interface instead, when the timer-seconds method gives you the exact same ability plus a way to control when to unfreeze... Well, with my power-managment hat on: we probably want freeze functionality to be generic; it makes sense for other devices, too. My battery is so low I can not use wifi any more = userspace freezes wifi. Now, having this kind of timeout in all the cases looks pretty ugly to my eyes. Pavel -- 64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html