public inbox for virtio-comment@lists.linux.dev
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>,
	Heng Qi <hengqi@linux.alibaba.com>
Cc: Halil Pasic <pasic@linux.ibm.com>,
	virtio-comment@lists.linux.dev, Jason Wang <jasowang@redhat.com>,
	Parav Pandit <parav@nvidia.com>,
	Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Subject: Re: [PATCH v5] virtio-net: clarify coalescing parameters settings
Date: Tue, 11 Jun 2024 16:04:01 +0200	[thread overview]
Message-ID: <87tthzk23y.fsf@redhat.com> (raw)
In-Reply-To: <20240610104618-mutt-send-email-mst@kernel.org>

On Mon, Jun 10 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Jun 10, 2024 at 09:35:45PM +0800, Heng Qi wrote:
>> On Mon, 10 Jun 2024 14:46:02 +0200, Halil Pasic <pasic@linux.ibm.com> wrote:
>> > On Sat, 8 Jun 2024 10:34:22 +0800
>> > Heng Qi <hengqi@linux.alibaba.com> wrote:
>> > 
>> > > On Fri, 7 Jun 2024 22:02:46 +0200, Halil Pasic <pasic@linux.ibm.com> wrote:
>> > > > On Tue, 28 May 2024 12:47:02 +0800
>> > > > Heng Qi <hengqi@linux.alibaba.com> wrote:
>> > > >   
>> > > > > The device can set any initial coalescing parameters (0 or non-zero)
>> > > > > for the receive/send queue before the setting command is executed,
>> > > > > not just 0, enhancing device performance even without DIM enabled.
>> > > > > 
>> > > > > So we need to clarify descriptions that don't fit the behavior.  
>> > > > 
>> > > > Sorry I'm late to the party -- again! Just for my understanding: how/why
>> > > > is this a clarification and not just a (basically incompatible) change?  
>> > > 
>> > > In my opinion, "clarification" means that something may have been described
>> > > incorrectly before, and we now need to discuss, explain clearly, and correct
>> > > the possibly incorrect description.
>> > > 
>> > 
>> > I figure the difference in perceived semantics of the word
>> > "clarification" is at the root of my confusion. Let us have a look at
>> > https://dictionary.cambridge.org/de/worterbuch/englisch/clarification
>> > 
>> > According to my understanding a "clarification", while an improvement in
>> > ease of understanding and/or decrease of ambiguity (possibly to no
>> > ambiguity at all) implies that what receiving a clarification is not
>> > outright wrong.
>> > 
>> > When rectifying something that is outright incorrect or wrong, I would
>> > refer to that with words like "correction", "fix", "erratum" or
>> > "corrigendum".
>> > 
>> > > > 
>> > > > I mean if I read this correctly, before the driver had the guaranty
>> > > > that if the parameters are not set by the driver, negotiating the
>> > > > feature does not introduce any coalescing. After this in theory
>> > > > the device could just pick some max value and potentially introduce
>> > > > maximal latency in certain scenarios.  
>> > > 
>> > > "maximum latency" also means "throughput improvement".
>> > > 
>> > 
>> > Under certain assumptions. But not necessarily. Again my concern is
>> > mostly the type of change. The virtio standard maintain a revision
>> > history appendix, and I would like to avoid the nature of this change
>> > being misrepresented there. If Connie and/or Michael think it is worth
>> > fixing, I believe it can be fixed with an editorial change.
>> > 
>> > AFAIU VIRTIO_NET_F_NOTF_COAL and VIRTIO_NET_F_VQ_NOTF_COAL are about to
>> > land with virtio-1.3, i.e. there is no released/standardized virtio
>> > version where the "initialize to 0" is released. In that sense it looks
>> > like we are still on time to change this. But I am not 100% certain. In
>> > any case I don't think this as a huge impact and I'm fine going ahead
>> > with the change.
>> 
>> Sorry for the late reply, I'm on vacation.
>> 
>> I agree with this, and I prefer to release this patch as a correction for
>> virtio1.3 instead of a new patch for virtio1.4, because if devices support moderation coalescing based on virtio1.3,
>> and after virtio1.4 is released, these devices need to be updated again for a
>> more reasonable coalescing parameters.
>> 
>> Cornelia and Michael, what do you think?
>
> The TC is just voting to start the public review process.
> We can defer that by a couple more weeks if there is
> a known issue to address.
>
> For that I expect we want a final patch  and a couple of acks
> on list from TC members by end of the ballot, June 13.

