All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Jesper Juhl <jj@chaosbits.net>
Cc: linux-scsi@vger.kernel.org,
	"James E.J. Bottomley" <James.Bottomley@suse.de>,
	linux-kernel@vger.kernel.org, Eric Youngdale <eric@andante.org>,
	"David S. Miller" <davem@davemloft.net>,
	Mike Anderson <andmike@us.ibm.com>,
	Russell King <rmk@arm.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH][resend][SCSI] Reduce number of sequential pointer derefs in scsi_error.c and reduce size as well
Date: Thu, 18 Nov 2010 13:18:58 -0800	[thread overview]
Message-ID: <AANLkTim1MChiEa9rBL0ZfSCak6G2x4ivK47Svd6swEBS@mail.gmail.com> (raw)
In-Reply-To: <alpine.LNX.2.00.1011182140510.8526@swampdragon.chaosbits.net>

On Thu, Nov 18, 2010 at 12:49 PM, Jesper Juhl <jj@chaosbits.net> wrote:
>
> Fair enough. Seeing your version of this and looking a second time at mine
> I have to agree completely.
> You are absolutely right in stating that the compiler will handle this
> just fine, so it's only a readabillity issue and your version is a *lot*
> more readable than what I came up with.

Btw, one thing to look out for is all those function calls: it really
looks like many of them would be better off not having to dereference
the thing inside each helper function, but just have it dereferenced
in the caller.

You might trying passing in "struct Scsi_Host *host" to a lot of those
helper functions in addition to the 'scmd' part. There's a lot of
"scmd->device->host" going on, and even if you remove some of them
_within_ a function, if you really want to get rid of them you should
probably do one of them in the caller.

That's why the queuecommand() function was changed to take

    struct Scsi_Host *h, struct scsi_cmnd *

as its arguments, because that host is used so commonly. And passing
two arguments is usually free (ie almost all architectures pass it in
registers), and that host variable almost always already exists in the
caller because the caller already needed it.

So if you changed the functions that only take "scsi_cmnd *" as an
argument to match the new queuecommand() interface, I bet you'd get
more cleanups. And the interfaces would match.

              Linus

  parent reply	other threads:[~2010-11-18 21:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-18 19:30 [PATCH][resend][SCSI] Reduce number of sequential pointer derefs in scsi_error.c and reduce size as well Jesper Juhl
2010-11-18 20:41 ` Linus Torvalds
2010-11-18 20:49   ` Jesper Juhl
2010-11-18 20:57     ` Jesper Juhl
2010-11-18 21:06     ` Russell King
2010-11-18 20:59       ` Jesper Juhl
2010-11-18 21:18     ` Linus Torvalds [this message]
2010-11-18 21:21       ` Jesper Juhl
2010-11-20 20:30       ` Jesper Juhl
2010-11-21  0:03         ` Linus Torvalds
2010-11-21 18:48           ` Jesper Juhl
2010-12-21 17:37             ` James Bottomley
2010-12-22 20:23               ` Jesper Juhl

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=AANLkTim1MChiEa9rBL0ZfSCak6G2x4ivK47Svd6swEBS@mail.gmail.com \
    --to=torvalds@linux-foundation.org \
    --cc=James.Bottomley@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=andmike@us.ibm.com \
    --cc=davem@davemloft.net \
    --cc=eric@andante.org \
    --cc=jj@chaosbits.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=rmk@arm.linux.org.uk \
    /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.