Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751834AbdG0O6W (ORCPT ); Thu, 27 Jul 2017 10:58:22 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:21784 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750836AbdG0O6V (ORCPT ); Thu, 27 Jul 2017 10:58:21 -0400 Subject: Re: [Xen-devel] [PATCH v2 09/13] xen/pvcalls: implement recvmsg To: Stefano Stabellini References: <1501017730-12797-1-git-send-email-sstabellini@kernel.org> <1501017730-12797-9-git-send-email-sstabellini@kernel.org> <3485ca4d-9c8f-fe0d-fc07-31578e370228@oracle.com> Cc: jgross@suse.com, Stefano Stabellini , linux-kernel@vger.kernel.org, xen-devel@lists.xen.org From: Boris Ostrovsky Message-ID: <2dc8286c-35ab-841d-07a8-397cc38fd969@oracle.com> Date: Thu, 27 Jul 2017 10:59:50 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1359 Lines: 37 On 07/26/2017 08:08 PM, Stefano Stabellini wrote: > On Wed, 26 Jul 2017, Boris Ostrovsky wrote: >>>> + count++; >>>> + else >>>> + wait_event_interruptible(map->active.inflight_conn_req, >>>> + pvcalls_front_read_todo(map)); >>>> + } >>> Should we be using PVCALLS_FRONT_MAX_SPIN here? In sendmsg it is >>> counting non-sleeping iterations but here we are sleeping so >>> PVCALLS_FRONT_MAX_SPIN (5000) may take a while. >>> >>> In fact, what shouldn't this waiting be a function of MSG_DONTWAIT >> err, which it already is. But the question still stands (except for >> MSG_DONTWAIT). > The code (admittedly unintuitive) is busy-looping (non-sleeping) for > 5000 iterations *before* attempting to sleep. So in that regard, recvmsg > and sendmsg use PVCALLS_FRONT_MAX_SPIN in the same way: only for > non-sleeping iterations. > OK. Why not go directly into wait_event_interruptible()? I see you write in the commit message If not enough data is available on the ring, rather than returning immediately or sleep-waiting, spin for up to 5000 cycles. This small optimization turns out to improve performance and latency significantly. Is this because of scheduling latency? I think this should be mentioned not just in the commit message but also as a comment in the code. (I also think it's not "not enough data" but rather "no data"?) -boris