Sorry I mistakenly focused on NET case, please pass it over. I will do any relevant suggestion in patch-based way. Thanks. On Sun, 2017-08-13 at 23:11 +0900, Koichiro Den wrote: > Thanks for your comments, Michael and Jason. And I'm sorry about late > response. > To be honest, I am on a summer vacation until next Tuesday. > > I noticed that what I wrote was not sufficient. Regardless of caching > mechanism > existence, the "event" could legitimately be at any point out of the latest > interval, which vhost_notify checks it against, meaning that if it's out of > the > interval we cannot distinguish whether or not it lags behind or has a > lead. And > it seems to conform to the spec. Thanks for leading me to the spec. The corner > case I point out here is: > (0) event idx feature turned on + TX napi turned off > -> (1) guest-side TX traffic bursting occurs and delayed callback set > -> (2) some interruption triggers skb_xmit_done > -> (3) guest-side modest traffic makes the interval proceed to unbounded > extent > without updating "event" since NO_INTERRUPT continues to be set on its shadow > flag. > > IMHO, if you plan to make TX napi the only choice, doing this sort of > optimisation beforehand seems likely to be in vain. > > So, if the none-TX napi case continues to coexist, what I would like to > suggest > is not just the sort of my last email, but like making maximum staleness of > "event" less than or equal to vq.num, and afterward caching suggestion. > Otherwise, I guess I should not repost my last email since it would make > matters > too complicated even though it will soon be removed when TX-napi becomes the > only choice. > > Thanks! > > > On Wed, 2017-08-09 at 07:37 +0300, Michael S. Tsirkin wrote: > > On Wed, Aug 09, 2017 at 10:38:10AM +0800, Jason Wang wrote: > > > I think don't think current code can work well if vq.num is grater than > > > 2^15. Since all cached idx is u16. This looks like a bug which needs to be > > > fixed. > > > > That's a limitation of virtio 1.0. > > > > > > * else if the interval of vq.num is [2^15, 2^16): > > > > the logic in the original patch (809ecb9bca6a9) suffices > > > > * else (= less than 2^15) (optional): > > > > checking only (vring_need_event(vq->last_used_event, new + vq->num, new) > > > > would suffice. > > > > > > > > Am I missing something, or is this irrelevant? > > > > Could you pls repost the suggestion copying virtio-dev mailing list > > (subscriber only, sorry about that, but host/guest ABI discussions > > need to copy that list)? > > > > > Looks not, I think this may work. Let me do some test. > > > > > > Thanks > > > > I think that at this point it's prudent to add a feature bit > > as the virtio spec does not require to never move the event index back. > >
