Skip to contentSkip to conversation

hotplug polling, take 5

PATCH In review Tejun Heo 2 participants 8 messages Oct 15, 2006 – Nov 1, 2006 libata

lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

PATCH 1/1 hotplug polling, take 5

Hello, all.

This is the fifth take of hotplug polling patchset. This take doesn't contain any real change than rebasing over the current upstream[U]. PMP patchset will be posted soon on top of this patchset and I wanted to avoid confusion by posting patchsets in order.

As the name implies, this patchset implements hotplug by polling. hp-poll is used to

* Monitor ports EH gave up. When EH gives up on a port, it freezes the port to protect the rest of the system from it. The user used to have to issue manual scan to retry the port. hp-poll can monitor such port and retry it when hotplug event is detected. This is also used by PMP support.

* Support hotplug on controllers which can report hotplug conditions but cannot raise interrupt.

Changes from the last take[L] are.

* updated to the current upstream

This patchset is also available in the following git tree.

http://htj.dyndns.org/git/?p=libata-tj.git;a=shortlog;h=hp-poll
git://htj.dyndns.org/libata-tj hp-poll

Thanks.

--
tejun

[U] da54f5fe54c7d75e2db7d17961fb36a8c28a8501
[L] http://article.gmane.org/gmane.linux.ide/13418

author
#1 lore raw

PATCH 1/3 libata: implement hotplug by polling

This patch implements hotplug by polling - hp-poll. It is used for

* hotplug event detection on controllers which don't provide PHY status changed interrupt.

* hotplug event detection on disabled ports after reset failure. libata used to leave such ports in frozen state to protect the system from malfunctioning controller/device. With hp-poll, hotplug events no such ports are watched safely by polling, such that removing/replacing the malfunctioning device triggers EH retry on the port.

There are three port ops for hp-poll - hp_poll_activate, hp_poll, hp_poll_deactivate. Only hp_poll is mandatory for hp-poll to work. This patch also implements SATA standard polling callbacks which poll SError.N/X bits - sata_std_hp_poll_activate() and sata_std_hp_poll().

By default, hp-poll is enabled only on disabled ports. If a LLD doesn't support hotplug interrupts but can poll for hotplug events, it should indicate so by setting ATA_FLAG_HP_POLLING which tells libata to turn on hp-poll by default.

---
 drivers/ata/libata-core.c |   79 +++++++++++++++++++++++++++++
 drivers/ata/libata-eh.c   |  123 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/ata/libata.h      |    2 +
 include/linux/libata.h    |   10 ++++
 4 files changed, 211 insertions(+), 3 deletions(-)
drivers/ata/libata-core.c +78 -1
index c127d6f..006ab12 100644
@@ -2909,6 +2909,76 @@ void ata_std_postreset(struct ata_port *
 }
 
 /**
+ *	sata_std_hp_poll_activate - standard SATA hotplug polling activation
+ *	@ap: the target ata_port
+ *
+ *	Activate SATA std hotplug polling.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ */
+void sata_std_hp_poll_activate(struct ata_port *ap)
+{
+	u32 serror;
+
+	sata_scr_read(ap, SCR_ERROR, &serror);
+	serror &= SERR_PHYRDY_CHG | SERR_DEV_XCHG;
+	if (serror)
+		sata_scr_write(ap, SCR_ERROR, serror);
+
+	ap->hp_poll_data = 0;
+}
+
+/**
+ *	sata_std_hp_poll - standard SATA hotplug polling callback
+ *	@ap: the target ata_port
+ *
+ *	Poll SError.N/X for hotplug event.  Hotplug event is triggered
+ *	only after PHY stays stable for at least one full polling
+ *	interval after the first event detection.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host_set lock)
+ *
+ *	RETURNS:
+ *	1 if hotplug event has occurred, 0 otherwise.
+ */
+int sata_std_hp_poll(struct ata_port *ap)
+{
+	unsigned long state = (unsigned long)ap->hp_poll_data;
+	u32 serror;
+	int rc = 0;
+
+	sata_scr_read(ap, SCR_ERROR, &serror);
+	serror &= SERR_PHYRDY_CHG | SERR_DEV_XCHG;
+
+	switch (state) {
+	case 0:
+		if (serror) {
+			/* PHY status could be bouncing crazy due to
+			 * faulty controller or device.  Don't fire
+			 * hotplug event till hotplug event stays
+			 * quiescent for one full polling interval.
+			 */
+			sata_scr_write(ap, SCR_ERROR, serror);
+			state = 1;
+		}
+		break;
+
+	case 1:
+		if (!serror)
+			rc = 1;
+		else
+			sata_scr_write(ap, SCR_ERROR, serror);
+		break;
+	}
+
+	ap->hp_poll_data = (void *)state;
+
+	return rc;
+}
+
+/**
  *	ata_dev_same_device - Determine whether new ID matches configured device
  *	@dev: device to compare against
  *	@new_class: class of the new device
@@ -5437,6 +5507,7 @@ void ata_host_init(struct ata_host *host
 	host->dev = dev;
 	host->flags = flags;
 	host->ops = ops;
+	INIT_WORK(&host->hp_poll_task, ata_hp_poll_worker, host);
 }
 
 /**
@@ -5691,11 +5762,15 @@ void ata_port_detach(struct ata_port *ap
 
 	ata_port_wait_eh(ap);
 
-	/* Flush hotplug task.  The sequence is similar to
+	/* deactivate hotplug polling */
+	ata_hp_poll_deactivate(ap);
+
+	/* Flush hotplug tasks.  The sequence is similar to
 	 * ata_port_flush_task().
 	 */
 	flush_workqueue(ata_aux_wq);
 	cancel_delayed_work(&ap->hotplug_task);
+	cancel_delayed_work(&ap->host->hp_poll_task);
 	flush_workqueue(ata_aux_wq);
 
  skip_eh:
@@ -6129,6 +6204,8 @@ EXPORT_SYMBOL_GPL(ata_std_prereset);
 EXPORT_SYMBOL_GPL(ata_std_softreset);
 EXPORT_SYMBOL_GPL(sata_std_hardreset);
 EXPORT_SYMBOL_GPL(ata_std_postreset);
+EXPORT_SYMBOL_GPL(sata_std_hp_poll_activate);
+EXPORT_SYMBOL_GPL(sata_std_hp_poll);
 EXPORT_SYMBOL_GPL(ata_dev_revalidate);
 EXPORT_SYMBOL_GPL(ata_dev_classify);
 EXPORT_SYMBOL_GPL(ata_dev_pair);
drivers/ata/libata-eh.c +121 -2

172 lines not on this page · raw message

drivers/ata/libata.h +2 -0
index a5ecb71..017dc08 100644
@@ -118,5 +118,7 @@ extern enum scsi_eh_timer_return ata_scs
 extern void ata_scsi_error(struct Scsi_Host *host);
 extern void ata_port_wait_eh(struct ata_port *ap);
 extern void ata_qc_schedule_eh(struct ata_queued_cmd *qc);
+extern void ata_hp_poll_worker(void *data);
+extern void ata_hp_poll_deactivate(struct ata_port *ap);
 
 #endif /* __LIBATA_H__ */
