[PATCH 0/2] hpsa: add sysfs host attribute to indicate whether reset_devices is honored

[PATCH 0/2] hpsa: add sysfs host attribute to indicate whether reset_devices is honored

[Stephen M. Cameron]

At Redhat's request, this adds a sysfs host attribute to indicate whether
the reset_devices kernel parameter can be honored by the device.  This enables 
kexec tools to warn the user if they attempt to designate a non-resettable device
as the dump device.  If the device is not resettable, kdump won't work.

---

Stephen M. Cameron (2):
      hpsa: move device attributes to avoid forward declarations
      hpsa: export resettable_on_kexec host attribute


 drivers/scsi/hpsa.c |  276 ++++++++++++++++++++++++++++-----------------------
 1 files changed, 152 insertions(+), 124 deletions(-)

-- 
-- steve

[PATCH 1/2] hpsa: move device attributes to avoid forward declarations

[Stephen M. Cameron]

From: Stephen M. Cameron <scameron@beardog.cce.hp.com>

Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
---
 drivers/scsi/hpsa.c |  253 ++++++++++++++++++++++++---------------------------
 1 files changed, 120 insertions(+), 133 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 09a529e..dcabef4 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -155,21 +155,7 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd);
 static int hpsa_slave_alloc(struct scsi_device *sdev);
 static void hpsa_slave_destroy(struct scsi_device *sdev);
 
-static ssize_t raid_level_show(struct device *dev,
-	struct device_attribute *attr, char *buf);
-static ssize_t lunid_show(struct device *dev,
-	struct device_attribute *attr, char *buf);
-static ssize_t unique_id_show(struct device *dev,
-	struct device_attribute *attr, char *buf);
-static ssize_t host_show_firmware_revision(struct device *dev,
-	     struct device_attribute *attr, char *buf);
-static ssize_t host_show_commands_outstanding(struct device *dev,
-	     struct device_attribute *attr, char *buf);
-static ssize_t host_show_transport_mode(struct device *dev,
-	struct device_attribute *attr, char *buf);
 static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno);
