All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: Jens Axboe <axboe@kernel.dk>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	linux-kernel@vger.kernel.org,
	Wanlong Gao <gaowanlong@cn.fujitsu.com>,
	asias@redhat.com, mst@redhat.com, kvm@vger.kernel.org,
	virtualization@lists.linux-foundation.org
Subject: Re: [RFC PATCH 0/8] virtio: new API for addition of buffers, scatterlist changes
Date: Wed, 13 Feb 2013 20:16:42 +1030	[thread overview]
Message-ID: <87halglf5p.fsf@rustcorp.com.au> (raw)
In-Reply-To: <20130208115240.GN15092@kernel.dk>

Jens Axboe <axboe@kernel.dk> writes:
> On Fri, Feb 08 2013, Rusty Russell wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> > The virtqueue_add_buf function has two limitations:
>> >
>> > 1) it requires the caller to provide all the buffers in a single call;
>> >
>> > 2) it does not support chained scatterlists: the buffers must be
>> > provided as an array of struct scatterlist.
>> >
>> > Because of these limitations, virtio-scsi has to copy each request into
>> > a scatterlist internal to the driver.  It cannot just use the one that
>> > was prepared by the upper SCSI layers.
>> 
>> Hi Paulo,
>> 
>>         Note that you've defined your problem in terms of your solution
>> here.  For clarity:
>> 
>> The problem: we want to prepend and append to a scatterlist.  We can't
>>         append, because the chained scatterlist implementation requires
>>         an element to be appended to join two scatterlists together.
>> 
>> The solution: fix scatterlists by introducing struct sg_ring:
>>         struct sg_ring {
>>                 struct list_head ring;
>>         	unsigned int nents;
>>         	unsigned int orig_nents; /* do we want to replace sg_table? */
>>                 struct scatterlist *sg;
>>         };
>
> This would definitely be more flexible than the current chaining.
> However:
>
>> The workaround: make virtio accept multiple scatterlists for a single
>>         buffer.
>> 
>> There's nothing wrong with your workaround, but if other subsystems have
>> the same problem we do, perhaps we should consider a broader solution?
>
> Do other use cases actually exist? I don't think I've come across this
> requirement before, since it was introduced (6 years ago, from a cursory
> look at the git logs!).

Thanks Jens.

OK, let's not over-solve the problem then, we'll make a virtio-specific
solution.

Paulo, I'll take your patches once you repost.

Thanks,
Rusty.

WARNING: multiple messages have this Message-ID (diff)
From: Rusty Russell <rusty@rustcorp.com.au>
To: Jens Axboe <axboe@kernel.dk>
Cc: kvm@vger.kernel.org, mst@redhat.com,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [RFC PATCH 0/8] virtio: new API for addition of buffers, scatterlist changes
Date: Wed, 13 Feb 2013 20:16:42 +1030	[thread overview]
Message-ID: <87halglf5p.fsf@rustcorp.com.au> (raw)
In-Reply-To: <20130208115240.GN15092@kernel.dk>

Jens Axboe <axboe@kernel.dk> writes:
> On Fri, Feb 08 2013, Rusty Russell wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> > The virtqueue_add_buf function has two limitations:
>> >
>> > 1) it requires the caller to provide all the buffers in a single call;
>> >
>> > 2) it does not support chained scatterlists: the buffers must be
>> > provided as an array of struct scatterlist.
>> >
>> > Because of these limitations, virtio-scsi has to copy each request into
>> > a scatterlist internal to the driver.  It cannot just use the one that
>> > was prepared by the upper SCSI layers.
>> 
>> Hi Paulo,
>> 
>>         Note that you've defined your problem in terms of your solution
>> here.  For clarity:
>> 
>> The problem: we want to prepend and append to a scatterlist.  We can't
>>         append, because the chained scatterlist implementation requires
>>         an element to be appended to join two scatterlists together.
>> 
>> The solution: fix scatterlists by introducing struct sg_ring:
>>         struct sg_ring {
>>                 struct list_head ring;
>>         	unsigned int nents;
>>         	unsigned int orig_nents; /* do we want to replace sg_table? */
>>                 struct scatterlist *sg;
>>         };
>
> This would definitely be more flexible than the current chaining.
> However:
>
>> The workaround: make virtio accept multiple scatterlists for a single
>>         buffer.
>> 
>> There's nothing wrong with your workaround, but if other subsystems have
>> the same problem we do, perhaps we should consider a broader solution?
>
> Do other use cases actually exist? I don't think I've come across this
> requirement before, since it was introduced (6 years ago, from a cursory
> look at the git logs!).