include/linux/libata.h +10 -0
index d0a7ad5..4fc74c4 100644
@@ -175,6 +175,7 @@ enum {
 	ATA_FLAG_SKIP_D2H_BSY	= (1 << 12), /* can't wait for the first D2H
 					      * Register FIS clearing BSY */
 	ATA_FLAG_DEBUGMSG	= (1 << 13),
+	ATA_FLAG_HP_POLLING	= (1 << 14), /* hotplug by polling */
 
 	/* The following flag belongs to ap->pflags but is kept in
 	 * ap->flags because it's referenced in many LLDs and will be
@@ -192,6 +193,7 @@ enum {
 	ATA_PFLAG_LOADING	= (1 << 4), /* boot/loading probe */
 	ATA_PFLAG_UNLOADING	= (1 << 5), /* module is unloading */
 	ATA_PFLAG_SCSI_HOTPLUG	= (1 << 6), /* SCSI hotplug scheduled */
+	ATA_PFLAG_HP_POLL	= (1 << 7), /* hp-poll active */
 
 	ATA_PFLAG_FLUSH_PORT_TASK = (1 << 16), /* flush port task */
 	ATA_PFLAG_SUSPENDED	= (1 << 17), /* port is suspended (power) */
@@ -401,6 +403,7 @@ struct ata_host {
 	unsigned long		flags;
 	int			simplex_claimed;	/* Keep seperate in case we
 							   ever need to do this locked */
+	struct work_struct	hp_poll_task;
 	struct ata_port		*ports[0];
 };
 
@@ -581,6 +584,7 @@ struct ata_port {
 	pm_message_t		pm_mesg;
 	int			*pm_result;
 
+	void			*hp_poll_data;
 	void			*private_data;
 
 	u8			sector_buf[ATA_SECT_SIZE]; /* owned by EH */
@@ -628,6 +632,10 @@ struct ata_port_operations {
 	void (*error_handler) (struct ata_port *ap);
 	void (*post_internal_cmd) (struct ata_queued_cmd *qc);
 
+	void (*hp_poll_activate) (struct ata_port *ap);
+	void (*hp_poll_deactivate) (struct ata_port *ap);
+	int (*hp_poll) (struct ata_port *ap);
+
 	irq_handler_t irq_handler;
 	void (*irq_clear) (struct ata_port *);
 
@@ -702,6 +710,8 @@ extern int ata_std_prereset(struct ata_p
 extern int ata_std_softreset(struct ata_port *ap, unsigned int *classes);
 extern int sata_std_hardreset(struct ata_port *ap, unsigned int *class);
 extern void ata_std_postreset(struct ata_port *ap, unsigned int *classes);
+extern void sata_std_hp_poll_activate(struct ata_port *ap);
+extern int sata_std_hp_poll(struct ata_port *ap);
 extern int ata_dev_revalidate(struct ata_device *dev, int post_reset);
 extern void ata_port_disable(struct ata_port *);
 extern void ata_std_ports(struct ata_ioports *ioaddr);
Signature · 1 line
1.4.2.3
author
#2 lore raw

PATCH 2/3 libata: add hp-poll support to controllers with hotplug interrutps

In reply to Tejun Heo

This patch adds hp-poll support to the following drivers.

* ahci
* sata_nv (nf2 and c804 only)
* sata_sil
* sata_sil24

All the above controllers are capable of hotplug-by-interrupt and hp-poll will be used only for watching disabled ports.

---
 drivers/ata/ahci.c       |    4 +++-
 drivers/ata/sata_nv.c    |    4 ++++
 drivers/ata/sata_sil.c   |    2 ++
 drivers/ata/sata_sil24.c |    3 +++
 4 files changed, 12 insertions(+), 1 deletions(-)
drivers/ata/ahci.c +3 -1
index 2592912..d95d99b 100644
@@ -262,10 +262,12 @@ static const struct ata_port_operations 
 
 	.freeze			= ahci_freeze,
 	.thaw			= ahci_thaw,
-
 	.error_handler		= ahci_error_handler,
 	.post_internal_cmd	= ahci_post_internal_cmd,
 
+	.hp_poll_activate	= sata_std_hp_poll_activate,
+	.hp_poll		= sata_std_hp_poll,
+
 	.port_suspend		= ahci_port_suspend,
 	.port_resume		= ahci_port_resume,
 
drivers/ata/sata_nv.c +4 -0
index 323b607..a75d5cc 100644
@@ -200,6 +200,8 @@ static const struct ata_port_operations 
 	.thaw			= nv_nf2_thaw,
 	.error_handler		= nv_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
+	.hp_poll_activate	= sata_std_hp_poll_activate,
+	.hp_poll		= sata_std_hp_poll,
 	.data_xfer		= ata_pio_data_xfer,
 	.irq_handler		= nv_nf2_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
@@ -227,6 +229,8 @@ static const struct ata_port_operations 
 	.thaw			= nv_ck804_thaw,
 	.error_handler		= nv_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
+	.hp_poll_activate	= sata_std_hp_poll_activate,
+	.hp_poll		= sata_std_hp_poll,
 	.data_xfer		= ata_pio_data_xfer,
 	.irq_handler		= nv_ck804_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
drivers/ata/sata_sil.c +2 -0
index ca8d993..5c8a349 100644
@@ -205,6 +205,8 @@ static const struct ata_port_operations 
 	.thaw			= sil_thaw,
 	.error_handler		= ata_bmdma_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
+	.hp_poll_activate	= sata_std_hp_poll_activate,
+	.hp_poll		= sata_std_hp_poll,
 	.irq_handler		= sil_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= sil_scr_read,
drivers/ata/sata_sil24.c +3 -0
index 169e200..c84efc3 100644
@@ -410,6 +410,9 @@ static const struct ata_port_operations 
 	.error_handler		= sil24_error_handler,
 	.post_internal_cmd	= sil24_post_internal_cmd,
 
+	.hp_poll_activate	= sata_std_hp_poll_activate,
+	.hp_poll		= sata_std_hp_poll,
+
 	.port_start		= sil24_port_start,
 	.port_stop		= sil24_port_stop,
 	.host_stop		= sil24_host_stop,
Signature · 1 line
1.4.2.3
author
#3 lore raw

PATCH 3/3 libata: add hp-poll support to controllers without hotplug interrupts

In reply to Tejun Heo

This patch adds hp-poll support to the following drivers.

* sata_nv (generic)
* sata_sil (SIL_FLAG_NO_SATA_IRQ replaced w/ ATA_FLAG_HP_POLLING)
* sata_sis
* sata_svw
* sata_uli
* sata_vsc

All the above drivers currently don't support hotplug interrupts (H/W restrictions or lack of doc/effort) and all hotplug operations should be done via hp-poll; thus, ATA_FLAG_HP_POLLING is set for these drivers.

sata_via is excluded because it randomly locks up on SCR register access.

---
 drivers/ata/sata_nv.c  |    5 ++++-
 drivers/ata/sata_sil.c |    9 ++++-----
 drivers/ata/sata_sis.c |    5 ++++-
 drivers/ata/sata_svw.c |    4 +++-
 drivers/ata/sata_uli.c |    6 +++++-
 drivers/ata/sata_vsc.c |    4 +++-
 6 files changed, 23 insertions(+), 10 deletions(-)
drivers/ata/sata_nv.c +4 -1
index a75d5cc..fcdae1a 100644
@@ -173,6 +173,8 @@ static const struct ata_port_operations 
 	.thaw			= ata_bmdma_thaw,
 	.error_handler		= nv_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
+	.hp_poll_activate	= sata_std_hp_poll_activate,
+	.hp_poll		= sata_std_hp_poll,
 	.data_xfer		= ata_pio_data_xfer,
 	.irq_handler		= nv_generic_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
@@ -245,7 +247,8 @@ static struct ata_port_info nv_port_info
 	/* generic */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HP_POLLING,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,
drivers/ata/sata_sil.c +4 -5
index 5c8a349..1b2e36a 100644
@@ -52,7 +52,6 @@ enum {
 	/*
 	 * host flags
 	 */
-	SIL_FLAG_NO_SATA_IRQ	= (1 << 28),
 	SIL_FLAG_RERR_ON_DMA_ACT = (1 << 29),
 	SIL_FLAG_MOD15WRITE	= (1 << 30),
 
@@ -230,7 +229,7 @@ static const struct ata_port_info sil_po
 	{
 		.sht		= &sil_sht,
 		.flags		= SIL_DFL_PORT_FLAGS | SIL_FLAG_MOD15WRITE |
-				  SIL_FLAG_NO_SATA_IRQ,
+				  ATA_FLAG_HP_POLLING,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -454,8 +453,8 @@ static irqreturn_t sil_interrupt(int irq
 		if (unlikely(!ap || ap->flags & ATA_FLAG_DISABLED))
 			continue;
 
-		/* turn off SATA_IRQ if not supported */
-		if (ap->flags & SIL_FLAG_NO_SATA_IRQ)
+		/* ignore SATA_IRQ if not supported */
+		if (ap->flags & ATA_FLAG_HP_POLLING)
 			bmdma2 &= ~SIL_DMA_SATA_IRQ;
 
 		if (bmdma2 == 0xffffffff ||
@@ -496,7 +495,7 @@ static void sil_thaw(struct ata_port *ap
 	ata_bmdma_irq_clear(ap);
 
 	/* turn on SATA IRQ if supported */
-	if (!(ap->flags & SIL_FLAG_NO_SATA_IRQ))
+	if (!(ap->flags & ATA_FLAG_HP_POLLING))
 		writel(SIL_SIEN_N, mmio_base + sil_port[ap->port_no].sien);
 
 	/* turn on IRQ */
drivers/ata/sata_sis.c +4 -1
index 0738f52..f31ff5f 100644
@@ -117,6 +117,8 @@ static const struct ata_port_operations 
 	.thaw			= ata_bmdma_thaw,
 	.error_handler		= ata_bmdma_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
+	.hp_poll_activate	= sata_std_hp_poll_activate,
+	.hp_poll		= sata_std_hp_poll,
 	.irq_handler		= ata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= sis_scr_read,
@@ -128,7 +130,8 @@ static const struct ata_port_operations 
 
 static struct ata_port_info sis_port_info = {
 	.sht		= &sis_sht,
-	.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+	.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+			  ATA_FLAG_HP_POLLING,
 	.pio_mask	= 0x1f,
 	.mwdma_mask	= 0x7,
 	.udma_mask	= 0x7f,
drivers/ata/sata_svw.c +3 -1
index db32d15..301e42c 100644
@@ -324,6 +324,8 @@ static const struct ata_port_operations 
 	.thaw			= ata_bmdma_thaw,
 	.error_handler		= ata_bmdma_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
+	.hp_poll_activate	= sata_std_hp_poll_activate,
+	.hp_poll		= sata_std_hp_poll,
 	.irq_handler		= ata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= k2_sata_scr_read,
@@ -424,7 +426,7 @@ static int k2_sata_init_one (struct pci_
 
 	probe_ent->sht = &k2_sata_sht;
 	probe_ent->port_flags = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				ATA_FLAG_MMIO;
+				ATA_FLAG_MMIO | ATA_FLAG_HP_POLLING;
 	probe_ent->port_ops = &k2_sata_ops;
 	probe_ent->n_ports = 4;
 	probe_ent->irq = pdev->irq;
drivers/ata/sata_uli.c +5 -1
index 5c603ca..6e9a8dd 100644
@@ -115,6 +115,9 @@ static const struct ata_port_operations 
 	.error_handler		= ata_bmdma_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
+	.hp_poll_activate	= sata_std_hp_poll_activate,
+	.hp_poll		= sata_std_hp_poll,
+
 	.irq_handler		= ata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 
@@ -128,7 +131,8 @@ static const struct ata_port_operations 
 
 static struct ata_port_info uli_port_info = {
 	.sht            = &uli_sht,
-	.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+	.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+			  ATA_FLAG_HP_POLLING,
 	.pio_mask       = 0x1f,		/* pio0-4 */
 	.udma_mask      = 0x7f,		/* udma0-6 */
 	.port_ops       = &uli_ops,
drivers/ata/sata_vsc.c +3 -1
index e654b99..c92fb0d 100644
@@ -301,6 +301,8 @@ static const struct ata_port_operations 
 	.thaw			= ata_bmdma_thaw,
 	.error_handler		= ata_bmdma_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
+	.hp_poll_activate	= sata_std_hp_poll_activate,
+	.hp_poll		= sata_std_hp_poll,
 	.irq_handler		= vsc_sata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= vsc_sata_scr_read,
@@ -395,7 +397,7 @@ static int __devinit vsc_sata_init_one (
 
 	probe_ent->sht = &vsc_sata_sht;
 	probe_ent->port_flags = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				ATA_FLAG_MMIO;
+				ATA_FLAG_MMIO | ATA_FLAG_HP_POLLING;
 	probe_ent->port_ops = &vsc_sata_ops;
 	probe_ent->n_ports = 4;
 	probe_ent->irq = pdev->irq;
Signature · 1 line
1.4.2.3
#4 lore raw
In reply to Tejun Heo
Quoted 10 lines · Tejun Heo

Hello, all.

This is the fifth take of hotplug polling patchset. This take doesn't contain any real change than rebasing over the current upstream[U]. PMP patchset will be posted soon on top of this patchset and I wanted to avoid confusion by posting patchsets in order.

As the name implies, this patchset implements hotplug by polling. hp-poll is used to

Quoted 8 lines

* Monitor ports EH gave up. When EH gives up on a port, it freezes the port to protect the rest of the system from it. The user used to have to issue manual scan to retry the port. hp-poll can monitor such port and retry it when hotplug event is detected. This is also used by PMP support.

* Support hotplug on controllers which can report hotplug conditions but cannot raise interrupt.

Patchset seems sane. I'll need to re-read patch #1 in depth, but I give everything a tentative ACK for now.

My biggest concern is power usage. On laptops for example, the 99% common case is that the user will never hot[un]plug a drive, so we shouldn't waste power bothering with poking disabled ports.

Jeff

author
#5 lore raw
Quoted 17 lines · Jeff Garzik
Quoted 16 lines · Tejun Heo

Hello, all.

This is the fifth take of hotplug polling patchset. This take doesn't contain any real change than rebasing over the current upstream[U]. PMP patchset will be posted soon on top of this patchset and I wanted to avoid confusion by posting patchsets in order.

As the name implies, this patchset implements hotplug by polling. hp-poll is used to

* Monitor ports EH gave up. When EH gives up on a port, it freezes the port to protect the rest of the system from it. The user used to have to issue manual scan to retry the port. hp-poll can monitor such port and retry it when hotplug event is detected. This is also used by PMP support.

Quoted 6 lines
Quoted 2 lines

* Support hotplug on controllers which can report hotplug conditions but cannot raise interrupt.

Patchset seems sane. I'll need to re-read patch #1 in depth, but I give everything a tentative ACK for now.

My biggest concern is power usage. On laptops for example, the 99% common case is that the user will never hot[un]plug a drive, so we shouldn't waste power bothering with poking disabled ports.

Link powersave patchset should handle that together but I'm still not sure what to do w/ user interface (which sysfs node to use). Also, controllers which are used on laptops && use hp-poll by default are sata_nv (old ones) and some variant of sata_sil used in ati chipset. So, it shouldn't cause trouble for most laptop users.

Signature · 1 line
tejun
#6 lore raw
Quoted 18 lines · Tejun Heo
Quoted 17 lines · Jeff Garzik
Quoted 16 lines · Tejun Heo

Hello, all.

This is the fifth take of hotplug polling patchset. This take doesn't contain any real change than rebasing over the current upstream[U]. PMP patchset will be posted soon on top of this patchset and I wanted to avoid confusion by posting patchsets in order.

As the name implies, this patchset implements hotplug by polling. hp-poll is used to

* Monitor ports EH gave up. When EH gives up on a port, it freezes the port to protect the rest of the system from it. The user used to have to issue manual scan to retry the port. hp-poll can monitor such port and retry it when hotplug event is detected. This is also used by PMP support.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

* Support hotplug on controllers which can report hotplug conditions but cannot raise interrupt.

Patchset seems sane. I'll need to re-read patch #1 in depth, but I give everything a tentative ACK for now.

My biggest concern is power usage. On laptops for example, the 99% common case is that the user will never hot[un]plug a drive, so we shouldn't waste power bothering with poking disabled ports.

Link powersave patchset should handle that together but I'm still not sure what to do w/ user interface (which sysfs node to use). Also, controllers which are used on laptops && use hp-poll by default are sata_nv (old ones) and some variant of sata_sil used in ati chipset. So, it shouldn't cause trouble for most laptop users.

I'm not saying it causes trouble. Just saying that polling unused ports is practically guaranteed to require more power than never doing so.

It shouldn't be hard to figure out where to poke stuff into sysfs...

Jeff

author
#7 lore raw
Quoted 3 lines · Jeff Garzik
Quoted 1 line

Link powersave patchset should handle that together but I'm still not sure what to do w/ user interface (which sysfs node to use). Also, controllers which are used on laptops && use hp-poll by default are sata_nv (old ones) and some variant of sata_sil used in ati chipset. So, it shouldn't cause trouble for most laptop users.

I'm not saying it causes trouble. Just saying that polling unused ports is practically guaranteed to require more power than never doing so.

Yeap, no objection there.

Quoted 1 line

It shouldn't be hard to figure out where to poke stuff into sysfs...

I dunno. It's policy decision and we don't have ATA sysfs nodes yet. So, where do we begin? Is it time to start /sys/class/ata_host/ataX tree?

Thanks.

Signature · 1 line
tejun