Received: by 2002:a05:6a10:f347:0:0:0:0 with SMTP id d7csp1497729pxu; Tue, 24 Nov 2020 01:33:25 -0800 (PST) X-Google-Smtp-Source: ABdhPJz3cx2H//tvZZXVyvAxoAz9Y8Wh0WWcPnCmEFQNUmFpDbQItsYR1BU+DyCNcbAd0Uz75ei6 X-Received: by 2002:a17:906:ee2:: with SMTP id x2mr3463972eji.326.1606210405500; Tue, 24 Nov 2020 01:33:25 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1606210405; cv=none; d=google.com; s=arc-20160816; b=kxhq/V6DcbWBSW0tlTI0q7PaDQfXrxG17eyAjnGTntqJa1MU2Jvcuyt4cNjQxXlf34 cUrzMOzAb+USN60thylW2SVQ/oMHUqsvKMPB1adKMyb0kdHWm47XMjA2WhtleiabEY3H z0sLmqnoHdx2pQ7hT4llggBp237vcBNP6OxW1nUBNdzadDySmfiphJoUpOKdQ6AKmtQx YfpNJfi57yFtRA2SVTs0tCQm0wcoylQmBVVByW4MbAegf0YUZpaQbDJJZAYCwfJhhzep dktMxR0z4VsbjHRAYQMfcnf2TiMcvN8szjTttR/sWIhRHD1If/4ZZaYnVrygm2WOA77A 6Nhw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=bHKb7nomqkZKADTymOqb9YebPYk5PICoUgtAna9zlJA=; b=lG8sHgUXbhwODHYD3VtouoWKfMh2HMPCQwaVkSZA6ycqAPc6y+OQxpuNk+DvcMyu+r iU66e8cgtUBGqDAllIgFTHeATbEgz7/Mr1W+LjOemK8TV1BsolM5nMwYPuM2RDrCQ8a6 2SI6ndlY5mbE3uOudc6YGvO1rwSWH3bn8Epg+yFo/rhmFgFWvUenajbHn3D60txLxhpC M5pZT4PNZCavstCaEjQT6utK8OD9D07SDPWTQGXbei4OnUZlUxcWJRZaXuzv/9Cai3Vz o4ePEjhuxSW1jzytPrv6R82rCZ8FzJnaD+A9f/kH6pyk4u49NL2tMwK0E5h6pneH+q51 GbnQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=kUjmDLTz; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id n18si7879058ejk.69.2020.11.24.01.33.02; Tue, 24 Nov 2020 01:33:25 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=kUjmDLTz; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731315AbgKXJ2s (ORCPT + 99 others); Tue, 24 Nov 2020 04:28:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52710 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731024AbgKXJ2s (ORCPT ); Tue, 24 Nov 2020 04:28:48 -0500 Received: from mail-pf1-x444.google.com (mail-pf1-x444.google.com [IPv6:2607:f8b0:4864:20::444]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E3414C0613D6; Tue, 24 Nov 2020 01:28:47 -0800 (PST) Received: by mail-pf1-x444.google.com with SMTP id w202so2108896pff.10; Tue, 24 Nov 2020 01:28:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=bHKb7nomqkZKADTymOqb9YebPYk5PICoUgtAna9zlJA=; b=kUjmDLTzJAMIAUeMr0l6en9vgdoe6CAUWMsC1khPiePFd01zJwUlO9BM0Y0FZ9aevz UuXpqW5MEhhEgxiJybCmJkUd11D1zymEkd0P6UC8fCf8h6sOGAqrsjwNOSv1mZQpq0zZ fZlpvwfJTi3YHbKIyxz8zY77tpIBemrM+ONVEO20Mv+MPgkkcKOyd0r9Y9rNZkMQnjNl sBhNUyOo+DOSQoz6NQYXO+sxKBvvZ+JRvCzfItY+k4r0/NDORlvz5jSKEeSoQiPTW9WC /s/+PPiOgrJFNWN5PnxVPOHnizrU6pDvXAyKBNR76m5mLGKuEYucE4XUr4W5PXlC7fKE Vy7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=bHKb7nomqkZKADTymOqb9YebPYk5PICoUgtAna9zlJA=; b=q2jktmY7o4LnmbbfPB1+nZejDj2PWU6RG/svkzuXaRno97KbPmWOC9z4VZLnsFOlFw SjbSr+UEkxKxHWIx4ea+QvNgd+JW5e5uYrce2I5Hikzo1WYcNpYYPTVR0eMzePSoZUyN WVFFzgQlBMfDhDXPIyv58Bd4P5RmYFT9i6RQX7z5nItn1SB+VbfGvP4KnBwShgNeHojV DAFhwSHKl2J4GwMKUf2AazG3wZ3cYrg0HW8/YkZpLkkN3MlvpPP7BlTfzBW9ONtDkJCs dMRzLaQPBbCumipF0O5nKvO1OnkjiKU9aljD/KN/hURiei9q3KI8IZTJ+mmVtOyNF67d hfbg== X-Gm-Message-State: AOAM5320BOoFgDH/KqZ1AZnSnz7p383Y/5wHVpX0AVkgA17zj2pEIdto wLsKpv3mhEl48DMFefbZm83pTmNaHHyDUmHIr5g= X-Received: by 2002:a63:3e0f:: with SMTP id l15mr3103811pga.208.1606210127443; Tue, 24 Nov 2020 01:28:47 -0800 (PST) MIME-Version: 1.0 References: <3306b4d8-8689-b0e7-3f6d-c3ad873b7093@intel.com> In-Reply-To: From: Magnus Karlsson Date: Tue, 24 Nov 2020 10:28:36 +0100 Message-ID: Subject: Re: [PATCH 2/3] xsk: change the tx writeable condition To: Xuan Zhuo Cc: =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= , Magnus Karlsson , Jonathan Lemon , "David S. Miller" , Jakub Kicinski , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , Network Development , bpf , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 18, 2020 at 9:25 AM Xuan Zhuo wrote: > > Modify the tx writeable condition from the queue is not full to the > number of remaining tx queues is less than the half of the total number > of queues. Because the tx queue not full is a very short time, this will > cause a large number of EPOLLOUT events, and cause a large number of > process wake up. > > Signed-off-by: Xuan Zhuo > --- > net/xdp/xsk.c | 20 +++++++++++++++++--- > net/xdp/xsk_queue.h | 6 ++++++ > 2 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c > index 7f0353e..bc3d4ece 100644 > --- a/net/xdp/xsk.c > +++ b/net/xdp/xsk.c > @@ -211,6 +211,17 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len, > return 0; > } > > +static bool xsk_writeable(struct xdp_sock *xs) Not clear what this function does from the name. How about xsk_tx_half_free() or maybe xsk_tx_writeable()? > +{ > + if (!xs->tx) > + return false; Skip this test as it will slow down the code. It is only needed in one place below. > + if (xskq_cons_left(xs->tx) > xs->tx->nentries / 2) > + return false; > + > + return true; > +} > + > static bool xsk_is_bound(struct xdp_sock *xs) > { > if (READ_ONCE(xs->state) == XSK_BOUND) { > @@ -296,7 +307,8 @@ void xsk_tx_release(struct xsk_buff_pool *pool) > rcu_read_lock(); > list_for_each_entry_rcu(xs, &pool->xsk_tx_list, tx_list) { > __xskq_cons_release(xs->tx); > - xs->sk.sk_write_space(&xs->sk); > + if (xsk_writeable(xs)) > + xs->sk.sk_write_space(&xs->sk); > } > rcu_read_unlock(); > } > @@ -442,7 +454,8 @@ static int xsk_generic_xmit(struct sock *sk) > > out: > if (sent_frame) > - sk->sk_write_space(sk); > + if (xsk_writeable(xs)) > + sk->sk_write_space(sk); > > mutex_unlock(&xs->mutex); > return err; > @@ -499,7 +512,8 @@ static __poll_t xsk_poll(struct file *file, struct socket *sock, > > if (xs->rx && !xskq_prod_is_empty(xs->rx)) > mask |= EPOLLIN | EPOLLRDNORM; > - if (xs->tx && !xskq_cons_is_full(xs->tx)) > + No reason to introduce a newline here. > + if (xsk_writeable(xs)) Add an explicit "xs->tx &&" in the if statement here as we removed the test in xsk_writeable. > mask |= EPOLLOUT | EPOLLWRNORM; > > return mask; > diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h > index cdb9cf3..82a5228 100644 > --- a/net/xdp/xsk_queue.h > +++ b/net/xdp/xsk_queue.h > @@ -264,6 +264,12 @@ static inline bool xskq_cons_is_full(struct xsk_queue *q) > q->nentries; > } > > +static inline __u64 xskq_cons_left(struct xsk_queue *q) Let us call this xskq_cons_entries_present() or xskq_cons_filled_entries(). The word "left" has the connotation that I still have stuff left to do. While this is kind of true for this case, it might not be for other cases that can use your function. The function provides how many (filled) entries that are present in the ring. Can you come up with a better name as I am not super fond of my suggestions? It would have been nice to call it xskq_cons_nb_entries() but there is already such a function that is lazy in nature and that allows access to the entries. > +{ > + /* No barriers needed since data is not accessed */ > + return READ_ONCE(q->ring->producer) - READ_ONCE(q->ring->consumer); > +} > + > /* Functions for producers */ > > static inline bool xskq_prod_is_full(struct xsk_queue *q) > -- > 1.8.3.1 >