All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <Niklas.Cassel@wdc.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: "Martin K . Petersen" <martin.petersen@oracle.com>,
	Jaegeuk Kim <jaegeuk@kernel.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>, Ming Lei <ming.lei@redhat.com>,
	Hannes Reinecke <hare@suse.de>,
	John Garry <john.g.garry@oracle.com>,
	Mike Christie <michael.christie@oracle.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Vishakha Channapattan <vishakhavc@google.com>,
	Jolly Shah <jollys@google.com>,
	Changyuan Lyu <changyuanl@google.com>,
	Damien Le Moal <dlemoal@kernel.org>
Subject: Re: [PATCH 4/4] scsi: Trace SCSI sense data
Date: Wed, 3 May 2023 07:16:21 +0000	[thread overview]
Message-ID: <ZFIKRFICCJv5AtKj@x1-carbon> (raw)
In-Reply-To: <1232c27c-4da6-a738-c138-b0e65fa74467@acm.org>

On Tue, May 02, 2023 at 01:31:10PM -0700, Bart Van Assche wrote:
> On 5/2/23 02:52, Niklas Cassel wrote:
> > i.e., something like:
> > 	if (cmd->sense_buffer && SCSI_SENSE_VALID(cmd) &&
> > 	    scsi_command_normalize_sense(cmd, &sshdr)) {
> > 
> > instead of cmd->result & 0xff.
> 
> Hmm ... doesn't the SCSI_SENSE_VALID() check above duplicate the
> scsi_sense_valid() check inside scsi_command_normalize_sense()?

scsi_command_normalize_sense() calls scsi_normalize_sense(),
so that is a function call.

scsi_normalize_sense() then performs a memset(),
a null pointer check on sense_buffer, and length check on sb_len,
then assigns sshdr->response_code, then calls scsi_sense_valid(),
and returns if not valid.


Considering that this tracepoint is called by scsi_done_internal(),
this tracepoint will be called for all SCSI commands (and not just
timed out commands).

I'm not sure how much overhead the function call, memset, and
assignment is, but I would guess that it is not free, and since
this is done for all commands, I would assume that we waste less
CPU cycles by simply checking SCSI_SENSE_VALID() beforehand.


Kind regards,
Niklas

      reply	other threads:[~2023-05-03  7:16 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-25 23:34 [PATCH 0/4] SCSI core patches Bart Van Assche
2023-04-25 23:34 ` [PATCH 1/4] scsi: core: Use min() instead of open-coding it Bart Van Assche
2023-04-25 23:34 ` [PATCH 2/4] scsi: core: Update a source code comment Bart Van Assche
2023-04-28  5:38   ` Christoph Hellwig
2023-04-28 16:58     ` Bart Van Assche
2023-04-25 23:34 ` [PATCH 3/4] scsi: Only kick the requeue list if necessary Bart Van Assche
2023-04-26  3:23   ` kernel test robot
2023-04-26  5:16   ` kernel test robot
2023-04-25 23:34 ` [PATCH 4/4] scsi: Trace SCSI sense data Bart Van Assche
2023-04-25 23:52   ` Steven Rostedt
2023-04-28  8:09   ` Niklas Cassel
2023-04-28 18:36     ` Bart Van Assche
2023-05-02  9:52       ` Niklas Cassel
2023-05-02 20:31         ` Bart Van Assche
2023-05-03  7:16           ` Niklas Cassel [this message]

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=ZFIKRFICCJv5AtKj@x1-carbon \
    --to=niklas.cassel@wdc.com \
    --cc=bvanassche@acm.org \
    --cc=changyuanl@google.com \
    --cc=dlemoal@kernel.org \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=jaegeuk@kernel.org \
    --cc=john.g.garry@oracle.com \
    --cc=jollys@google.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mhiramat@kernel.org \
    --cc=michael.christie@oracle.com \
    --cc=ming.lei@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=vishakhavc@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.