public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: jens.axboe@oracle.com
Cc: fujita.tomonori@lab.ntt.co.jp, linux-kernel@vger.kernel.org,
	linux-scsi@vger.kernel.org
Subject: Re: [PATCH] SPARC64: fix iommu sg chaining
Date: Wed, 17 Oct 2007 02:13:05 -0700 (PDT)	[thread overview]
Message-ID: <20071017.021305.74746838.davem@davemloft.net> (raw)
In-Reply-To: <20071017084528.GI5043@kernel.dk>

From: Jens Axboe <jens.axboe@oracle.com>
Date: Wed, 17 Oct 2007 10:45:28 +0200

> Righto, it's invalid to call sg_next() on the last entry!

Unfortunately, that's what the sparc64 code wanted to do, this
transformation in the sparc64 sg chaining patch is not equilavent:

-	struct scatterlist *sg_end = sg + nelems;
+	struct scatterlist *sg_end = sg_last(sg, nelems);
 ...
-			while (sg < sg_end &&
+			while (sg != sg_end &&

No, that's not what the code was doing.  The while loop
has to process the last entry in the list,

We really needed "sg_end" to be "one past the last element",
rather than "the last element".

Since you say that sg_next() on the last entry is illegal,
and that's what this code would have done to try and reach
loop termination (it doesn't actually derefrence that
"end plus one" scatterlist entry) I'll try to code this up
some other way.

Besides, sg_last() is so absurdly expensive, it has to walk the entire
chain in the chaining case.  So better to implement this without it.

I would suggest that other sg_last() uses be audited for the same bug.

  reply	other threads:[~2007-10-17  9:13 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-17  5:07 [PATCH] SPARC64: fix iommu sg chaining FUJITA Tomonori
2007-10-17  7:21 ` Jens Axboe
2007-10-17  8:33   ` David Miller
2007-10-17  8:42     ` David Miller
2007-10-17  8:45       ` Jens Axboe
2007-10-17  9:13         ` David Miller [this message]
2007-10-17  9:16           ` Jens Axboe
2007-10-17  9:24             ` FUJITA Tomonori
2007-10-17  9:27               ` Jens Axboe
2007-10-17  9:45               ` David Miller
2007-10-17 11:08                 ` FUJITA Tomonori
2007-10-17 11:13                   ` Jens Axboe
2007-10-17 11:16                     ` Jens Axboe
2007-10-17 10:54             ` David Miller
2007-10-17 10:58               ` Jens Axboe
2007-10-17 11:01                 ` Jens Axboe
2007-10-17 11:10                   ` David Miller
2007-10-17 11:11                     ` Jens Axboe
2007-10-17 11:18                       ` David Miller
2007-10-17 11:27                         ` Jens Axboe
2007-10-17 11:37                   ` FUJITA Tomonori
2007-10-17 11:41                     ` Jens Axboe
2007-10-17 11:57                       ` FUJITA Tomonori
2007-10-17 12:05                         ` FUJITA Tomonori
2007-10-17 14:36                           ` Jens Axboe
2007-10-17 23:01                     ` FUJITA Tomonori
2007-10-17 11:04                 ` David Miller
2007-10-17 11:04                   ` Jens Axboe
2007-10-17 11:04                 ` FUJITA Tomonori

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=20071017.021305.74746838.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    /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