-static ssize_t host_store_rescan(struct device *dev,
-	 struct device_attribute *attr, const char *buf, size_t count);
 static int check_for_unit_attention(struct ctlr_info *h,
 	struct CommandList *c);
 static void check_ioctl_unit_attention(struct ctlr_info *h,
@@ -190,53 +176,6 @@ static int __devinit hpsa_wait_for_board_state(struct pci_dev *pdev,
 #define BOARD_NOT_READY 0
 #define BOARD_READY 1
 
-static DEVICE_ATTR(raid_level, S_IRUGO, raid_level_show, NULL);
-static DEVICE_ATTR(lunid, S_IRUGO, lunid_show, NULL);
-static DEVICE_ATTR(unique_id, S_IRUGO, unique_id_show, NULL);
-static DEVICE_ATTR(rescan, S_IWUSR, NULL, host_store_rescan);
-static DEVICE_ATTR(firmware_revision, S_IRUGO,
-	host_show_firmware_revision, NULL);
-static DEVICE_ATTR(commands_outstanding, S_IRUGO,
-	host_show_commands_outstanding, NULL);
-static DEVICE_ATTR(transport_mode, S_IRUGO,
-	host_show_transport_mode, NULL);
-
-static struct device_attribute *hpsa_sdev_attrs[] = {
-	&dev_attr_raid_level,
-	&dev_attr_lunid,
-	&dev_attr_unique_id,
-	NULL,
-};
-
-static struct device_attribute *hpsa_shost_attrs[] = {
-	&dev_attr_rescan,
-	&dev_attr_firmware_revision,
-	&dev_attr_commands_outstanding,
-	&dev_attr_transport_mode,
-	NULL,
-};
-
-static struct scsi_host_template hpsa_driver_template = {
-	.module			= THIS_MODULE,
-	.name			= "hpsa",
-	.proc_name		= "hpsa",
-	.queuecommand		= hpsa_scsi_queue_command,
-	.scan_start		= hpsa_scan_start,
-	.scan_finished		= hpsa_scan_finished,
-	.change_queue_depth	= hpsa_change_queue_depth,
-	.this_id		= -1,
-	.use_clustering		= ENABLE_CLUSTERING,
-	.eh_device_reset_handler = hpsa_eh_device_reset_handler,
-	.ioctl			= hpsa_ioctl,
-	.slave_alloc		= hpsa_slave_alloc,
-	.slave_destroy		= hpsa_slave_destroy,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl		= hpsa_compat_ioctl,
-#endif
-	.sdev_attrs = hpsa_sdev_attrs,
-	.shost_attrs = hpsa_shost_attrs,
-};
-
 static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev)
 {
 	unsigned long *priv = shost_priv(sdev->host);
@@ -334,83 +273,11 @@ static ssize_t host_show_transport_mode(struct device *dev,
 			"performant" : "simple");
 }
 
-/* Enqueuing and dequeuing functions for cmdlists. */
-static inline void addQ(struct list_head *list, struct CommandList *c)
-{
-	list_add_tail(&c->list, list);
-}
-
-static inline u32 next_command(struct ctlr_info *h)
-{
-	u32 a;
-
-	if (unlikely(!(h->transMethod & CFGTBL_Trans_Performant)))
-		return h->access.command_completed(h);
-
-	if ((*(h->reply_pool_head) & 1) == (h->reply_pool_wraparound)) {
-		a = *(h->reply_pool_head); /* Next cmd in ring buffer */
-		(h->reply_pool_head)++;
-		h->commands_outstanding--;
-	} else {
-		a = FIFO_EMPTY;
-	}
-	/* Check for wraparound */
-	if (h->reply_pool_head == (h->reply_pool + h->max_commands)) {
-		h->reply_pool_head = h->reply_pool;
-		h->reply_pool_wraparound ^= 1;
-	}
-	return a;
-}
-
-/* set_performant_mode: Modify the tag for cciss performant
- * set bit 0 for pull model, bits 3-1 for block fetch
- * register number
- */
-static void set_performant_mode(struct ctlr_info *h, struct CommandList *c)
-{
-	if (likely(h->transMethod & CFGTBL_Trans_Performant))
-		c->busaddr |= 1 | (h->blockFetchTable[c->Header.SGList] << 1);
-}
-
-static void enqueue_cmd_and_start_io(struct ctlr_info *h,
-	struct CommandList *c)
-{
-	unsigned long flags;
-
-	set_performant_mode(h, c);
-	spin_lock_irqsave(&h->lock, flags);
-	addQ(&h->reqQ, c);
-	h->Qdepth++;
-	start_io(h);
-	spin_unlock_irqrestore(&h->lock, flags);
-}
-
-static inline void removeQ(struct CommandList *c)
-{
-	if (WARN_ON(list_empty(&c->list)))
-		return;
-	list_del_init(&c->list);
-}
-
-static inline int is_hba_lunid(unsigned char scsi3addr[])
-{
-	return memcmp(scsi3addr, RAID_CTLR_LUNID, 8) == 0;
-}
-
 static inline int is_logical_dev_addr_mode(unsigned char scsi3addr[])
 {
 	return (scsi3addr[3] & 0xC0) == 0x40;
 }
 
-static inline int is_scsi_rev_5(struct ctlr_info *h)
-{
-	if (!h->hba_inquiry_data)
-		return 0;
-	if ((h->hba_inquiry_data[2] & 0x07) == 5)
-		return 1;
-	return 0;
-}
-
 static const char *raid_label[] = { "0", "4", "1(1+0)", "5", "5+1", "ADG",
 	"UNKNOWN"
 };
@@ -502,6 +369,126 @@ static ssize_t unique_id_show(struct device *dev,
 			sn[12], sn[13], sn[14], sn[15]);
 }
 
