linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 06/13] libata-hp: prepare for persistent device flags
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (3 preceding siblings ...)
  2006-04-11 14:06 ` [PATCH 07/13] libata-hp: implement ap->orig_sata_spd_limit Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-11 14:06 ` [PATCH 05/13] libata-hp: call ata_dev_init() from ata_bus_probe() Tejun Heo
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

This patch makes dev->flags persistent, defines ATA_DFLAG_INIT_MASK as
lower 16 bits of device flags and clear them in ata_dev_init().  So,
device flags in bits [0:15] are cleared over hotplugging while flags
from bit 16 upward are maintained.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |    2 ++
 include/linux/libata.h     |    3 ++-
 2 files changed, 4 insertions(+), 1 deletions(-)

3a7b8630e4c5cc409562164a941b40bc2b6b8728
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 09e82f9..930f387 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1444,6 +1444,8 @@ err_out_nosup:
  */
 static void ata_dev_init(struct ata_port *ap, struct ata_device *dev)
 {
+	dev->flags &= ~ATA_DFLAG_INIT_MASK;
+
 	memset((void *)dev + ATA_DEVICE_CLEAR_OFFSET, 0,
 	       sizeof(*dev) - ATA_DEVICE_CLEAR_OFFSET);
 	dev->pio_mask = UINT_MAX;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 809f3b9..33ce8a4 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -128,6 +128,7 @@ enum {
 
 	ATA_DFLAG_PIO		= (1 << 8), /* device currently in PIO mode */
 	ATA_DFLAG_FAILED	= (1 << 9), /* device has failed */
+	ATA_DFLAG_INIT_MASK	= (1 << 16) - 1,
 
 	ATA_DEV_UNKNOWN		= 0,	/* unknown device */
 	ATA_DEV_ATA		= 1,	/* ATA device */
@@ -396,9 +397,9 @@ struct ata_ering {
 
 struct ata_device {
 	unsigned int		devno;		/* 0 or 1 */
+	unsigned long		flags;		/* ATA_DFLAG_xxx */
 	/* fields above n_sectors are not cleared across device init */
 	u64			n_sectors;	/* size of device, if ATA */
-	unsigned long		flags;		/* ATA_DFLAG_xxx */
 	unsigned int		class;		/* ATA_DEV_xxx */
 	u16			*id;		/* IDENTIFY xxx DEVICE data */
 	u8			pio_mode;
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 04/13] libata-hp: make some fields of ata_device persistent
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
  2006-04-11 14:06 ` [PATCH 08/13] libata-hp: move device enable/disable out of ata_bus_probe() Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-11 14:06 ` [PATCH 01/13] libata-hp: separate out __ata_scsi_find_dev() Tejun Heo
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

Lifetimes of some fields span over device plugging/unplugging.  This
patch moves such persistent fields to the top of ata_device and
separate them with ATA_DEVICE_CLEAR_OFFSET.  Fields above the offset
are initialized once during host initializatino while all other fields
are cleared before hotplugging.  Currently only dev->devno is
persistent.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |    2 +-
 include/linux/libata.h     |    5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

9160b11ebb15ef4f9501a1aba807ed221ef713ed
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 1378b6f..3406f3d 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1446,7 +1446,6 @@ static void ata_dev_init(struct ata_port
 {
 	memset((void *)dev + ATA_DEVICE_CLEAR_OFFSET, 0,
 	       sizeof(*dev) - ATA_DEVICE_CLEAR_OFFSET);
-	dev->devno = dev - ap->device;
 	dev->pio_mask = UINT_MAX;
 	dev->mwdma_mask = UINT_MAX;
 	dev->udma_mask = UINT_MAX;
@@ -4885,6 +4884,7 @@ static void ata_host_init(struct ata_por
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		struct ata_device *dev = &ap->device[i];
+		dev->devno = i;
 		ata_dev_init(ap, dev);
 	}
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e86d63c..809f3b9 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -395,10 +395,11 @@ struct ata_ering {
 	struct ata_ering_entry	name_entries[size];
 
 struct ata_device {
+	unsigned int		devno;		/* 0 or 1 */
+	/* fields above n_sectors are not cleared across device init */
 	u64			n_sectors;	/* size of device, if ATA */
 	unsigned long		flags;		/* ATA_DFLAG_xxx */
 	unsigned int		class;		/* ATA_DEV_xxx */
-	unsigned int		devno;		/* 0 or 1 */
 	u16			*id;		/* IDENTIFY xxx DEVICE data */
 	u8			pio_mode;
 	u8			dma_mode;
@@ -423,6 +424,8 @@ struct ata_device {
 	DEFINE_ATA_ERING	(ering, ATA_DEV_ERING_SIZE);
 };
 
+#define ATA_DEVICE_CLEAR_OFFSET		offsetof(struct ata_device, n_sectors)
+
 struct ata_port {
 	struct Scsi_Host	*host;	/* our co-allocated scsi host */
 	const struct ata_port_operations *ops;
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 08/13] libata-hp: move device enable/disable out of ata_bus_probe()
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-11 14:06 ` [PATCH 04/13] libata-hp: make some fields of ata_device persistent Tejun Heo
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

For drivers support hot/warm plugging, the controller should stay
enabled whether devices are attached to it or not.  Move
ata_port_probe() and disable calls out of ata_bus_probe() and disable
only if the driver doesn't implement new style ->error_handler, which
is responsible for hotplug.

While at it, kill the FIXME about controllers with no device attached.
Hotplug is coming and all drivers should support it in not so distant
future.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   19 ++++++-------------
 1 files changed, 6 insertions(+), 13 deletions(-)

84a5e767247a9a0047e4b485fbd9de7582d9b283
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 45ef1e2..e68a64d 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1478,8 +1478,6 @@ static int ata_bus_probe(struct ata_port
 	int i, rc, down_xfermask;
 	struct ata_device *dev;
 
-	ata_port_probe(ap);
-
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		tries[i] = ATA_PROBE_MAX_TRIES;
 		ata_dev_init(ap, &ap->device[i]);
@@ -1556,10 +1554,6 @@ static int ata_bus_probe(struct ata_port
 	for (i = 0; i < ATA_MAX_DEVICES; i++)
 		if (ata_dev_enabled(&ap->device[i]))
 			return 0;
-
-	/* no device present, disable port */
-	ata_port_disable(ap);
-	ap->ops->port_disable(ap);
 	return -ENODEV;
 
  fail:
@@ -5033,6 +5027,8 @@ int ata_device_add(const struct ata_prob
 
 		ap = host_set->ports[i];
 
+		ata_port_probe(ap);
+
 		/* init ap->orig_sata_spd_limit to boot value */
 		if ((ap->flags & ATA_FLAG_SATA) && ap->ops->scr_read) {
 			u32 spd;
@@ -5045,13 +5041,10 @@ int ata_device_add(const struct ata_prob
 		rc = ata_bus_probe(ap);
 		DPRINTK("ata%u: bus probe end\n", ap->id);
 
-		if (rc) {
-			/* FIXME: do something useful here?
-			 * Current libata behavior will
-			 * tear down everything when
-			 * the module is removed
-			 * or the h/w is unplugged.
-			 */
+		if (rc && !ap->ops->error_handler) {
+			/* no device attached and no hotplug support */
+			ata_port_disable(ap);
+			ap->ops->port_disable(ap);
 		}
 
 		rc = scsi_add_host(ap->host, dev);
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 02/13] libata-hp: add more SERR_* constants in preparation for hotplug support
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (6 preceding siblings ...)
  2006-04-11 14:06 ` [PATCH 03/13] libata-hp: implement ata_dev_init() Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-11 14:06 ` [PATCH 10/13] libata-hp: make ata_bus_probe() extern Tejun Heo
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

Add PHY status related SERR_* constants in preparation for hotplug
support.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 include/linux/ata.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

55f539ff11bacd214ae47e1f0e7ea8d57869dc76
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 0509340..9c18526 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -208,6 +208,8 @@ enum {
 	SERR_PERSISTENT		= (1 << 9), /* persistent data/comm error */
 	SERR_PROTOCOL		= (1 << 10), /* protocol violation */
 	SERR_INTERNAL		= (1 << 11), /* host internal error */
+	SERR_PHYRDY_CHG		= (1 << 16), /* PHY RDY changed */
+	SERR_DEV_XCHG		= (1 << 26), /* device exchanged */
 
 	/* struct ata_taskfile flags */
 	ATA_TFLAG_LBA48		= (1 << 0), /* enable 48-bit LBA and "HOB" */
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 01/13] libata-hp: separate out __ata_scsi_find_dev()
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
  2006-04-11 14:06 ` [PATCH 08/13] libata-hp: move device enable/disable out of ata_bus_probe() Tejun Heo
  2006-04-11 14:06 ` [PATCH 04/13] libata-hp: make some fields of ata_device persistent Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-11 14:06 ` [PATCH 07/13] libata-hp: implement ap->orig_sata_spd_limit Tejun Heo
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

Separate out __ata_scsi_find_dev() from ata_scsi_find_dev().  The
underscored version doesn't check whether the ATA device is enabled
and useuable.  It unconditionally returns the associated ATA device if
channel/id/lun is valid.  This function will be used by hotplug.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-scsi.c |   31 ++++++++++++++++---------------
 1 files changed, 16 insertions(+), 15 deletions(-)

ec6e2517ec4dd32773cfed7e8d8ad3e559509d51
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index 5f81136..4ce390a 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -52,8 +52,9 @@
 #define SECTOR_SIZE	512
 
 typedef unsigned int (*ata_xlat_func_t)(struct ata_queued_cmd *qc, const u8 *scsicmd);
-static struct ata_device *
-ata_scsi_find_dev(struct ata_port *ap, const struct scsi_device *scsidev);
+
+static struct ata_device *__ata_scsi_find_dev(struct ata_port *ap,
+					      const struct scsi_device *scsidev);
 
 #define RW_RECOVERY_MPAGE 0x1
 #define RW_RECOVERY_MPAGE_LEN 12
@@ -2317,6 +2318,17 @@ static unsigned int atapi_xlat(struct at
 	return 0;
 }
 
+static struct ata_device *
+__ata_scsi_find_dev(struct ata_port *ap, const struct scsi_device *scsidev)
+{
+	/* skip commands not addressed to targets we simulate */
+	if (likely(scsidev->id < ATA_MAX_DEVICES &&
+		   scsidev->channel == 0 && scsidev->lun == 0))
+		return &ap->device[scsidev->id];
+
+	return NULL;
+}
+
 /**
  *	ata_scsi_find_dev - lookup ata_device from scsi_cmnd
  *	@ap: ATA port to which the device is attached
@@ -2333,23 +2345,12 @@ static unsigned int atapi_xlat(struct at
  *	RETURNS:
  *	Associated ATA device, or %NULL if not found.
  */
-
 static struct ata_device *
 ata_scsi_find_dev(struct ata_port *ap, const struct scsi_device *scsidev)
 {
-	struct ata_device *dev;
-
-	/* skip commands not addressed to targets we simulate */
-	if (likely(scsidev->id < ATA_MAX_DEVICES))
-		dev = &ap->device[scsidev->id];
-	else
-		return NULL;
-
-	if (unlikely((scsidev->channel != 0) ||
-		     (scsidev->lun != 0)))
-		return NULL;
+	struct ata_device *dev = __ata_scsi_find_dev(ap, scsidev);
 
-	if (unlikely(!ata_dev_enabled(dev)))
+	if (unlikely(!dev || !ata_dev_enabled(dev)))
 		return NULL;
 
 	if (!atapi_enabled || (ap->flags & ATA_FLAG_NO_ATAPI)) {
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 03/13] libata-hp: implement ata_dev_init()
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (5 preceding siblings ...)
  2006-04-11 14:06 ` [PATCH 05/13] libata-hp: call ata_dev_init() from ata_bus_probe() Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-11 14:06 ` [PATCH 02/13] libata-hp: add more SERR_* constants in preparation for hotplug support Tejun Heo
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

Move initialization of struct ata_device into ata_dev_init() in
preparation for hotplug.  This patch calls ata_dev_init() from
ata_host_init() and thus makes no functional difference.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)

442eed97d84e43eb0860e19398c508c4a02c2860
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index d49d4c7..1378b6f 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1433,6 +1433,27 @@ err_out_nosup:
 }
 
 /**
+ *	ata_dev_init - Initialize an ata_device structure
+ *	@ap: ATA port device to initialize is attached to
+ *	@dev: Device structure to initialize
+ *
+ *	Initialize @dev in preparation for probing.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
+static void ata_dev_init(struct ata_port *ap, struct ata_device *dev)
+{
+	memset((void *)dev + ATA_DEVICE_CLEAR_OFFSET, 0,
+	       sizeof(*dev) - ATA_DEVICE_CLEAR_OFFSET);
+	dev->devno = dev - ap->device;
+	dev->pio_mask = UINT_MAX;
+	dev->mwdma_mask = UINT_MAX;
+	dev->udma_mask = UINT_MAX;
+	ata_ering_init(&dev->ering, ATA_DEV_ERING_SIZE);
+}
+
+/**
  *	ata_bus_probe - Reset and probe ATA bus
  *	@ap: Bus to probe
  *
@@ -4828,7 +4849,6 @@ static void ata_host_remove(struct ata_p
  *	LOCKING:
  *	Inherited from caller.
  */
-
 static void ata_host_init(struct ata_port *ap, struct Scsi_Host *host,
 			  struct ata_host_set *host_set,
 			  const struct ata_probe_ent *ent, unsigned int port_no)
@@ -4865,11 +4885,7 @@ static void ata_host_init(struct ata_por
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		struct ata_device *dev = &ap->device[i];
-		dev->devno = i;
-		dev->pio_mask = UINT_MAX;
-		dev->mwdma_mask = UINT_MAX;
-		dev->udma_mask = UINT_MAX;
-		ata_ering_init(&dev->ering, ATA_DEV_ERING_SIZE);
+		ata_dev_init(ap, dev);
 	}
 
 #ifdef ATA_IRQ_TRAP
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCHSET 8/9] prep for hotplug support
@ 2006-04-11 14:06 Tejun Heo
  2006-04-11 14:06 ` [PATCH 08/13] libata-hp: move device enable/disable out of ata_bus_probe() Tejun Heo
                   ` (14 more replies)
  0 siblings, 15 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide, htejun

Hello, all.

This is the first take of prep-for-hotplug-support patchset.  As the
name suggests, this patchset prepares libata for hotplug support.
This patchset contains 15 patches.

#01-02	update sata_sil (hotplug related constants, new interrupt handler)
#03-04	misc libata preps
#05-12	prepare ata_bus_probe()
#13-15	hotplug flags, extra fields, hotplug_wq

This patchset is against

  upstream (c2a6585296009379e0f4eff39cdcb108b457ebf2)
  + [1] misc-reset-updates patchset (repost)
  + [2] implement-and-use-ata_wait_register patchset (repost)
  + [3] misc-ata_bus_probe-updates patchset
  + [4] fixes-errata-workaround-and-reset-updates patchset, take 2
  + [5] implement-scsi_eh_schedule patchset
  + [6] fix-scsi_kill_request-busy-count-handling patch
  + [7] new-EH-framework patchset, take 2
  + [8] new-EH-implementation patchset, take 2
  + [9] add-NCQ-support, take 2

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.ide/9495
[2] http://article.gmane.org/gmane.linux.ide/9499
[3] http://article.gmane.org/gmane.linux.ide/9506
[4] http://article.gmane.org/gmane.linux.ide/9516
[5] http://article.gmane.org/gmane.linux.ide/9290
[6] http://article.gmane.org/gmane.linux.ide/9487
[7] http://article.gmane.org/gmane.linux.ide/9524
[8] http://article.gmane.org/gmane.linux.ide/9540
[9] http://article.gmane.org/gmane.linux.ide/9555



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 07/13] libata-hp: implement ap->orig_sata_spd_limit
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (2 preceding siblings ...)
  2006-04-11 14:06 ` [PATCH 01/13] libata-hp: separate out __ata_scsi_find_dev() Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-27  9:19   ` Jeff Garzik
  2006-04-11 14:06 ` [PATCH 06/13] libata-hp: prepare for persistent device flags Tejun Heo
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

Add ap->orig_sata_spd_limit and initialize it once during the boot
initialization (or driver load initialization).  ap->sata_spd_limit is
reset to ap->orig_sata_spd_limit on hotplug.  This prevents limits
introduced by earlier devices from affecting new devices.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   20 ++++++++++++--------
 include/linux/libata.h     |    1 +
 2 files changed, 13 insertions(+), 8 deletions(-)

37d8c01b360760e0c22e5cede70921c116c21ad4
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 930f387..45ef1e2 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1444,6 +1444,9 @@ err_out_nosup:
  */
 static void ata_dev_init(struct ata_port *ap, struct ata_device *dev)
 {
+	/* for now, SATA spd limit is bound to the first device */
+	ap->sata_spd_limit = ap->orig_sata_spd_limit;
+
 	dev->flags &= ~ATA_DFLAG_INIT_MASK;
 
 	memset((void *)dev + ATA_DEVICE_CLEAR_OFFSET, 0,
@@ -2503,17 +2506,10 @@ static int sata_phy_resume(struct ata_po
 void ata_std_probeinit(struct ata_port *ap)
 {
 	if ((ap->flags & ATA_FLAG_SATA) && ap->ops->scr_read) {
-		u32 spd;
-
 		/* set cable type and resume link */
 		ap->cbl = ATA_CBL_SATA;
 		sata_phy_resume(ap);
 
-		/* init sata_spd_limit to the current value */
-		spd = (scr_read(ap, SCR_CONTROL) & 0xf0) >> 4;
-		if (spd)
-			ap->sata_spd_limit &= (1 << spd) - 1;
-
 		/* wait for device */
 		if (sata_dev_present(ap))
 			ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT);
@@ -4878,7 +4874,7 @@ static void ata_host_init(struct ata_por
 	ap->flags |= ent->host_flags;
 	ap->ops = ent->port_ops;
 	ap->cbl = ATA_CBL_NONE;
-	ap->sata_spd_limit = UINT_MAX;
+	ap->orig_sata_spd_limit = UINT_MAX;
 	ap->active_tag = ATA_TAG_POISON;
 	ap->last_ctl = 0xFF;
 
@@ -5037,6 +5033,14 @@ int ata_device_add(const struct ata_prob
 
 		ap = host_set->ports[i];
 
+		/* init ap->orig_sata_spd_limit to boot value */
+		if ((ap->flags & ATA_FLAG_SATA) && ap->ops->scr_read) {
+			u32 spd;
+			spd = (scr_read(ap, SCR_CONTROL) & 0xf0) >> 4;
+			if (spd)
+				ap->orig_sata_spd_limit &= (1 << spd) - 1;
+		}
+
 		DPRINTK("ata%u: bus probe begin\n", ap->id);
 		rc = ata_bus_probe(ap);
 		DPRINTK("ata%u: bus probe end\n", ap->id);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 33ce8a4..94f1e9a 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -449,6 +449,7 @@ struct ata_port {
 	unsigned int		mwdma_mask;
 	unsigned int		udma_mask;
 	unsigned int		cbl;	/* cable type; ATA_CBL_xxx */
+	unsigned int		orig_sata_spd_limit; /* boot value */
 	unsigned int		sata_spd_limit;	/* SATA PHY speed limit */
 
 	struct ata_device	device[ATA_MAX_DEVICES];
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 05/13] libata-hp: call ata_dev_init() from ata_bus_probe()
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (4 preceding siblings ...)
  2006-04-11 14:06 ` [PATCH 06/13] libata-hp: prepare for persistent device flags Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-11 14:06 ` [PATCH 03/13] libata-hp: implement ata_dev_init() Tejun Heo
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

Call ata_dev_init() before actual probing instead of during host
initialization.  This is in preparation of hotplug.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

bc205de9459b473c8fef86b3f4e6e356aa9186fd
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 3406f3d..09e82f9 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1466,7 +1466,6 @@ static void ata_dev_init(struct ata_port
  *	RETURNS:
  *	Zero on success, negative errno otherwise.
  */
-
 static int ata_bus_probe(struct ata_port *ap)
 {
 	unsigned int classes[ATA_MAX_DEVICES];
@@ -1476,8 +1475,10 @@ static int ata_bus_probe(struct ata_port
 
 	ata_port_probe(ap);
 
-	for (i = 0; i < ATA_MAX_DEVICES; i++)
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		tries[i] = ATA_PROBE_MAX_TRIES;
+		ata_dev_init(ap, &ap->device[i]);
+	}
 
  retry:
 	down_xfermask = 0;
@@ -4882,11 +4883,8 @@ static void ata_host_init(struct ata_por
 	INIT_WORK(&ap->port_task, NULL, NULL);
 	INIT_LIST_HEAD(&ap->eh_done_q);
 
-	for (i = 0; i < ATA_MAX_DEVICES; i++) {
-		struct ata_device *dev = &ap->device[i];
-		dev->devno = i;
-		ata_dev_init(ap, dev);
-	}
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		ap->device[i].devno = i;
 
 #ifdef ATA_IRQ_TRAP
 	ap->stats.unhandled_irq = 1;
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 10/13] libata-hp: make ata_bus_probe() extern
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (7 preceding siblings ...)
  2006-04-11 14:06 ` [PATCH 02/13] libata-hp: add more SERR_* constants in preparation for hotplug support Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-11 14:06 ` [PATCH 11/13] libata-hp: add hotplug flags and flag handling functions Tejun Heo
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

Make ata_bus_probe() extern and put its prototype in libata private
header file.  It will be called from libata-eh.c.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |    2 +-
 drivers/scsi/libata.h      |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

b0d71bd83fe780a3831d5a2414df632cd3f0aaf9
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index a6c984f..0206791 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1473,7 +1473,7 @@ static void ata_dev_init(struct ata_port
  *	the port has no enabled device after probing, or any negative
  *	errno from ->probe_reset.
  */
-static int ata_bus_probe(struct ata_port *ap)
+int ata_bus_probe(struct ata_port *ap)
 {
 	int existing_dev_failed = 0;
 	unsigned int classes[ATA_MAX_DEVICES];
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index 7faa9a4..659badb 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -43,6 +43,7 @@ struct ata_scsi_args {
 extern int atapi_enabled;
 extern int atapi_dmadir;
 extern int libata_fua;
+extern int ata_bus_probe(struct ata_port *ap);
 extern struct ata_queued_cmd *ata_qc_new_init(struct ata_port *ap,
 				      struct ata_device *dev);
 extern int ata_rwcmd_protocol(struct ata_queued_cmd *qc);
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 12/13] libata-hp: store attached SCSI device
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (9 preceding siblings ...)
  2006-04-11 14:06 ` [PATCH 11/13] libata-hp: add hotplug flags and flag handling functions Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-27  9:20   ` Jeff Garzik
  2006-04-11 14:06 ` [PATCH 13/13] libata-hp: add ata_hotplug_wq Tejun Heo
                   ` (3 subsequent siblings)
  14 siblings, 1 reply; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

Add device persistent field dev->sdev and store the attached SCSI
device.  With hotplug, libata needs to know the attached SCSI device
to offline and detach it, but scsi_device_lookup() cannot be used
because libata will reuse SCSI ID numbers - dead but not gone devices
(due to zombie opens, etc...) interfere with the lookup.

dev->sdev doesn't hold reference to the SCSI device.  dev->sdev is
cleared when the SCSI device goes away.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-scsi.c |   14 ++++++++++----
 include/linux/libata.h     |    1 +
 2 files changed, 11 insertions(+), 4 deletions(-)

784f79074e0ab0441363c7a87d1b93d6d11ac437
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index 4ce390a..1793474 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -2740,17 +2740,23 @@ void ata_scsi_simulate(struct ata_port *
 
 void ata_scsi_scan_host(struct ata_port *ap)
 {
-	struct ata_device *dev;
 	unsigned int i;
 
 	if (ap->flags & ATA_FLAG_DISABLED)
 		return;
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
-		dev = &ap->device[i];
+		struct ata_device *dev = &ap->device[i];
 
-		if (ata_dev_enabled(dev))
-			scsi_scan_target(&ap->host->shost_gendev, 0, i, 0, 0);
+		if (ata_dev_enabled(dev) && dev->sdev == NULL) {
+			struct scsi_device *sdev;
+
+			sdev = __scsi_add_device(ap->host, 0, i, 0, NULL);
+			if (!IS_ERR(sdev)) {
+				dev->sdev = sdev;
+				scsi_device_put(sdev);
+			}
+		}
 	}
 }
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 2a93c44..5f8faca 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -407,6 +407,7 @@ struct ata_ering {
 struct ata_device {
 	unsigned int		devno;		/* 0 or 1 */
 	unsigned long		flags;		/* ATA_DFLAG_xxx */
+	struct scsi_device	*sdev;		/* attached SCSI device */
 	/* fields above n_sectors are not cleared across device init */
 	u64			n_sectors;	/* size of device, if ATA */
 	unsigned int		class;		/* ATA_DEV_xxx */
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 09/13] libata-hp: prepare ata_bus_probe() for hotplug
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (11 preceding siblings ...)
  2006-04-11 14:06 ` [PATCH 13/13] libata-hp: add ata_hotplug_wq Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-12  1:41 ` [PATCHSET 8/9] prep for hotplug support Tejun Heo
  2006-04-27  9:22 ` Jeff Garzik
  14 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

Update ata_bus_probe() such that it probes only for ATA_DEV_UNKNOWN
devices, ignore errors from already existing devices and return 1 in
such cases.  This is in preparation for hotplug.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   37 +++++++++++++++++++++++++++++--------
 1 files changed, 29 insertions(+), 8 deletions(-)

57aed4a29833e54263afce342e44361642a6b8a0
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index e68a64d..a6c984f 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1469,18 +1469,24 @@ static void ata_dev_init(struct ata_port
  *	PCI/etc. bus probe sem.
  *
  *	RETURNS:
- *	Zero on success, negative errno otherwise.
+ *	Zero on success, one if any existing device failed, -ENODEV if
+ *	the port has no enabled device after probing, or any negative
+ *	errno from ->probe_reset.
  */
 static int ata_bus_probe(struct ata_port *ap)
 {
+	int existing_dev_failed = 0;
 	unsigned int classes[ATA_MAX_DEVICES];
 	int tries[ATA_MAX_DEVICES];
 	int i, rc, down_xfermask;
 	struct ata_device *dev;
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
-		tries[i] = ATA_PROBE_MAX_TRIES;
-		ata_dev_init(ap, &ap->device[i]);
+		if (ap->device[i].class == ATA_DEV_UNKNOWN) {
+			tries[i] = ATA_PROBE_MAX_TRIES;
+			ata_dev_init(ap, &ap->device[i]);
+		} else
+			tries[i] = -1;
 	}
 
  retry:
@@ -1514,6 +1520,16 @@ static int ata_bus_probe(struct ata_port
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		dev = &ap->device[i];
 
+		/* Existing devices are EH's responsitbilities.  Just
+		 * revalidate them.
+		 */
+		if (tries[i] < 0) {
+			if (ata_dev_enabled(dev))
+				if (ata_dev_revalidate(ap, dev, 1))
+					existing_dev_failed = 1;
+			continue;
+		}
+
 		if (tries[i])
 			dev->class = classes[i];
 
@@ -1547,14 +1563,19 @@ static int ata_bus_probe(struct ata_port
 		rc = ata_set_mode(ap, &dev);
 
 	if (rc) {
-		down_xfermask = 1;
-		goto fail;
+		if (tries[dev->devno] < 0) {
+			existing_dev_failed = 1;
+		} else {
+			down_xfermask = 1;
+			goto fail;
+		}
 	}
 
+	rc = -ENODEV;
 	for (i = 0; i < ATA_MAX_DEVICES; i++)
 		if (ata_dev_enabled(&ap->device[i]))
-			return 0;
-	return -ENODEV;
+			rc = 0;
+	return existing_dev_failed ? 1 : rc;
 
  fail:
 	switch (rc) {
@@ -5041,7 +5062,7 @@ int ata_device_add(const struct ata_prob
 		rc = ata_bus_probe(ap);
 		DPRINTK("ata%u: bus probe end\n", ap->id);
 
-		if (rc && !ap->ops->error_handler) {
+		if (rc < 0 && !ap->ops->error_handler) {
 			/* no device attached and no hotplug support */
 			ata_port_disable(ap);
 			ap->ops->port_disable(ap);
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 11/13] libata-hp: add hotplug flags and flag handling functions
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (8 preceding siblings ...)
  2006-04-11 14:06 ` [PATCH 10/13] libata-hp: make ata_bus_probe() extern Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-11 14:06 ` [PATCH 12/13] libata-hp: store attached SCSI device Tejun Heo
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

Add ap->hotplug_flags field, define ATA_HOTPLUG_*, ATA_DFLAG_DETACH_*
and flag manipulation functions.

As ap->hotplug_flags will be accessed by EH and SCSI hotplug work
simultaneosly, changes should be atomic.  Define functions to
manipulate ap->hostplug_flags - ata_set_hotplug_flags(),
ata_clr_hotplug_flags() and ata_schedule_probe(), which is a shortcut
for setting ATA_HOTPLUG_PROBE, a common operation for LLDDs.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 include/linux/libata.h |   37 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 36 insertions(+), 1 deletions(-)

a0d2a754a788df7a123013480a3295d0c61c0940
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 94f1e9a..2a93c44 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -130,6 +130,9 @@ enum {
 	ATA_DFLAG_FAILED	= (1 << 9), /* device has failed */
 	ATA_DFLAG_INIT_MASK	= (1 << 16) - 1,
 
+	ATA_DFLAG_DETACH_ATA	= (1 << 16), /* detach ATA device */
+	ATA_DFLAG_DETACH_SCSI	= (1 << 17), /* detach SCSI device */
+
 	ATA_DEV_UNKNOWN		= 0,	/* unknown device */
 	ATA_DEV_ATA		= 1,	/* ATA device */
 	ATA_DEV_ATA_UNSUP	= 2,	/* ATA device (unsupported) */
@@ -161,9 +164,15 @@ enum {
 	ATA_FLAG_DISABLED	= (1 << 19), /* port is disabled, ignore it */
 	ATA_FLAG_FROZEN		= (1 << 20), /* port is frozen */
 	ATA_FLAG_SUSPENDED	= (1 << 21), /* port is suspended (power) */
-
 	/* bits 24:31 of ap->flags are reserved for LLDD specific flags */
 
+	/* struct ata_port hotplug_flags */
+	ATA_HOTPLUG_RUNNING	= (1 << 0), /* hotplugging online */
+	ATA_HOTPLUG_PROBE	= (1 << 1), /* probe requested */
+	ATA_HOTPLUG_DID_PROBE	= (1 << 2), /* already probed in this run */
+	ATA_HOTPLUG_SCSI_PLUG	= (1 << 3), /* SCSI hotplug scheduled */
+	ATA_HOTPLUG_SCSI_UNPLUG	= (1 << 4), /* SCSI hotunplug scheduled */
+
 	/* struct ata_queued_cmd flags */
 	ATA_QCFLAG_ACTIVE	= (1 << 0), /* cmd not yet ack'd to scsi lyer */
 	ATA_QCFLAG_SG		= (1 << 1), /* have s/g table? */
@@ -471,6 +480,8 @@ struct ata_port {
 	u32			msg_enable;
 	struct list_head	eh_done_q;
 
+	unsigned long		hotplug_flags;
+
 	void			*private_data;
 };
 
@@ -800,6 +811,30 @@ static inline unsigned int ata_dev_absen
 	return ata_class_absent(dev->class);
 }
 
+/* hotplug flag manipulation helpers */
+static inline void ata_set_hotplug_flags(struct ata_port *ap,
+					 unsigned int flags)
+{
+	unsigned long old;
+	do {
+		old = ap->hotplug_flags;
+	} while (cmpxchg(&ap->hotplug_flags, old, old | flags) != old);
+}
+
+static inline void ata_clr_hotplug_flags(struct ata_port *ap,
+					 unsigned int flags)
+{
+	unsigned long old;
+	do {
+		old = ap->hotplug_flags;
+	} while (cmpxchg(&ap->hotplug_flags, old, old & ~flags) != old);
+}
+
+static inline void ata_schedule_probe(struct ata_port *ap)
+{
+	ata_set_hotplug_flags(ap, ATA_HOTPLUG_PROBE);
+}
+
 static inline u8 ata_chk_status(struct ata_port *ap)
 {
 	return ap->ops->check_status(ap);
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 13/13] libata-hp: add ata_hotplug_wq
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (10 preceding siblings ...)
  2006-04-11 14:06 ` [PATCH 12/13] libata-hp: store attached SCSI device Tejun Heo
@ 2006-04-11 14:06 ` Tejun Heo
  2006-04-13  6:11   ` zhao, forrest
  2006-04-11 14:06 ` [PATCH 09/13] libata-hp: prepare ata_bus_probe() for hotplug Tejun Heo
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 24+ messages in thread
From: Tejun Heo @ 2006-04-11 14:06 UTC (permalink / raw)
  To: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide; +Cc: Tejun Heo

It's best to run ATA hotplug from EH but attaching SCSI devices needs
working EH.  ata_hotplug_wq is used to give SCSI hotplug operations a
separate context.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |    9 +++++++++
 drivers/scsi/libata.h      |    1 +
 2 files changed, 10 insertions(+), 0 deletions(-)

1438492c4408fc908d3fb5865fd56231360a2fcd
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 0206791..610f963 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -72,6 +72,8 @@ static void ata_dev_xfermask(struct ata_
 static unsigned int ata_unique_id = 1;
 static struct workqueue_struct *ata_wq;
 
+struct workqueue_struct *ata_hotplug_wq;
+
 int atapi_enabled = 1;
 module_param(atapi_enabled, int, 0444);
 MODULE_PARM_DESC(atapi_enabled, "Enable discovery of ATAPI devices (0=off, 1=on)");
@@ -5300,6 +5302,12 @@ static int __init ata_init(void)
 	if (!ata_wq)
 		return -ENOMEM;
 
+	ata_hotplug_wq = create_workqueue("ata_hotplug");
+	if (!ata_hotplug_wq) {
+		destroy_workqueue(ata_wq);
+		return -ENOMEM;
+	}
+
 	printk(KERN_DEBUG "libata version " DRV_VERSION " loaded.\n");
 	return 0;
 }
@@ -5307,6 +5315,7 @@ static int __init ata_init(void)
 static void __exit ata_exit(void)
 {
 	destroy_workqueue(ata_wq);
+	destroy_workqueue(ata_hotplug_wq);
 }
 
 module_init(ata_init);
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index 659badb..09fddf6 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -40,6 +40,7 @@ struct ata_scsi_args {
 };
 
 /* libata-core.c */
+extern struct workqueue_struct *ata_hotplug_wq;
 extern int atapi_enabled;
 extern int atapi_dmadir;
 extern int libata_fua;
-- 
1.2.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCHSET 8/9] prep for hotplug support
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (12 preceding siblings ...)
  2006-04-11 14:06 ` [PATCH 09/13] libata-hp: prepare ata_bus_probe() for hotplug Tejun Heo
@ 2006-04-12  1:41 ` Tejun Heo
  2006-04-27  9:22 ` Jeff Garzik
  14 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-12  1:41 UTC (permalink / raw)
  To: Tejun Heo; +Cc: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide

Tejun Heo wrote:
> This is the first take of prep-for-hotplug-support patchset.  As the
> name suggests, this patchset prepares libata for hotplug support.
> This patchset contains 15 patches.

13 patches, obviously. :/

> #01-02	update sata_sil (hotplug related constants, new interrupt handler)

These two are moved to the next patchset right before sata_sil hotplug 
update at the last minutes and I forgot to update the head message.  Sorry.

> #03-04	misc libata preps
> #05-12	prepare ata_bus_probe()
> #13-15	hotplug flags, extra fields, hotplug_wq

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 13/13] libata-hp: add ata_hotplug_wq
  2006-04-11 14:06 ` [PATCH 13/13] libata-hp: add ata_hotplug_wq Tejun Heo
@ 2006-04-13  6:11   ` zhao, forrest
  2006-04-13  6:33     ` Tejun Heo
  0 siblings, 1 reply; 24+ messages in thread
From: zhao, forrest @ 2006-04-13  6:11 UTC (permalink / raw)
  To: Tejun Heo; +Cc: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide

On Tue, 2006-04-11 at 23:06 +0900, Tejun Heo wrote:
> 1438492c4408fc908d3fb5865fd56231360a2fcd
> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
> index 0206791..610f963 100644
> --- a/drivers/scsi/libata-core.c
> +++ b/drivers/scsi/libata-core.c
> @@ -72,6 +72,8 @@ static void ata_dev_xfermask(struct ata_
>  static unsigned int ata_unique_id = 1;
>  static struct workqueue_struct *ata_wq;
>  
> +struct workqueue_struct *ata_hotplug_wq;
> +
>  int atapi_enabled = 1;
>  module_param(atapi_enabled, int, 0444);
>  MODULE_PARM_DESC(atapi_enabled, "Enable discovery of ATAPI devices (0=off, 1=on)");
> @@ -5300,6 +5302,12 @@ static int __init ata_init(void)
>  	if (!ata_wq)
>  		return -ENOMEM;
>  
> +	ata_hotplug_wq = create_workqueue("ata_hotplug");
> +	if (!ata_hotplug_wq) {
> +		destroy_workqueue(ata_wq);
> +		return -ENOMEM;
> +	}
> +
>  	printk(KERN_DEBUG "libata version " DRV_VERSION " loaded.\n");
>  	return 0;
>  }

create_workqueue() creates one workqueue for each logic CPU in
the system, so when loading libata.ko in a system with 8 CPU's
8 work queues for ata_hotplug will be created. Does it make sense
to use create_singlethread_workqueue() to create only one workqueue
for ata_hotplug since hotplug is not the operation happened with
high frequency?

Thanks,
Forrest

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 13/13] libata-hp: add ata_hotplug_wq
  2006-04-13  6:11   ` zhao, forrest
@ 2006-04-13  6:33     ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-13  6:33 UTC (permalink / raw)
  To: zhao, forrest; +Cc: jgarzik, alan, axboe, albertcc, lkosewsk, linux-ide

zhao, forrest wrote:
> On Tue, 2006-04-11 at 23:06 +0900, Tejun Heo wrote:
>> 1438492c4408fc908d3fb5865fd56231360a2fcd
>> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
>> index 0206791..610f963 100644
>> --- a/drivers/scsi/libata-core.c
>> +++ b/drivers/scsi/libata-core.c
>> @@ -72,6 +72,8 @@ static void ata_dev_xfermask(struct ata_
>>  static unsigned int ata_unique_id = 1;
>>  static struct workqueue_struct *ata_wq;
>>  
>> +struct workqueue_struct *ata_hotplug_wq;
>> +
>>  int atapi_enabled = 1;
>>  module_param(atapi_enabled, int, 0444);
>>  MODULE_PARM_DESC(atapi_enabled, "Enable discovery of ATAPI devices (0=off, 1=on)");
>> @@ -5300,6 +5302,12 @@ static int __init ata_init(void)
>>  	if (!ata_wq)
>>  		return -ENOMEM;
>>  
>> +	ata_hotplug_wq = create_workqueue("ata_hotplug");
>> +	if (!ata_hotplug_wq) {
>> +		destroy_workqueue(ata_wq);
>> +		return -ENOMEM;
>> +	}
>> +
>>  	printk(KERN_DEBUG "libata version " DRV_VERSION " loaded.\n");
>>  	return 0;
>>  }
> 
> create_workqueue() creates one workqueue for each logic CPU in
> the system, so when loading libata.ko in a system with 8 CPU's
> 8 work queues for ata_hotplug will be created. Does it make sense
> to use create_singlethread_workqueue() to create only one workqueue
> for ata_hotplug since hotplug is not the operation happened with
> high frequency?
> 

Yeap, that makes a lot of sense.  Will change to single threaded variant.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 07/13] libata-hp: implement ap->orig_sata_spd_limit
  2006-04-11 14:06 ` [PATCH 07/13] libata-hp: implement ap->orig_sata_spd_limit Tejun Heo
@ 2006-04-27  9:19   ` Jeff Garzik
  2006-04-27 10:18     ` Tejun Heo
  0 siblings, 1 reply; 24+ messages in thread
From: Jeff Garzik @ 2006-04-27  9:19 UTC (permalink / raw)
  To: Tejun Heo; +Cc: alan, axboe, albertcc, lkosewsk, linux-ide

Tejun Heo wrote:
> Add ap->orig_sata_spd_limit and initialize it once during the boot
> initialization (or driver load initialization).  ap->sata_spd_limit is
> reset to ap->orig_sata_spd_limit on hotplug.  This prevents limits
> introduced by earlier devices from affecting new devices.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

Rather than "orig_" call it "hw_" or similar.  This is similar to the 
block layer's max_sectors and max_hw_sectors situation.

	Jeff




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 12/13] libata-hp: store attached SCSI device
  2006-04-11 14:06 ` [PATCH 12/13] libata-hp: store attached SCSI device Tejun Heo
@ 2006-04-27  9:20   ` Jeff Garzik
  0 siblings, 0 replies; 24+ messages in thread
From: Jeff Garzik @ 2006-04-27  9:20 UTC (permalink / raw)
  To: Tejun Heo; +Cc: alan, axboe, albertcc, lkosewsk, linux-ide

Tejun Heo wrote:
> Add device persistent field dev->sdev and store the attached SCSI
> device.  With hotplug, libata needs to know the attached SCSI device
> to offline and detach it, but scsi_device_lookup() cannot be used
> because libata will reuse SCSI ID numbers - dead but not gone devices
> (due to zombie opens, etc...) interfere with the lookup.
> 
> dev->sdev doesn't hold reference to the SCSI device.  dev->sdev is
> cleared when the SCSI device goes away.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

ACK, though we'll have to revisit once libata supports direct use of 
block layer for ATA disks.  But given other email it sounds like you are 
thinking about this already.

	Jeff




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHSET 8/9] prep for hotplug support
  2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
                   ` (13 preceding siblings ...)
  2006-04-12  1:41 ` [PATCHSET 8/9] prep for hotplug support Tejun Heo
@ 2006-04-27  9:22 ` Jeff Garzik
  2006-04-27 10:27   ` Tejun Heo
  14 siblings, 1 reply; 24+ messages in thread
From: Jeff Garzik @ 2006-04-27  9:22 UTC (permalink / raw)
  To: Tejun Heo; +Cc: alan, axboe, albertcc, lkosewsk, linux-ide

Tejun Heo wrote:
> Hello, all.
> 
> This is the first take of prep-for-hotplug-support patchset.  As the
> name suggests, this patchset prepares libata for hotplug support.
> This patchset contains 15 patches.
> 
> #01-02	update sata_sil (hotplug related constants, new interrupt handler)
> #03-04	misc libata preps
> #05-12	prepare ata_bus_probe()
> #13-15	hotplug flags, extra fields, hotplug_wq

ACK, modulo the minor comments just sent.

Also, in general libata needs a rethink when it comes to workqueues. 
Sometimes a single thread is more appropriate than the workqueue default 
of one-thread-per-CPU.  Using workqueues in the default state is quite 
efficient, but can also given you a huge number of idle threads on a 
$BIGNUM CPU box.  Workqueues on a 16-CPU box can be pretty insane...

	Jeff




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 07/13] libata-hp: implement ap->orig_sata_spd_limit
  2006-04-27  9:19   ` Jeff Garzik
@ 2006-04-27 10:18     ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-27 10:18 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: alan, axboe, albertcc, lkosewsk, linux-ide

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Add ap->orig_sata_spd_limit and initialize it once during the boot
>> initialization (or driver load initialization).  ap->sata_spd_limit is
>> reset to ap->orig_sata_spd_limit on hotplug.  This prevents limits
>> introduced by earlier devices from affecting new devices.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> Rather than "orig_" call it "hw_" or similar.  This is similar to the 
> block layer's max_sectors and max_hw_sectors situation.
> 

Sure.  I'll choose one of hw_sata_spd_limit or phy_sata_spd_limit.

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHSET 8/9] prep for hotplug support
  2006-04-27  9:22 ` Jeff Garzik
@ 2006-04-27 10:27   ` Tejun Heo
  2006-04-27 10:36     ` Jeff Garzik
  0 siblings, 1 reply; 24+ messages in thread
From: Tejun Heo @ 2006-04-27 10:27 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: alan, axboe, albertcc, lkosewsk, linux-ide

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Hello, all.
>>
>> This is the first take of prep-for-hotplug-support patchset.  As the
>> name suggests, this patchset prepares libata for hotplug support.
>> This patchset contains 15 patches.
>>
>> #01-02    update sata_sil (hotplug related constants, new interrupt 
>> handler)
>> #03-04    misc libata preps
>> #05-12    prepare ata_bus_probe()
>> #13-15    hotplug flags, extra fields, hotplug_wq
> 
> ACK, modulo the minor comments just sent.
> 
> Also, in general libata needs a rethink when it comes to workqueues. 
> Sometimes a single thread is more appropriate than the workqueue default 
> of one-thread-per-CPU.  Using workqueues in the default state is quite 
> efficient, but can also given you a huge number of idle threads on a 
> $BIGNUM CPU box.  Workqueues on a 16-CPU box can be pretty insane...
> 

Yeah, Forrest pointed out the same thing and I've converted hotplug_wq 
to single threaded variant, which should be enough for SCSI hotplugging. 
  For ata_wq, I think it's wiser to keep it multi-threaded for the time 
being.  Maybe what should be done is to allocate a single threaded 
workqueue per host_set.

$BIGNUM CPU boxes tend to have $BIGNUM GB of memories, so multithreaded 
wq might not be that critical resource-wise.  But, still, inefficient 
and doesn't scale well.

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHSET 8/9] prep for hotplug support
  2006-04-27 10:27   ` Tejun Heo
@ 2006-04-27 10:36     ` Jeff Garzik
  2006-04-27 10:47       ` Tejun Heo
  0 siblings, 1 reply; 24+ messages in thread
From: Jeff Garzik @ 2006-04-27 10:36 UTC (permalink / raw)
  To: Tejun Heo; +Cc: alan, axboe, albertcc, lkosewsk, linux-ide

Tejun Heo wrote:
> Yeah, Forrest pointed out the same thing and I've converted hotplug_wq 
> to single threaded variant, which should be enough for SCSI hotplugging. 
>  For ata_wq, I think it's wiser to keep it multi-threaded for the time 
> being.  Maybe what should be done is to allocate a single threaded 
> workqueue per host_set.

One thread per host_set would suck for any situation where that's doing 
ATAPI or PIO tasks.  Would love to find a good metric...


> $BIGNUM CPU boxes tend to have $BIGNUM GB of memories, so multithreaded 
> wq might not be that critical resource-wise.  But, still, inefficient 

That's a bad assumption on modern machines.  Think of a single 16-core, 
8-thread SPARC Niagara CPU, for example :)

	Jeff




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHSET 8/9] prep for hotplug support
  2006-04-27 10:36     ` Jeff Garzik
@ 2006-04-27 10:47       ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-04-27 10:47 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: alan, axboe, albertcc, lkosewsk, linux-ide

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Yeah, Forrest pointed out the same thing and I've converted hotplug_wq 
>> to single threaded variant, which should be enough for SCSI 
>> hotplugging.  For ata_wq, I think it's wiser to keep it multi-threaded 
>> for the time being.  Maybe what should be done is to allocate a single 
>> threaded workqueue per host_set.
> 
> One thread per host_set would suck for any situation where that's doing 
> ATAPI or PIO tasks.  Would love to find a good metric...

Dynamic pool of worker threads maybe?

> 
>> $BIGNUM CPU boxes tend to have $BIGNUM GB of memories, so 
>> multithreaded wq might not be that critical resource-wise.  But, 
>> still, inefficient 
> 
> That's a bad assumption on modern machines.  Think of a single 16-core, 
> 8-thread SPARC Niagara CPU, for example :)
> 

Yeah, that one is pretty extreme.  But most mainstream machines have <= 
8 threads of execution and even they are equipped with GB's of memory. 
But, I do agree that we want something better.

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2006-04-27 10:47 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-04-11 14:06 [PATCHSET 8/9] prep for hotplug support Tejun Heo
2006-04-11 14:06 ` [PATCH 08/13] libata-hp: move device enable/disable out of ata_bus_probe() Tejun Heo
2006-04-11 14:06 ` [PATCH 04/13] libata-hp: make some fields of ata_device persistent Tejun Heo
2006-04-11 14:06 ` [PATCH 01/13] libata-hp: separate out __ata_scsi_find_dev() Tejun Heo
2006-04-11 14:06 ` [PATCH 07/13] libata-hp: implement ap->orig_sata_spd_limit Tejun Heo
2006-04-27  9:19   ` Jeff Garzik
2006-04-27 10:18     ` Tejun Heo
2006-04-11 14:06 ` [PATCH 06/13] libata-hp: prepare for persistent device flags Tejun Heo
2006-04-11 14:06 ` [PATCH 05/13] libata-hp: call ata_dev_init() from ata_bus_probe() Tejun Heo
2006-04-11 14:06 ` [PATCH 03/13] libata-hp: implement ata_dev_init() Tejun Heo
2006-04-11 14:06 ` [PATCH 02/13] libata-hp: add more SERR_* constants in preparation for hotplug support Tejun Heo
2006-04-11 14:06 ` [PATCH 10/13] libata-hp: make ata_bus_probe() extern Tejun Heo
2006-04-11 14:06 ` [PATCH 11/13] libata-hp: add hotplug flags and flag handling functions Tejun Heo
2006-04-11 14:06 ` [PATCH 12/13] libata-hp: store attached SCSI device Tejun Heo
2006-04-27  9:20   ` Jeff Garzik
2006-04-11 14:06 ` [PATCH 13/13] libata-hp: add ata_hotplug_wq Tejun Heo
2006-04-13  6:11   ` zhao, forrest
2006-04-13  6:33     ` Tejun Heo
2006-04-11 14:06 ` [PATCH 09/13] libata-hp: prepare ata_bus_probe() for hotplug Tejun Heo
2006-04-12  1:41 ` [PATCHSET 8/9] prep for hotplug support Tejun Heo
2006-04-27  9:22 ` Jeff Garzik
2006-04-27 10:27   ` Tejun Heo
2006-04-27 10:36     ` Jeff Garzik
2006-04-27 10:47       ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).