From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756033AbbLDJw4 (ORCPT ); Fri, 4 Dec 2015 04:52:56 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36918 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752123AbbLDJwx (ORCPT ); Fri, 4 Dec 2015 04:52:53 -0500 From: Vitaly Kuznetsov To: Long Li Cc: "K. Y. Srinivasan" , Haiyang Zhang , "James E.J. Bottomley" , devel@linuxdriverproject.org, linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org Subject: Re: [PATCH] storvsc: add more logging for error and warning messages References: <1449200865-20694-1-git-send-email-longli@microsoft.com> Date: Fri, 04 Dec 2015 10:52:49 +0100 In-Reply-To: <1449200865-20694-1-git-send-email-longli@microsoft.com> (Long Li's message of "Thu, 3 Dec 2015 19:47:45 -0800") Message-ID: <87vb8e5ymm.fsf@vitty.brq.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Long Li writes: > Introduce a logging level for storvsc to log certain error/warning > messages. Those messages are helpful in some environments, > e.g. Microsoft Azure, for customer support and troubleshooting > purposes. I have an alternative suggestion: let's use dynamic debug! Basically, we need to convert all non-error logging to using dev_dbg() and this can be enabled dynamically when needed, even reboot won't be required. > > Signed-off-by: Long Li > --- > drivers/scsi/storvsc_drv.c | 30 +++++++++++++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > index 40c43ae..afa1647 100644 > --- a/drivers/scsi/storvsc_drv.c > +++ b/drivers/scsi/storvsc_drv.c > @@ -164,6 +164,21 @@ static int sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE; > */ > static int vmstor_proto_version; > > +#define STORVSC_LOGGING_NONE 0 > +#define STORVSC_LOGGING_ERROR 1 > +#define STORVSC_LOGGING_WARN 2 > + > +static int logging_level = STORVSC_LOGGING_ERROR; > +module_param(logging_level, int, S_IRUGO|S_IWUSR); > +MODULE_PARM_DESC(logging_level, > + "Logging level, 0 - None, 1 - Error (default), 2 - Warning."); > + > +static inline bool do_logging(int level) > +{ > + return (logging_level >= level) ? true : false; > +} > + > + > struct vmscsi_win8_extension { > /* > * The following were added in Windows 8 > @@ -1183,7 +1198,7 @@ static void storvsc_command_completion(struct storvsc_cmd_request *cmd_request) > > scmnd->result = vm_srb->scsi_status; > > - if (scmnd->result) { > + if (scmnd->result && do_logging(STORVSC_LOGGING_ERROR)) { > if (scsi_normalize_sense(scmnd->sense_buffer, > SCSI_SENSE_BUFFERSIZE, &sense_hdr)) > scsi_print_sense_hdr(scmnd->device, "storvsc", > @@ -1239,12 +1254,25 @@ static void storvsc_on_io_completion(struct hv_device *device, > stor_pkt->vm_srb.sense_info_length = > vstor_packet->vm_srb.sense_info_length; > > + if (vstor_packet->vm_srb.scsi_status != 0 || > + vstor_packet->vm_srb.srb_status != SRB_STATUS_SUCCESS) > + if (do_logging(STORVSC_LOGGING_WARN)) > + dev_warn(&device->device, > + "cmd 0x%x scsi status 0x%x srb status 0x%x\n", > + stor_pkt->vm_srb.cdb[0], > + vstor_packet->vm_srb.scsi_status, > + vstor_packet->vm_srb.srb_status); > > if ((vstor_packet->vm_srb.scsi_status & 0xFF) == 0x02) { > /* CHECK_CONDITION */ > if (vstor_packet->vm_srb.srb_status & > SRB_STATUS_AUTOSENSE_VALID) { > /* autosense data available */ > + if (do_logging(STORVSC_LOGGING_WARN)) > + dev_warn(&device->device, > + "stor pkt %p autosense data valid - len %d\n", > + request, > + vstor_packet->vm_srb.sense_info_length); > > memcpy(request->cmd->sense_buffer, > vstor_packet->vm_srb.sense_data, -- Vitaly