+static DEVICE_ATTR(raid_level, S_IRUGO, raid_level_show, NULL);
+static DEVICE_ATTR(lunid, S_IRUGO, lunid_show, NULL);
+static DEVICE_ATTR(unique_id, S_IRUGO, unique_id_show, NULL);
+static DEVICE_ATTR(rescan, S_IWUSR, NULL, host_store_rescan);
+static DEVICE_ATTR(firmware_revision, S_IRUGO,
+	host_show_firmware_revision, NULL);
+static DEVICE_ATTR(commands_outstanding, S_IRUGO,
+	host_show_commands_outstanding, NULL);
+static DEVICE_ATTR(transport_mode, S_IRUGO,
+	host_show_transport_mode, NULL);
+
+static struct device_attribute *hpsa_sdev_attrs[] = {
+	&dev_attr_raid_level,
+	&dev_attr_lunid,
+	&dev_attr_unique_id,
+	NULL,
+};
+
+static struct device_attribute *hpsa_shost_attrs[] = {
+	&dev_attr_rescan,
+	&dev_attr_firmware_revision,
+	&dev_attr_commands_outstanding,
+	&dev_attr_transport_mode,
+	NULL,
+};
+
+static struct scsi_host_template hpsa_driver_template = {
+	.module			= THIS_MODULE,
+	.name			= "hpsa",
+	.proc_name		= "hpsa",
+	.queuecommand		= hpsa_scsi_queue_command,
+	.scan_start		= hpsa_scan_start,
+	.scan_finished		= hpsa_scan_finished,
+	.change_queue_depth	= hpsa_change_queue_depth,
+	.this_id		= -1,
+	.use_clustering		= ENABLE_CLUSTERING,
+	.eh_device_reset_handler = hpsa_eh_device_reset_handler,
+	.ioctl			= hpsa_ioctl,
+	.slave_alloc		= hpsa_slave_alloc,
+	.slave_destroy		= hpsa_slave_destroy,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl		= hpsa_compat_ioctl,
+#endif
+	.sdev_attrs = hpsa_sdev_attrs,
+	.shost_attrs = hpsa_shost_attrs,
+};
+
+
+/* Enqueuing and dequeuing functions for cmdlists. */
+static inline void addQ(struct list_head *list, struct CommandList *c)
+{
+	list_add_tail(&c->list, list);
+}
+
+static inline u32 next_command(struct ctlr_info *h)
+{
+	u32 a;
+
+	if (unlikely(!(h->transMethod & CFGTBL_Trans_Performant)))
+		return h->access.command_completed(h);
+
+	if ((*(h->reply_pool_head) & 1) == (h->reply_pool_wraparound)) {
+		a = *(h->reply_pool_head); /* Next cmd in ring buffer */
+		(h->reply_pool_head)++;
+		h->commands_outstanding--;
+	} else {
+		a = FIFO_EMPTY;
+	}
+	/* Check for wraparound */
+	if (h->reply_pool_head == (h->reply_pool + h->max_commands)) {
+		h->reply_pool_head = h->reply_pool;
+		h->reply_pool_wraparound ^= 1;
+	}
+	return a;
+}
+
+/* set_performant_mode: Modify the tag for cciss performant
+ * set bit 0 for pull model, bits 3-1 for block fetch
+ * register number
+ */
+static void set_performant_mode(struct ctlr_info *h, struct CommandList *c)
+{
+	if (likely(h->transMethod & CFGTBL_Trans_Performant))
+		c->busaddr |= 1 | (h->blockFetchTable[c->Header.SGList] << 1);
+}
+
+static void enqueue_cmd_and_start_io(struct ctlr_info *h,
+	struct CommandList *c)
+{
+	unsigned long flags;
+
+	set_performant_mode(h, c);
+	spin_lock_irqsave(&h->lock, flags);
+	addQ(&h->reqQ, c);
+	h->Qdepth++;
+	start_io(h);
+	spin_unlock_irqrestore(&h->lock, flags);
+}
+
+static inline void removeQ(struct CommandList *c)
+{
+	if (WARN_ON(list_empty(&c->list)))
+		return;
+	list_del_init(&c->list);
+}
+
+static inline int is_hba_lunid(unsigned char scsi3addr[])
+{
+	return memcmp(scsi3addr, RAID_CTLR_LUNID, 8) == 0;
+}
+
+static inline int is_scsi_rev_5(struct ctlr_info *h)
+{
+	if (!h->hba_inquiry_data)
+		return 0;
+	if ((h->hba_inquiry_data[2] & 0x07) == 5)
+		return 1;
+	return 0;
+}
+
 static int hpsa_find_target_lun(struct ctlr_info *h,
 	unsigned char scsi3addr[], int bus, int *target, int *lun)
 {

[PATCH 2/2] hpsa: export resettable_on_kexec host attribute

[Stephen M. Cameron]

From: Stephen M. Cameron <scameron@beardog.cce.hp.com>

This attribute, requested by Redhat, allows kexec-tools to know
whether the controller can honor the reset_devices kernel parameter
and actually reset the controller.  For kdump to work properly it
is necessary that the reset_devices parameter be honored.  This
attribute enables kexec-tools to warn the user if they attempt to
designate a non-resettable controller as the dump device.

Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
---
 drivers/scsi/hpsa.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index dcabef4..1d4fe80 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -273,6 +273,44 @@ static ssize_t host_show_transport_mode(struct device *dev,
 			"performant" : "simple");
 }
 
