io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: dgilbert@interlog.com, Christoph Hellwig <hch@infradead.org>,
	Pavel Begunkov <asml.silence@gmail.com>
Cc: Ming Lei <ming.lei@redhat.com>,
	linux-block@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
	Matthew Wilcox <willy@infradead.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	"Darrick J . Wong" <darrick.wong@oracle.com>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	io-uring@vger.kernel.org, linux-kernel@vger.kernel.org,
	target-devel@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH v1 0/6] no-copy bvec
Date: Thu, 24 Dec 2020 09:30:17 -0800	[thread overview]
Message-ID: <bdd002f433928dd545d336a982516afa4e095d49.camel@HansenPartnership.com> (raw)
In-Reply-To: <8abc56c2-4db8-5ee3-ab2d-8960d0eeeb0d@interlog.com>

On Wed, 2020-12-23 at 15:23 -0500, Douglas Gilbert wrote:
> On 2020-12-23 11:04 a.m., James Bottomley wrote:
> > On Wed, 2020-12-23 at 15:51 +0000, Christoph Hellwig wrote:
> > > On Wed, Dec 23, 2020 at 12:52:59PM +0000, Pavel Begunkov wrote:
> > > > Can scatterlist have 0-len entries? Those are directly
> > > > translated into bvecs, e.g. in nvme/target/io-cmd-file.c and
> > > > target/target_core_file.c. I've audited most of others by this
> > > > moment, they're fine.
> > > 
> > > For block layer SGLs we should never see them, and for nvme
> > > neither. I think the same is true for the SCSI target code, but
> > > please double check.
> > 
> > Right, no-one ever wants to see a 0-len scatter list entry.  The
> > reason is that every driver uses the sgl to program the device DMA
> > engine in the way NVME does.  a 0 length sgl would be a dangerous
> > corner case: some DMA engines would ignore it and others would go
> > haywire, so if we ever let a 0 length list down into the driver,
> > they'd have to understand the corner case behaviour of their DMA
> > engine and filter it accordingly, which is why we disallow them in
> > the upper levels, since they're effective nops anyway.
> 
> When using scatter gather lists at the far end (i.e. on the storage
> device) the T10 examples (WRITE SCATTERED and POPULATE TOKEN in SBC-
> 4) explicitly allow the "number of logical blocks" in their sgl_s to
> be zero and state that it is _not_ to be considered an error.

But that's pretty irrelevant.  The scatterlists that block has been
constructing to drive DMA engines pre-date SCSI's addition of SGLs by
decades (all SCSI commands before the object commands use a linear
buffer which is implemented in the HBA engine as a scatterlist but not
described by the SCSI standard as one).

So the answer to the question should the block layer emit zero length
sgl elements is "no" because they can confuse some DMA engines.

If there's a more theoretical question of whether the target driver in
adding commands it doesn't yet support should inject zero length SGL
elements into block because SCSI allows it, the answer is still "no"
because we don't want block to have SGLs that may confuse other DMA
engines.  There's lots of daft corner cases in the SCSI standard we
don't implement and a nop for SGL elements seems to be one of the more
hare brained because it adds no useful feature and merely causes
compatibility issues.

James
 


      parent reply	other threads:[~2020-12-24 17:31 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-15  0:20 [PATCH v1 0/6] no-copy bvec Pavel Begunkov
2020-12-15  0:20 ` [PATCH v1 1/6] target/file: allocate the bvec array as part of struct target_core_file_cmd Pavel Begunkov
2020-12-15  0:20 ` [PATCH v1 2/6] iov_iter: optimise bvec iov_iter_advance() Pavel Begunkov
2020-12-15  9:37   ` David Laight
2020-12-15 11:23     ` Pavel Begunkov
2020-12-15 13:54       ` David Laight
2020-12-15 13:56         ` Pavel Begunkov
2020-12-22 14:03   ` Christoph Hellwig
2020-12-15  0:20 ` [PATCH v1 3/6] bio: deduplicate adding a page into bio Pavel Begunkov
2020-12-22 14:04   ` Christoph Hellwig
2020-12-15  0:20 ` [PATCH v1 4/6] block/psi: remove PSI annotations from direct IO Pavel Begunkov
2020-12-15  0:56   ` Dave Chinner
2020-12-15  1:03     ` Pavel Begunkov
2020-12-15  1:33       ` Dave Chinner
2020-12-15 11:41         ` Pavel Begunkov
2020-12-22 14:07   ` Christoph Hellwig
2020-12-15  0:20 ` [PATCH v1 5/6] bio: add a helper calculating nr segments to alloc Pavel Begunkov
2020-12-15  1:00   ` Dave Chinner
2020-12-15  1:07     ` Pavel Begunkov
2020-12-15  1:09     ` Dave Chinner
2020-12-22 14:07   ` Christoph Hellwig
2020-12-15  0:20 ` [PATCH v1 6/6] block/iomap: don't copy bvec for direct IO Pavel Begunkov
2020-12-15  1:09   ` Dave Chinner
2020-12-15  1:15     ` Pavel Begunkov
2020-12-22 14:15   ` Christoph Hellwig
2020-12-15  1:41 ` [PATCH v1 0/6] no-copy bvec Ming Lei
2020-12-15 11:14   ` Pavel Begunkov
2020-12-15 12:03     ` Ming Lei
2020-12-15 14:05       ` Pavel Begunkov
2020-12-22 14:11         ` Christoph Hellwig
2020-12-23 12:52           ` Pavel Begunkov
2020-12-23 15:51             ` Christoph Hellwig
2020-12-23 16:04               ` James Bottomley
2020-12-23 20:23                 ` Douglas Gilbert
2020-12-23 20:32                   ` Pavel Begunkov
2020-12-24  6:41                     ` Christoph Hellwig
2020-12-24 16:45                       ` Douglas Gilbert
2020-12-24 17:30                   ` James Bottomley [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=bdd002f433928dd545d336a982516afa4e095d49.camel@HansenPartnership.com \
    --to=james.bottomley@hansenpartnership.com \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=corbet@lwn.net \
    --cc=darrick.wong@oracle.com \
    --cc=dgilbert@interlog.com \
    --cc=hannes@cmpxchg.org \
    --cc=hch@infradead.org \
    --cc=io-uring@vger.kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=ming.lei@redhat.com \
    --cc=target-devel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.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;
as well as URLs for NNTP newsgroup(s).