Thanks Jens.

OK, let's not over-solve the problem then, we'll make a virtio-specific
solution.

Paulo, I'll take your patches once you repost.

Thanks,
Rusty.

  reply	other threads:[~2013-02-13 10:18 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-07 12:22 [RFC PATCH 0/8] virtio: new API for addition of buffers, scatterlist changes Paolo Bonzini
2013-02-07 12:22 ` Paolo Bonzini
2013-02-07 12:22 ` [RFC PATCH 1/8] virtio: add functions for piecewise addition of buffers Paolo Bonzini
2013-02-07 12:22   ` Paolo Bonzini
2013-02-07 12:22 ` [RFC PATCH 2/8] virtio-blk: reorganize virtblk_add_req Paolo Bonzini
2013-02-07 12:22   ` Paolo Bonzini
2013-02-07 12:22 ` [RFC PATCH 3/8] virtio-blk: use virtqueue_start_buf on bio path Paolo Bonzini
2013-02-07 12:22   ` Paolo Bonzini
2013-02-07 12:22 ` [RFC PATCH 4/8] virtio-blk: use virtqueue_start_buf on req path Paolo Bonzini
2013-02-07 12:22 ` Paolo Bonzini
2013-02-07 12:22 ` [RFC PATCH 5/8] scatterlist: introduce sg_unmark_end Paolo Bonzini
2013-02-07 12:22   ` Paolo Bonzini
2013-02-07 12:35   ` Jens Axboe
2013-02-07 12:35   ` Jens Axboe
2013-02-07 12:22 ` [RFC PATCH 6/8] virtio-net: unmark scatterlist ending after virtqueue_add_buf Paolo Bonzini
2013-02-07 12:22   ` Paolo Bonzini
2013-02-07 12:22 ` [RFC PATCH 7/8] virtio-scsi: use virtqueue_start_buf Paolo Bonzini
2013-02-07 12:22   ` Paolo Bonzini
2013-02-07 12:22 ` [RFC PATCH 8/8] virtio: reimplement virtqueue_add_buf using new functions Paolo Bonzini
2013-02-07 12:22 ` Paolo Bonzini
2013-02-07 13:09 ` [RFC PATCH 0/8] virtio: new API for addition of buffers, scatterlist changes Michael S. Tsirkin
2013-02-07 13:09   ` Michael S. Tsirkin
2013-02-07 13:14   ` Paolo Bonzini
2013-02-07 13:14     ` Paolo Bonzini
2013-02-07 13:23     ` Michael S. Tsirkin
2013-02-07 13:23       ` Michael S. Tsirkin
2013-02-07 13:20       ` Paolo Bonzini
2013-02-07 13:20         ` Paolo Bonzini
2013-02-07 13:31         ` Michael S. Tsirkin
2013-02-07 13:31           ` Michael S. Tsirkin
2013-02-07 13:30           ` Paolo Bonzini
2013-02-07 13:30             ` Paolo Bonzini
2013-02-08  4:05 ` Rusty Russell
2013-02-08  4:05   ` Rusty Russell
2013-02-08  6:35   ` Paolo Bonzini
2013-02-08  6:35   ` Paolo Bonzini
2013-02-08 11:52   ` Jens Axboe
2013-02-08 11:52   ` Jens Axboe
2013-02-13  9:46     ` Rusty Russell [this message]
2013-02-13  9:46       ` Rusty Russell

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=87halglf5p.fsf@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=asias@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=gaowanlong@cn.fujitsu.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=virtualization@lists.linux-foundation.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 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.