From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751389AbdHMQMg convert rfc822-to-8bit (ORCPT ); Sun, 13 Aug 2017 12:12:36 -0400 Received: from sender-of-o52.zoho.com ([135.84.80.217]:21351 "EHLO sender-of-o52.zoho.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750996AbdHMQMf (ORCPT ); Sun, 13 Aug 2017 12:12:35 -0400 Message-ID: <1502640746.3547.3.camel@klaipeden.com> Subject: Re: [PATCH net] Revert "vhost: cache used event for better performance" From: Koichiro Den To: "Michael S. Tsirkin" , Jason Wang Cc: virtualization@lists.linux-foundation.org, kvm@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Date: Mon, 14 Aug 2017 01:12:26 +0900 In-Reply-To: <1502633460.3547.1.camel@klaipeden.com> References: <1501056197-3368-1-git-send-email-jasowang@redhat.com> <20170726155435-mutt-send-email-mst@kernel.org> <117542fc-eef9-c043-7c9e-daafceb7db4e@redhat.com> <6b3a9a98-c095-1729-3528-dd521f136797@redhat.com> <20170726190745-mutt-send-email-mst@kernel.org> <1501396011.21001.11.camel@klaipeden.com> <6698b85c-18f4-17e6-db70-7708692fb761@redhat.com> <20170809073422-mutt-send-email-mst@kernel.org> <1502633460.3547.1.camel@klaipeden.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.6 (3.22.6-2.fc25) Mime-Version: 1.0 Content-Transfer-Encoding: 8BIT X-ZohoMailClient: External Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > >