RE: [PATCH] VIRTIO_F_USED_EVENT_AUTO_DISABLE: add new used buffer notification suppression mechanism

Lege Wang
2024-07-03T04:32:46Z

hi

Thanks for your review.

>
> > DPU(Data Processing Unit), DPU would need to submit DMAs to copy guest
> > suppression informations to DPU internal memory, then check whether
> > used event should be notified by examing this internal memory, which
> > is obvious expensive, and this DMA operation is needed every time
> > DPU attempts to issue used event,
>
> Several things could be used to help here:
>
> 1) With the event index, as long as the used index doesn't pass used
> events you don't need to fetch even index every time
Yeah, I agree VIRTIO_F_EVENT_IDX could help here, but I think it's a relatively
crude mechanism, I have two questions below:
1. Used event notification suppression structure is still located in
host memory(in dpu case), I'm not sure whether used_event would
be allowed to update in the running of one virtio device, If it's allowed,
seems devices still need to fetch newest used_event info timely.
2. According to virtio-spec, for example, if used_event is 0, then a device with
VIRTIO_F_EVENT_IDX would send a used buffer notification to the driver after
the first buffer is used (and again after the 65536th buffer), should this behavior
be strictly followed? For example, driver with VIRTIO_F_EVENT_IDX negotiated
sets used_event to 0, used buffer notification still could be sent for any used buffer event.

> 2) Interrupt coalescing may help to throttle the interrupt rate
> 3) Spec doesn't forbid implementation specific heuristics for
> interrupt moderation
Yes, indeed we're trying to use the information about the latest used ring entry
driver has processed, which is introduced in this patch, to do interrupt coalescing or moderation.

> > now, but it may choose appropriate timing to send used events according to
> > its notification coalescing policy , now it also does not need to DMA guest
> > memory to obtain notification suppression.
>
> Performance numbers are welcomed:
>
> 1) compare disabling via register against memory
> 2) compare auto disable against no auto
I see, but sorry, currently we don't have any performance numbers yet,
but theoretical analysis and think it'll help improve performance. Some
popular nic also supports this feature, which disables interrupt after sending
one and needs driver to enable it again. When enabling interrupt again, drivers
can attach some extra information.

>
> > + The driver may benefit from just informing device the latest used
> ring entry
> > + it has processed, but not enabling used buffer notification. This field
> only
> > + occupies one bit, if set to 1, it will enable used buffer notification,
> otherwise not.
> > +\end{description}
>
> Any reason we can't reuse the existing meachsim like used index /
> event suppression here?
Please see my comments above.

Regards,
Xiaoguang Wang
>
> Basically it just sounds like we need a transport specific method for
> them besides being part of the virtqueue.
>
> Thanks