FTR, I'd be fine with that if there's a consensus, but I'll not be
around to handle this personally. I also have no objection to proceeding
with the process.


  parent reply	other threads:[~2024-06-11 14:04 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-28  4:47 [PATCH v5] virtio-net: clarify coalescing parameters settings Heng Qi
2024-05-28  4:50 ` Heng Qi
2024-05-31  6:36   ` Heng Qi
2024-05-31  9:39     ` Cornelia Huck
2024-06-07 20:02 ` Halil Pasic
2024-06-08  2:34   ` Heng Qi
2024-06-10 12:46     ` Halil Pasic
2024-06-10 13:35       ` Heng Qi
2024-06-10 14:50         ` Michael S. Tsirkin
2024-06-10 15:12           ` Parav Pandit
2024-06-11 14:04           ` Cornelia Huck [this message]
2024-06-10 20:19         ` Halil Pasic
2024-06-11 10:40           ` Heng Qi
2024-06-11 16:29             ` Michael S. Tsirkin
2024-06-11 17:43               ` Parav Pandit
2024-06-13  6:13                 ` Michael S. Tsirkin
2024-06-17  2:27                   ` Heng Qi
2024-06-17 23:31                     ` Si-Wei Liu
2024-06-20  7:40                       ` Heng Qi
2024-06-21  1:21                         ` Si-Wei Liu
2024-06-21  3:24                           ` Heng Qi
2024-06-21 23:46                             ` Si-Wei Liu
2024-06-22  1:34                               ` Heng Qi
2024-06-25  4:51                                 ` Si-Wei Liu
2024-06-25  5:56                                   ` Parav Pandit
2024-06-26  1:14                                     ` Si-Wei Liu
2024-06-27 10:37                                       ` Halil Pasic
2024-06-27 11:27                                         ` Parav Pandit
2024-06-27 12:35                                         ` Michael S. Tsirkin
2024-06-27 12:45                                           ` Parav Pandit
2024-06-27 12:52                                             ` Michael S. Tsirkin
2024-06-27 13:03                                               ` Parav Pandit
2024-06-27 14:59                                                 ` Michael S. Tsirkin
2024-06-27 17:27                                               ` Si-Wei Liu
2024-06-27 17:14                                           ` Si-Wei Liu
2024-06-27 22:18                                             ` Michael S. Tsirkin
2024-06-28  6:56                                               ` Si-Wei Liu
2024-06-28  8:23                                                 ` Jason Wang
2024-06-28 19:31                                                   ` Si-Wei Liu
2024-06-30 17:04                                                     ` Michael S. Tsirkin
2024-07-03  6:09                                                     ` Jason Wang
2024-07-02 20:37                                                   ` Halil Pasic
2024-07-02 21:04                                                     ` Michael S. Tsirkin
2024-07-03  5:01                                                     ` Jason Wang
2024-06-29  6:47                                           ` Halil Pasic
2024-06-30 16:55                                             ` Michael S. Tsirkin
2024-07-02 21:43                                               ` Halil Pasic
2024-06-27 12:13                                       ` Parav Pandit
2024-06-27 12:42                                         ` Michael S. Tsirkin
2024-06-25  7:53                               ` Jason Wang
2024-06-25  8:06                                 ` Michael S. Tsirkin
2024-06-25  8:13                                   ` Jason Wang
2024-06-25  8:21                                     ` Michael S. Tsirkin
2024-06-11 23:03 ` Michael S. Tsirkin
2024-06-17  2:35   ` Heng Qi
2024-06-25  7:26     ` Michael S. Tsirkin

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=87tthzk23y.fsf@redhat.com \
    --to=cohuck@redhat.com \
    --cc=hengqi@linux.alibaba.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=parav@nvidia.com \
    --cc=pasic@linux.ibm.com \
    --cc=virtio-comment@lists.linux.dev \
    --cc=xuanzhuo@linux.alibaba.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox