From: Tomas Henzl <thenzl@redhat.com>
To: Don Brace <don.brace@pmcs.com>,
scott.teel@pmcs.com, Kevin.Barnett@pmcs.com,
scott.benesh@pmcs.com, james.bottomley@parallels.com,
hch@infradead.org, Justin.Lindley@pmcs.com, elliott@hpe.com
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH 01/21] hpsa: correct calls to dev_printk
Date: Mon, 26 Oct 2015 13:52:24 +0100 [thread overview]
Message-ID: <562E2208.5060200@redhat.com> (raw)
In-Reply-To: <20151024195232.17127.59627.stgit@brunhilda>
On 24.10.2015 21:52, Don Brace wrote:
> was seeing issues passing level into hpsa_show_dev_msg and then using
> the level as an arguement to dev_printk. For now, switch to dev_warn.
>
> Signed-off-by: Don Brace <don.brace@pmcs.com>
> ---
> drivers/scsi/hpsa.c | 41 ++++++++++++++++++++++++-----------------
> 1 file changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 40669f8..8454f22 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -1137,11 +1137,15 @@ static int hpsa_find_target_lun(struct ctlr_info *h,
> static inline void hpsa_show_dev_msg(const char *level, struct ctlr_info *h,
> struct hpsa_scsi_dev_t *dev, char *description)
> {
> - dev_printk(level, &h->pdev->dev,
> + if (dev == NULL)
> + return;
> +
> + dev_warn(&h->pdev->dev,
> "scsi %d:%d:%d:%d: %s %s %.8s %.16s RAID-%s SSDSmartPathCap%c En%c Exp=%d\n",
> h->scsi_host->host_no, dev->bus, dev->target, dev->lun,
> description,
> - scsi_device_type(dev->devtype),
> + dev->devtype >= 0 ?
> + scsi_device_type(dev->devtype) : "unknown",
Can devtype ever be negative? Found only this place in code
where it is set "this_device->devtype = (inq_buff[0] & 0x1f);"
And if it was negative the scsi_device_type will return "Unknown"
anyway. You probably could skip the test now and switch devtype
from int to unsigned in the future.
Cheers,
Tomas
> dev->vendor,
> dev->model,
> dev->raid_level > RAID_UNKNOWN ?
> @@ -1220,7 +1224,7 @@ lun_assigned:
> h->ndevices++;
> added[*nadded] = device;
> (*nadded)++;
> - hpsa_show_dev_msg(KERN_INFO, h, device,
> + hpsa_show_dev_msg(__stringify(KERN_INFO), h, device,
> device->expose_state & HPSA_SCSI_ADD ? "added" : "masked");
> device->offload_to_be_enabled = device->offload_enabled;
> device->offload_enabled = 0;
> @@ -1271,7 +1275,7 @@ static void hpsa_scsi_update_entry(struct ctlr_info *h, int hostno,
>
> offload_enabled = h->dev[entry]->offload_enabled;
> h->dev[entry]->offload_enabled = h->dev[entry]->offload_to_be_enabled;
> - hpsa_show_dev_msg(KERN_INFO, h, h->dev[entry], "updated");
> + hpsa_show_dev_msg(__stringify(KERN_INFO), h, h->dev[entry], "updated");
> h->dev[entry]->offload_enabled = offload_enabled;
> }
>
> @@ -1298,7 +1302,7 @@ static void hpsa_scsi_replace_entry(struct ctlr_info *h, int hostno,
> h->dev[entry] = new_entry;
> added[*nadded] = new_entry;
> (*nadded)++;
> - hpsa_show_dev_msg(KERN_INFO, h, new_entry, "replaced");
> + hpsa_show_dev_msg(__stringify(KERN_INFO), h, new_entry, "replaced");
> new_entry->offload_to_be_enabled = new_entry->offload_enabled;
> new_entry->offload_enabled = 0;
> }
> @@ -1320,7 +1324,7 @@ static void hpsa_scsi_remove_entry(struct ctlr_info *h, int hostno, int entry,
> for (i = entry; i < h->ndevices-1; i++)
> h->dev[i] = h->dev[i+1];
> h->ndevices--;
> - hpsa_show_dev_msg(KERN_INFO, h, sd, "removed");
> + hpsa_show_dev_msg(__stringify(KERN_INFO), h, sd, "removed");
> }
>
> #define SCSI3ADDR_EQ(a, b) ( \
> @@ -1710,7 +1714,8 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
> */
> if (sd[i]->volume_offline) {
> hpsa_show_volume_status(h, sd[i]);
> - hpsa_show_dev_msg(KERN_INFO, h, sd[i], "offline");
> + hpsa_show_dev_msg(__stringify(KERN_INFO), h, sd[i],
> + "offline");
> continue;
> }
>
> @@ -1774,7 +1779,8 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
> * future cmds to this device will get selection
> * timeout as if the device was gone.
> */
> - hpsa_show_dev_msg(KERN_WARNING, h, removed[i],
> + hpsa_show_dev_msg(__stringify(KERN_WARNING),
> + h, removed[i],
> "didn't find device for removal.");
> }
> }
> @@ -1789,7 +1795,7 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
> if (scsi_add_device(sh, added[i]->bus,
> added[i]->target, added[i]->lun) == 0)
> continue;
> - hpsa_show_dev_msg(KERN_WARNING, h, added[i],
> + hpsa_show_dev_msg(__stringify(KERN_WARNING), h, added[i],
> "addition failed, device not added.");
> /* now we have to remove it from h->dev,
> * since it didn't get added to scsi mid layer
> @@ -5188,7 +5194,7 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
> snprintf(msg, sizeof(msg),
> "cmd %d RESET FAILED, lockup detected",
> hpsa_get_cmd_index(scsicmd));
> - hpsa_show_dev_msg(KERN_WARNING, h, dev, msg);
> + hpsa_show_dev_msg(__stringify(KERN_WARNING), h, dev, msg);
> return FAILED;
> }
>
> @@ -5197,7 +5203,7 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
> snprintf(msg, sizeof(msg),
> "cmd %d RESET FAILED, new lockup detected",
> hpsa_get_cmd_index(scsicmd));
> - hpsa_show_dev_msg(KERN_WARNING, h, dev, msg);
> + hpsa_show_dev_msg(__stringify(KERN_WARNING), h, dev, msg);
> return FAILED;
> }
>
> @@ -5205,14 +5211,14 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
> if (is_hba_lunid(dev->scsi3addr))
> return SUCCESS;
>
> - hpsa_show_dev_msg(KERN_WARNING, h, dev, "resetting");
> + hpsa_show_dev_msg(__stringify(KERN_WARNING), h, dev, "resetting");
>
> /* send a reset to the SCSI LUN which the command was sent to */
> rc = hpsa_do_reset(h, dev, dev->scsi3addr, HPSA_RESET_TYPE_LUN,
> DEFAULT_REPLY_QUEUE);
> snprintf(msg, sizeof(msg), "reset %s",
> rc == 0 ? "completed successfully" : "failed");
> - hpsa_show_dev_msg(KERN_WARNING, h, dev, msg);
> + hpsa_show_dev_msg(__stringify(KERN_WARNING), h, dev, msg);
> return rc == 0 ? SUCCESS : FAILED;
> }
>
> @@ -5548,14 +5554,14 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc)
>
> /* If controller locked up, we can guarantee command won't complete */
> if (lockup_detected(h)) {
> - hpsa_show_dev_msg(KERN_WARNING, h, dev,
> + hpsa_show_dev_msg(__stringify(KERN_WARNING), h, dev,
> "ABORT FAILED, lockup detected");
> return FAILED;
> }
>
> /* This is a good time to check if controller lockup has occurred */
> if (detect_controller_lockup(h)) {
> - hpsa_show_dev_msg(KERN_WARNING, h, dev,
> + hpsa_show_dev_msg(__stringify(KERN_WARNING), h, dev,
> "ABORT FAILED, new lockup detected");
> return FAILED;
> }
> @@ -5610,7 +5616,8 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc)
> as->cmd_len, as->cmnd[0], as->cmnd[1],
> as->serial_number);
> dev_warn(&h->pdev->dev, "%s BEING SENT\n", msg);
> - hpsa_show_dev_msg(KERN_WARNING, h, dev, "Aborting command");
> + hpsa_show_dev_msg(__stringify(KERN_WARNING), h, dev,
> + "Aborting command");
>
> /*
> * Command is in flight, or possibly already completed
> @@ -5629,7 +5636,7 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc)
> wake_up_all(&h->abort_cmd_wait_queue);
> if (rc != 0) {
> dev_warn(&h->pdev->dev, "%s SENT, FAILED\n", msg);
> - hpsa_show_dev_msg(KERN_WARNING, h, dev,
> + hpsa_show_dev_msg(__stringify(KERN_WARNING), h, dev,
> "FAILED to abort command");
> cmd_free(h, abort);
> return FAILED;
>
> --
> 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
next prev parent reply other threads:[~2015-10-26 12:52 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-24 19:52 [PATCH 00/21] hpsa updates Don Brace
2015-10-24 19:52 ` [PATCH 01/21] hpsa: correct calls to dev_printk Don Brace
2015-10-26 12:52 ` Tomas Henzl [this message]
2015-10-26 13:41 ` Don Brace
2015-10-24 19:52 ` [PATCH 02/21] hpsa: allow driver requested rescans Don Brace
2015-10-26 14:36 ` Tomas Henzl
2015-10-24 19:52 ` [PATCH 03/21] hpsa: abandon rescans on memory alloaction failures Don Brace
2015-10-26 14:24 ` Tomas Henzl
2015-10-26 14:29 ` Don Brace
2015-10-26 14:32 ` Tomas Henzl
2015-10-26 14:36 ` Tomas Henzl
2015-10-24 19:52 ` [PATCH 04/21] hpsa: correct transfer length for 6 byte read/write commands Don Brace
2015-10-26 14:37 ` Tomas Henzl
2015-10-24 19:52 ` [PATCH 05/21] hpsa: fix hpsa_adjust_hpsa_scsi_table Don Brace
2015-10-26 14:55 ` Tomas Henzl
2015-10-26 15:22 ` Don Brace
2015-10-24 19:53 ` [PATCH 06/21] hpsa: fix physical target reset Don Brace
2015-10-24 19:53 ` [PATCH 07/21] hpsa: correct check for non-disk devices Don Brace
2015-10-24 19:53 ` [PATCH 08/21] hpsa: correct ioaccel2 sg chain len Don Brace
2015-10-24 19:53 ` [PATCH 09/21] hpsa: simplify check for device exposure Don Brace
2015-10-24 19:53 ` [PATCH 10/21] hpsa: simplify update scsi devices Don Brace
2015-10-24 19:53 ` [PATCH 11/21] hpsa: add function is_logical_device Don Brace
2015-10-24 19:53 ` [PATCH 12/21] hpsa: enhance hpsa_get_device_id Don Brace
2015-10-24 19:53 ` [PATCH 13/21] hpsa: refactor hpsa_figure_bus_target_lun Don Brace
2015-10-24 19:53 ` [PATCH 14/21] hpsa: move scsi_add_device and scsi_remove_device calls to new function Don Brace
2015-10-24 19:53 ` [PATCH 15/21] External array LUNs must use target and lun numbers assigned by the Don Brace
2015-10-24 19:54 ` [PATCH 16/21] hpsa: eliminate fake lun0 enclosures Don Brace
2015-10-24 19:54 ` [PATCH 17/21] hpsa: add discovery polling for PT RAID devices Don Brace
2015-10-24 19:54 ` [PATCH 18/21] hpsa: disable report lun data caching Don Brace
2015-10-24 19:54 ` [PATCH 19/21] hpsa: enhance device messages Don Brace
2015-10-24 20:19 ` kbuild test robot
2015-10-26 13:36 ` Don Brace
2015-10-24 19:54 ` [PATCH 20/21] hpsa: add in sas transport class Don Brace
2015-10-24 19:54 ` [PATCH 21/21] hpsa: bump the driver version Don Brace
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=562E2208.5060200@redhat.com \
--to=thenzl@redhat.com \
--cc=Justin.Lindley@pmcs.com \
--cc=Kevin.Barnett@pmcs.com \
--cc=don.brace@pmcs.com \
--cc=elliott@hpe.com \
--cc=hch@infradead.org \
--cc=james.bottomley@parallels.com \
--cc=linux-scsi@vger.kernel.org \
--cc=scott.benesh@pmcs.com \
--cc=scott.teel@pmcs.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).