From: Lege Wang <lege.wang@jaguarmicro.com>
To: Parav Pandit <parav@nvidia.com>,
"virtio-comment@lists.oasis-open.org"
<virtio-comment@lists.oasis-open.org>
Subject: RE: [virtio-comment] [RFC PATCH] VIRTIO_F_USED_EVENT_AUTO_DISABLE: add new used buffer notification suppression mechanism
Date: Thu, 22 Feb 2024 01:56:28 +0000 [thread overview]
Message-ID: <SI2PR06MB5385AF3B8EE696B201B95F7CFF562@SI2PR06MB5385.apcprd06.prod.outlook.com> (raw)
In-Reply-To: <PH0PR12MB5481935584B62FD5ACE5FE3CDC472@PH0PR12MB5481.namprd12.prod.outlook.com>
hi,
I'm back from a long vacation, and have some internal work to do, sorry for
long late response.
> > >
> > > Hence a better driver used buffer notification should be,
> > >
> > > 1. index upto which it has processed
> > I would change it to " index upto which it will process next", the reason is
> that
> > virtio core driver codes have already a well-maintained "struct
> > vring_virtqueue->last_used_idx", we can use it for notification, and using this
> > value, the device will also be able to learn the driver consumption rate.
> >
> It the index upto which the driver _has_ already processed, right?
Yes, and your previous comment is right, ignore my comment above.
>
> > > 2. explicit enable/disable bit to enable the notification
> > I wonder if there's a case that the driver just notifies the used index info, but
> > not enable the device notification,
> Yes. there is a scenario.
> For example, device has received 28 packets.
> When the driver is polling, it was given budget to poll only 20 packets. In this
> case NAPI context will come back again to poll rest of the 8 packets.
> In this scenario, it is desired to still update the polled completions (used
> elements), but not enable the notification.
> Notification can be enabled later when remaining 8 or more packets are
> processed.
OK, I see.
>
> > the driver will always need one enable
> > operation finally. And the notification data would be similar to:
> > le32 {
> > union {
> > vq_index: 16; /* Used if VIRTIO_F_NOTIF_CONFIG_DATA
> not
> > negotiated */
> > vq_notif_config_data: 16; /* Used if VIRTIO_F_NOTIF_CONFIG_DATA
> > negotiated */
> > };
> > next_off : 15;
> > next_wrap : 1;
> > };
> > next_off and next_wrap will denote last_used_idx info, and vq_index or
> > vq_notif_config_data is also necessary, so for one 32-bit register, there's no
> > room for this enable/disable bit.
> >
> Right. There isn't enough room for enable bit.
> I see two options.
>
> 1. When device wants to still work with 32-bit doorbells, it can reserve top bit
> of vq_index or next_off for enable/disable.
> Here the restriction is the queue depth would be limited to 16K descriptors,
> which is fairly large enough.
> Or having top bix of vq_index is better as, it may be well within limit for a
> device to offer 32K queues.
> For non rdma devices 32K queues is also seems reasonable large value.
I'd like to not add these restrictions, in case some devices need bigger number of
descriptors or queue depth.
>
> 2. Device can offer 64-bit format where enable bit is located in the upper
> 32-bit.
>
> 3. Or may be option for the device with two different feature bits, because
> 64-bit doorbells are relatively new for virtio to adapt to.
Sounds feasible, I'll try this way.
I'll prepare a V1 patch as soon as possible, thanks for your patience.
Regards,
Lege.wang
>
> >
> > Regards,
> > lege.wang
> > >
> > > These two fields clearly communicate to the device, upto how much used
> > > buffer is consumed and understand the driver consumption rate.
> > > This way, there is no ambiguity for device on where to write next used
> > > ring entry.
> > > (where to write next used entry is not driven by the notification
> > enablement).
> > >
> > > And the explicit enable bit indicates notifications are enabled back.
> > >
> > > Technically, only enable bit is needed, but doing #1 is helpful for
> > > the device to understand driver consumption pace.
> > > And for that may be current notification can be made 64-bits or as
> > > suggested in your patch to have new location.
> > >
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
prev parent reply other threads:[~2024-02-22 1:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-25 2:30 [virtio-comment] [RFC PATCH] VIRTIO_F_USED_EVENT_AUTO_DISABLE: add new used buffer notification suppression mechanism Xiaoguang Wang
2024-01-25 3:28 ` Lege Wang
2024-01-30 6:11 ` Lege Wang
2024-01-30 6:52 ` Parav Pandit
2024-01-31 6:33 ` Lege Wang
2024-01-31 6:45 ` Parav Pandit
2024-01-31 7:21 ` Lege Wang
2024-01-31 7:24 ` Parav Pandit
2024-01-31 8:39 ` Lege Wang
2024-02-01 9:44 ` Lege Wang
2024-02-05 7:40 ` Parav Pandit
2024-02-22 1:56 ` Lege Wang [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=SI2PR06MB5385AF3B8EE696B201B95F7CFF562@SI2PR06MB5385.apcprd06.prod.outlook.com \
--to=lege.wang@jaguarmicro.com \
--cc=parav@nvidia.com \
--cc=virtio-comment@lists.oasis-open.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 read-only IMAP folder(s) and NNTP newsgroup(s).