+/* List of controllers which cannot be reset on kexec with reset_devices */
+static u32 unresettable_controller[] = {
+	0x324a103C, /* Smart Array P712m */
+	0x324b103C, /* SmartArray P711m */
+	0x3223103C, /* Smart Array P800 */
+	0x3234103C, /* Smart Array P400 */
+	0x3235103C, /* Smart Array P400i */
+	0x3211103C, /* Smart Array E200i */
+	0x3212103C, /* Smart Array E200 */
+	0x3213103C, /* Smart Array E200i */
+	0x3214103C, /* Smart Array E200i */
+	0x3215103C, /* Smart Array E200i */
+	0x3237103C, /* Smart Array E500 */
+	0x3223103C, /* Smart Array P800 */
+	0x3234103C, /* Smart Array P400 */
+	0x323D103C, /* Smart Array P700m */
+};
+
+static int resettable_on_kexec(struct ctlr_info *h)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(unresettable_controller); i++)
+		if (unresettable_controller[i] == h->board_id)
+			return 0;
+	return 1;
+}
+
+static ssize_t host_show_resettable_on_kexec(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct ctlr_info *h;
+	struct Scsi_Host *shost = class_to_shost(dev);
+
+	h = shost_to_hba(shost);
+	return snprintf(buf, 20, "%d\n", resettable_on_kexec(h));
+}
+
 static inline int is_logical_dev_addr_mode(unsigned char scsi3addr[])
 {
 	return (scsi3addr[3] & 0xC0) == 0x40;
@@ -379,6 +417,8 @@ static DEVICE_ATTR(commands_outstanding, S_IRUGO,
 	host_show_commands_outstanding, NULL);
 static DEVICE_ATTR(transport_mode, S_IRUGO,
 	host_show_transport_mode, NULL);
+static DEVICE_ATTR(resettable_on_kexec, S_IRUGO,
+	host_show_resettable_on_kexec, NULL);
 
 static struct device_attribute *hpsa_sdev_attrs[] = {
 	&dev_attr_raid_level,
@@ -392,6 +432,7 @@ static struct device_attribute *hpsa_shost_attrs[] = {
 	&dev_attr_firmware_revision,
 	&dev_attr_commands_outstanding,
 	&dev_attr_transport_mode,
+	&dev_attr_resettable_on_kexec,
 	NULL,
 };
 

Re: [PATCH 2/2] hpsa: export resettable_on_kexec host attribute

[Américo Wang]

On Tue, Mar 08, 2011 at 05:10:03PM -0600, Stephen M. Cameron wrote:
>From: Stephen M. Cameron <scameron@beardog.cce.hp.com>
>
>This attribute, requested by Redhat, allows kexec-tools to know
>whether the controller can honor the reset_devices kernel parameter
>and actually reset the controller.  For kdump to work properly it
>is necessary that the reset_devices parameter be honored.  This
>attribute enables kexec-tools to warn the user if they attempt to
>designate a non-resettable controller as the dump device.
>
>Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>

Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com>

Maybe we should docment this sysfs attribute somewhere?

Thanks.

Re: [PATCH 2/2] hpsa: export resettable_on_kexec host attribute

[Tomas Henzl]

On 03/09/2011 12:10 AM, Stephen M. Cameron wrote:
> From: Stephen M. Cameron <scameron@beardog.cce.hp.com>
>
> This attribute, requested by Redhat, allows kexec-tools to know
> whether the controller can honor the reset_devices kernel parameter
> and actually reset the controller.  For kdump to work properly it
> is necessary that the reset_devices parameter be honored.  This
> attribute enables kexec-tools to warn the user if they attempt to
> designate a non-resettable controller as the dump device.
>
> Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> ---
>  drivers/scsi/hpsa.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index dcabef4..1d4fe80 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -273,6 +273,44 @@ static ssize_t host_show_transport_mode(struct device *dev,
>  			"performant" : "simple");
>  }
>  
> +/* List of controllers which cannot be reset on kexec with reset_devices */
> +static u32 unresettable_controller[] = {
> +	0x324a103C, /* Smart Array P712m */
> +	0x324b103C, /* SmartArray P711m */
> +	0x3223103C, /* Smart Array P800 */
> +	0x3234103C, /* Smart Array P400 */
> +	0x3235103C, /* Smart Array P400i */
> +	0x3211103C, /* Smart Array E200i */
> +	0x3212103C, /* Smart Array E200 */
> +	0x3213103C, /* Smart Array E200i */
> +	0x3214103C, /* Smart Array E200i */
> +	0x3215103C, /* Smart Array E200i */
> +	0x3237103C, /* Smart Array E500 */
> +	0x3223103C, /* Smart Array P800 */
> +	0x3234103C, /* Smart Array P400 */
> +	0x323D103C, /* Smart Array P700m */
> +};
>   
Hi Stephen,

thanks for posting this.
Some of the devices are served by the cciss driver by default - I guess
a very similar patch for cciss is needed too.
Shouldn't be the 0x409C0E11 and 0x409D0E11 (640x boards) also added to the list?
(And the 'unknown' devices.)

Tomas

> +
> +static int resettable_on_kexec(struct ctlr_info *h)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(unresettable_controller); i++)
> +		if (unresettable_controller[i] == h->board_id)
> +			return 0;
> +	return 1;
> +}
> +
> +static ssize_t host_show_resettable_on_kexec(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct ctlr_info *h;
> +	struct Scsi_Host *shost = class_to_shost(dev);
> +
> +	h = shost_to_hba(shost);
> +	return snprintf(buf, 20, "%d\n", resettable_on_kexec(h));
> +}
> +
>  static inline int is_logical_dev_addr_mode(unsigned char scsi3addr[])
>  {
>  	return (scsi3addr[3] & 0xC0) == 0x40;
> @@ -379,6 +417,8 @@ static DEVICE_ATTR(commands_outstanding, S_IRUGO,
>  	host_show_commands_outstanding, NULL);
>  static DEVICE_ATTR(transport_mode, S_IRUGO,
>  	host_show_transport_mode, NULL);
> +static DEVICE_ATTR(resettable_on_kexec, S_IRUGO,
> +	host_show_resettable_on_kexec, NULL);
>  
>  static struct device_attribute *hpsa_sdev_attrs[] = {
>  	&dev_attr_raid_level,
> @@ -392,6 +432,7 @@ static struct device_attribute *hpsa_shost_attrs[] = {
>  	&dev_attr_firmware_revision,
>  	&dev_attr_commands_outstanding,
>  	&dev_attr_transport_mode,
> +	&dev_attr_resettable_on_kexec,
>  	NULL,
>  };
>  
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   

Re: [PATCH 2/2] hpsa: export resettable_on_kexec host attribute

[scameron]

On Wed, Mar 09, 2011 at 01:27:53PM +0100, Tomas Henzl wrote:
> On 03/09/2011 12:10 AM, Stephen M. Cameron wrote:
> > From: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> >
> > This attribute, requested by Redhat, allows kexec-tools to know
> > whether the controller can honor the reset_devices kernel parameter
> > and actually reset the controller.  For kdump to work properly it
> > is necessary that the reset_devices parameter be honored.  This
> > attribute enables kexec-tools to warn the user if they attempt to
> > designate a non-resettable controller as the dump device.
> >
> > Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> > ---
> >  drivers/scsi/hpsa.c |   41 +++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 41 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> > index dcabef4..1d4fe80 100644
> > --- a/drivers/scsi/hpsa.c
> > +++ b/drivers/scsi/hpsa.c
> > @@ -273,6 +273,44 @@ static ssize_t host_show_transport_mode(struct device *dev,
> >  			"performant" : "simple");
> >  }
> >  
> > +/* List of controllers which cannot be reset on kexec with reset_devices */
> > +static u32 unresettable_controller[] = {
> > +	0x324a103C, /* Smart Array P712m */
> > +	0x324b103C, /* SmartArray P711m */
> > +	0x3223103C, /* Smart Array P800 */
> > +	0x3234103C, /* Smart Array P400 */
> > +	0x3235103C, /* Smart Array P400i */
> > +	0x3211103C, /* Smart Array E200i */
> > +	0x3212103C, /* Smart Array E200 */
> > +	0x3213103C, /* Smart Array E200i */
> > +	0x3214103C, /* Smart Array E200i */
> > +	0x3215103C, /* Smart Array E200i */
> > +	0x3237103C, /* Smart Array E500 */
> > +	0x3223103C, /* Smart Array P800 */
> > +	0x3234103C, /* Smart Array P400 */
> > +	0x323D103C, /* Smart Array P700m */
> > +};
> >   
> Hi Stephen,
> 
> thanks for posting this.
> Some of the devices are served by the cciss driver by default - I guess
> a very similar patch for cciss is needed too.

Yes.

> Shouldn't be the 0x409C0E11 and 0x409D0E11 (640x boards) also added to the list?
> (And the 'unknown' devices.)

Yes, thanks.

-- steve

> 
> Tomas
> 
> > +
> > +static int resettable_on_kexec(struct ctlr_info *h)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(unresettable_controller); i++)
> > +		if (unresettable_controller[i] == h->board_id)
> > +			return 0;
> > +	return 1;
> > +}
> > +
> > +static ssize_t host_show_resettable_on_kexec(struct device *dev,
> > +	struct device_attribute *attr, char *buf)
> > +{
> > +	struct ctlr_info *h;
> > +	struct Scsi_Host *shost = class_to_shost(dev);
> > +
> > +	h = shost_to_hba(shost);
> > +	return snprintf(buf, 20, "%d\n", resettable_on_kexec(h));
> > +}
> > +
> >  static inline int is_logical_dev_addr_mode(unsigned char scsi3addr[])
> >  {
> >  	return (scsi3addr[3] & 0xC0) == 0x40;
> > @@ -379,6 +417,8 @@ static DEVICE_ATTR(commands_outstanding, S_IRUGO,
> >  	host_show_commands_outstanding, NULL);
> >  static DEVICE_ATTR(transport_mode, S_IRUGO,
> >  	host_show_transport_mode, NULL);
> > +static DEVICE_ATTR(resettable_on_kexec, S_IRUGO,
> > +	host_show_resettable_on_kexec, NULL);
> >  
> >  static struct device_attribute *hpsa_sdev_attrs[] = {
> >  	&dev_attr_raid_level,
> > @@ -392,6 +432,7 @@ static struct device_attribute *hpsa_shost_attrs[] = {
> >  	&dev_attr_firmware_revision,
> >  	&dev_attr_commands_outstanding,
> >  	&dev_attr_transport_mode,
> > +	&dev_attr_resettable_on_kexec,
> >  	NULL,
> >  };
> >  
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >   

Re: [PATCH 2/2] hpsa: export resettable_on_kexec host attribute

[scameron]

On Wed, Mar 09, 2011 at 02:33:39PM +0800, Américo Wang wrote:
> On Tue, Mar 08, 2011 at 05:10:03PM -0600, Stephen M. Cameron wrote:
> >From: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> >
> >This attribute, requested by Redhat, allows kexec-tools to know
> >whether the controller can honor the reset_devices kernel parameter
> >and actually reset the controller.  For kdump to work properly it
> >is necessary that the reset_devices parameter be honored.  This
> >attribute enables kexec-tools to warn the user if they attempt to
> >designate a non-resettable controller as the dump device.
> >
> >Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> 
> Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com>
> 
> Maybe we should docment this sysfs attribute somewhere?

Yes, thanks.  

Perhaps in Documentation/ABI/testing/sysfs-bus-scsi-devices-hpsa
(which doesn't exist) and/or in Documentation/scsi/hpsa.txt (which does exist, and
is where other hpsa specific sysfs attributes are documented.)  Probably so
as not to duplicate things, the latter.  Or maybe what documentation of
sysfs things that is currently in hpsa.txt should be moved to Documentation/ABI/...
and a reference to it put in hpsa.txt?

-- steve
 

Re: [PATCH 2/2] hpsa: export resettable_on_kexec host attribute

[Vivek Goyal]

On Tue, Mar 08, 2011 at 05:10:03PM -0600, Stephen M. Cameron wrote:
> From: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> 
> This attribute, requested by Redhat, allows kexec-tools to know
> whether the controller can honor the reset_devices kernel parameter
> and actually reset the controller.  For kdump to work properly it
> is necessary that the reset_devices parameter be honored.  This
> attribute enables kexec-tools to warn the user if they attempt to
> designate a non-resettable controller as the dump device.
> 

Hi Stephen,

Thinking more about it, can we make "resettable_on_kexec" name even more
generic. Kexec/kdump is just one of the users of this functionality.

So the idea is whether device is soft resettable or not by the driver.
May be "soft_resettable" or just "resettable" might be a better name.

Thanks
Vivek

> Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> ---
>  drivers/scsi/hpsa.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index dcabef4..1d4fe80 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -273,6 +273,44 @@ static ssize_t host_show_transport_mode(struct device *dev,
>  			"performant" : "simple");
>  }
>  
> +/* List of controllers which cannot be reset on kexec with reset_devices */
> +static u32 unresettable_controller[] = {
> +	0x324a103C, /* Smart Array P712m */
> +	0x324b103C, /* SmartArray P711m */
> +	0x3223103C, /* Smart Array P800 */
> +	0x3234103C, /* Smart Array P400 */
> +	0x3235103C, /* Smart Array P400i */
> +	0x3211103C, /* Smart Array E200i */
> +	0x3212103C, /* Smart Array E200 */
> +	0x3213103C, /* Smart Array E200i */
> +	0x3214103C, /* Smart Array E200i */
> +	0x3215103C, /* Smart Array E200i */
> +	0x3237103C, /* Smart Array E500 */
> +	0x3223103C, /* Smart Array P800 */
> +	0x3234103C, /* Smart Array P400 */
> +	0x323D103C, /* Smart Array P700m */
> +};
> +
> +static int resettable_on_kexec(struct ctlr_info *h)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(unresettable_controller); i++)
> +		if (unresettable_controller[i] == h->board_id)
> +			return 0;
> +	return 1;
> +}
> +
> +static ssize_t host_show_resettable_on_kexec(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct ctlr_info *h;
> +	struct Scsi_Host *shost = class_to_shost(dev);
> +
> +	h = shost_to_hba(shost);
> +	return snprintf(buf, 20, "%d\n", resettable_on_kexec(h));
> +}
> +
>  static inline int is_logical_dev_addr_mode(unsigned char scsi3addr[])
>  {
>  	return (scsi3addr[3] & 0xC0) == 0x40;
> @@ -379,6 +417,8 @@ static DEVICE_ATTR(commands_outstanding, S_IRUGO,
>  	host_show_commands_outstanding, NULL);
>  static DEVICE_ATTR(transport_mode, S_IRUGO,
>  	host_show_transport_mode, NULL);
> +static DEVICE_ATTR(resettable_on_kexec, S_IRUGO,
> +	host_show_resettable_on_kexec, NULL);
>  
>  static struct device_attribute *hpsa_sdev_attrs[] = {
>  	&dev_attr_raid_level,
> @@ -392,6 +432,7 @@ static struct device_attribute *hpsa_shost_attrs[] = {
>  	&dev_attr_firmware_revision,
>  	&dev_attr_commands_outstanding,
>  	&dev_attr_transport_mode,
> +	&dev_attr_resettable_on_kexec,
>  	NULL,
>  };
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] hpsa: export resettable_on_kexec host attribute

[scameron]

On Wed, Mar 09, 2011 at 09:36:24AM -0500, Vivek Goyal wrote:
> On Tue, Mar 08, 2011 at 05:10:03PM -0600, Stephen M. Cameron wrote:
> > From: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> > 
> > This attribute, requested by Redhat, allows kexec-tools to know
> > whether the controller can honor the reset_devices kernel parameter
> > and actually reset the controller.  For kdump to work properly it
> > is necessary that the reset_devices parameter be honored.  This
> > attribute enables kexec-tools to warn the user if they attempt to
> > designate a non-resettable controller as the dump device.
> > 
> 
> Hi Stephen,
> 
> Thinking more about it, can we make "resettable_on_kexec" name even more
> generic. Kexec/kdump is just one of the users of this functionality.
> 
> So the idea is whether device is soft resettable or not by the driver.
> May be "soft_resettable" or just "resettable" might be a better name.

Sure.  "resettable" seems fine.

-- steve

> 
> Thanks
> Vivek
> 
> > Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> > ---
> >  drivers/scsi/hpsa.c |   41 +++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 41 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> > index dcabef4..1d4fe80 100644
> > --- a/drivers/scsi/hpsa.c
> > +++ b/drivers/scsi/hpsa.c
> > @@ -273,6 +273,44 @@ static ssize_t host_show_transport_mode(struct device *dev,
> >  			"performant" : "simple");
> >  }
> >  
> > +/* List of controllers which cannot be reset on kexec with reset_devices */
> > +static u32 unresettable_controller[] = {
> > +	0x324a103C, /* Smart Array P712m */
> > +	0x324b103C, /* SmartArray P711m */
> > +	0x3223103C, /* Smart Array P800 */
> > +	0x3234103C, /* Smart Array P400 */
> > +	0x3235103C, /* Smart Array P400i */
> > +	0x3211103C, /* Smart Array E200i */
> > +	0x3212103C, /* Smart Array E200 */
> > +	0x3213103C, /* Smart Array E200i */
> > +	0x3214103C, /* Smart Array E200i */
> > +	0x3215103C, /* Smart Array E200i */
> > +	0x3237103C, /* Smart Array E500 */
> > +	0x3223103C, /* Smart Array P800 */
> > +	0x3234103C, /* Smart Array P400 */
> > +	0x323D103C, /* Smart Array P700m */
> > +};
> > +
> > +static int resettable_on_kexec(struct ctlr_info *h)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(unresettable_controller); i++)
> > +		if (unresettable_controller[i] == h->board_id)
> > +			return 0;
> > +	return 1;
> > +}
> > +
> > +static ssize_t host_show_resettable_on_kexec(struct device *dev,
> > +	struct device_attribute *attr, char *buf)
> > +{
> > +	struct ctlr_info *h;
> > +	struct Scsi_Host *shost = class_to_shost(dev);
> > +
> > +	h = shost_to_hba(shost);
> > +	return snprintf(buf, 20, "%d\n", resettable_on_kexec(h));
> > +}
> > +
> >  static inline int is_logical_dev_addr_mode(unsigned char scsi3addr[])
> >  {
> >  	return (scsi3addr[3] & 0xC0) == 0x40;
> > @@ -379,6 +417,8 @@ static DEVICE_ATTR(commands_outstanding, S_IRUGO,
> >  	host_show_commands_outstanding, NULL);
> >  static DEVICE_ATTR(transport_mode, S_IRUGO,
> >  	host_show_transport_mode, NULL);
> > +static DEVICE_ATTR(resettable_on_kexec, S_IRUGO,
> > +	host_show_resettable_on_kexec, NULL);
> >  
> >  static struct device_attribute *hpsa_sdev_attrs[] = {
> >  	&dev_attr_raid_level,
> > @@ -392,6 +432,7 @@ static struct device_attribute *hpsa_shost_attrs[] = {
> >  	&dev_attr_firmware_revision,
> >  	&dev_attr_commands_outstanding,
> >  	&dev_attr_transport_mode,
> > +	&dev_attr_resettable_on_kexec,
> >  	NULL,
> >  };
> >  
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] hpsa: export resettable_on_kexec host attribute

[scameron]

On Wed, Mar 09, 2011 at 01:27:53PM +0100, Tomas Henzl wrote:
> On 03/09/2011 12:10 AM, Stephen M. Cameron wrote:
> > From: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> >
> > This attribute, requested by Redhat, allows kexec-tools to know
> > whether the controller can honor the reset_devices kernel parameter
>

[...]

> and actually reset the controller.  For kdump to work properly it
> Hi Stephen,
> 
> thanks for posting this.
> Some of the devices are served by the cciss driver by default - I guess
> a very similar patch for cciss is needed too.
> Shouldn't be the 0x409C0E11 and 0x409D0E11 (640x boards) also added to the list?
> (And the 'unknown' devices.)

There's a bit of a fine point here regarding the unknown devices.

If hpsa_allow_any=1 module parameter is set, then the unknown device
is considered to be resettable (as it's unknown, it's obviously not
on the list of known unresettable controllers).  If hpsa_allow_any
is not set, then the unknown devices are not reset -- and the driver
doesn't even try to do anything with them.  

So, the patch is consistent with this, in that if hpsa_allow_any is
not set, then there won't be any corresponding sysfs entries at all
for those devices because those devices won't be service by hpsa
at all.  And if hpsa_allow_any is set, then those devices will be
marked as resettable, and the reset code will attempt to reset them.

I think we've got all the unresettable devices listed (when I add the 6400
boards to the list of course) and I think we're going to try pretty hard to
make sure new boards are resettable, so, that's probably ok, right?

Or, do you want to be extra safe, and say that new, unknown boards are assumed
to be non-resettable?  (Since new boards generally mean driver changes to make
sure the driver knows those boards, that's not such a big deal -- except for
people who want to continue to use old OSes on new hardware, which, there seem
to be quite a few of those people.)

I think my preference would be to assume that unknown boards are resettable
if hpsa_allow_any=1, and assume unresettable otherwise (and for purposes of
sysfs attributes, this is what the patch already does.)

-- steve

Re: [PATCH 2/2] hpsa: export resettable_on_kexec host attribute

[Tomas Henzl]

On 03/09/2011 04:14 PM, scameron@beardog.cce.hp.com wrote:
> On Wed, Mar 09, 2011 at 01:27:53PM +0100, Tomas Henzl wrote:
>   
>> On 03/09/2011 12:10 AM, Stephen M. Cameron wrote:
>>     
>>> From: Stephen M. Cameron <scameron@beardog.cce.hp.com>
>>>
>>> This attribute, requested by Redhat, allows kexec-tools to know
>>> whether the controller can honor the reset_devices kernel parameter
>>>       
>>     
> [...]
>
>   
>> and actually reset the controller.  For kdump to work properly it
>> Hi Stephen,
>>
>> thanks for posting this.
>> Some of the devices are served by the cciss driver by default - I guess
>> a very similar patch for cciss is needed too.
>> Shouldn't be the 0x409C0E11 and 0x409D0E11 (640x boards) also added to the list?
>> (And the 'unknown' devices.)
>>     
> There's a bit of a fine point here regarding the unknown devices.
>
> If hpsa_allow_any=1 module parameter is set, then the unknown device
> is considered to be resettable (as it's unknown, it's obviously not
> on the list of known unresettable controllers).  If hpsa_allow_any
> is not set, then the unknown devices are not reset -- and the driver
> doesn't even try to do anything with them.  
>
> So, the patch is consistent with this, in that if hpsa_allow_any is
> not set, then there won't be any corresponding sysfs entries at all
> for those devices because those devices won't be service by hpsa
> at all.  And if hpsa_allow_any is set, then those devices will be
> marked as resettable, and the reset code will attempt to reset them.
>
> I think we've got all the unresettable devices listed (when I add the 6400
> boards to the list of course) and I think we're going to try pretty hard to
> make sure new boards are resettable, so, that's probably ok, right?
>   
> Or, do you want to be extra safe, and say that new, unknown boards are assumed
> to be non-resettable?  (Since new boards generally mean driver changes to make
> sure the driver knows those boards, that's not such a big deal -- except for
> people who want to continue to use old OSes on new hardware, which, there seem
> to be quite a few of those people.)
>   
My comment has targeted the new unknown boards, to resolve this it would be easier
to have a list of resettable controllers. (complement to what it is now).

Fact is, that I forgot that a hpsa_allow_any option has to be set before you can
use an 'unknown' controller and combined with your promise 
> we're going to try pretty hard to make sure new boards are resettable
I'm fine with the original approach.

-- tomash


> I think my preference would be to assume that unknown boards are resettable
> if hpsa_allow_any=1, and assume unresettable otherwise (and for purposes of
> sysfs attributes, this is what the patch already does.)
>
> -- steve
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>