From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f41.google.com (mail-lf1-f41.google.com [209.85.167.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C458F199E86 for ; Tue, 30 Apr 2024 19:19:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714504778; cv=none; b=nePRvxUyt0Uh+FtAeZ7mb4BLLY9D2D9cXsbP1mPvRRu+po8akUAKiF0UR+kFAp3WyT4tZHSeyXqx4D/kSrvLGsuTw1D+RLSqiA5Pb0fTWWVHEbB8617gkz22l+YJKEApWyo3RKwng9mM0y744j/PCBaVMf0Zc7uksvdo879qEAc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714504778; c=relaxed/simple; bh=rXrYXwxz8utS37Zdd/NHXMvvYHT1Pko55fdXpz+a0f8=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=fV/CZsjZCZO76FHREHOQbJ7XRvESLTO7zqoarWt60DIG8b/NglXkKOzztA6DUdyUMoKbyCQPdiXkF8yZXwu41WW8fm87GFVpH+s0MwsY6kUL+DCj4THVr3ljGe22HmMfXxQM7ifenk94s9Dd/0LICVlBqTzRXfNOQUfGNwISGOo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=yhrCM4Yd; arc=none smtp.client-ip=209.85.167.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="yhrCM4Yd" Received: by mail-lf1-f41.google.com with SMTP id 2adb3069b0e04-51b09c3a111so8632077e87.1 for ; Tue, 30 Apr 2024 12:19:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1714504773; x=1715109573; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=JBbLSd4KX1Ml6Kx/U9tspaOWXAez2c+6/7rMF07WiMA=; b=yhrCM4YdS1mVSkC6dZlf25VsI6foSQBCk1DyesxJ/3Nf9GIMHy2gLaao5m1yno4E4y LtG62zQbF9BTkR0aOMYlQm1EEMYIDUCq9PMx9uJmf3kKE2VKFDXgbV/etlhd83AV68Mw q0LoGQlekuHwfa+7Hc6GI0DZ/Iir29gHWQ93YeFSVeB4ebbTLs1IPAMrizUw1zcYmLPc dGrMy+HB86r/BLLws08RKvC1H2mXBtxc86+vZP7uOFNjHzPyfH/Yc46yDsB17PHHg3/8 cmY8hLG3CzLveY5qMgQT1zM60pq2ET0rzg8DrUA3vcerRTlTRWGeqNRVTZKVEPp06M3c o+RQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714504773; x=1715109573; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=JBbLSd4KX1Ml6Kx/U9tspaOWXAez2c+6/7rMF07WiMA=; b=Bq2MP0q3H8fUSMgUEpNj1kg/uXU9wFHDm8cY6xlFcp/ECwf8vuXz0ehffKJwJlOt2l V9TQQxVJ1PJrBoWZ+U7OC+pGVZlKAD539c3me2x0TdnAtJ463DInI3Ma3Ud9Rke7+CfT jJyfQ7cv/g4VCvDHduXQ0VpKEvKWULrxCeD3YJKXiGTmmgTRnpfz74MADwVBv2+a89ir pqnrHpJEL/0NUNrNSP8kCFcmd3yvbAxHA+eKWj7YF0UZmHxFTFoEVORbLTyAwqRWW69A GjbMokKPfYSaojvEPpI16StyKJVFAh3kgon/XaqNVFa+FsoE9cEhWng43iyUWKjuhtmq O1RA== X-Forwarded-Encrypted: i=1; AJvYcCXg0B5bdiUfhPDIZca9QFG8LP+R2Ek9dLIZ59Tos6P0J6yDFMVApHpru1qjdbQdjheAPFiO2Ltmp3dBHhUPN5ktqfDYoTo1O4rCrWU= X-Gm-Message-State: AOJu0YzMvhSea2LXwSgsggWgutITgO2/WnvXlOiIaRYm8sq0DiOgl65w IAZAd4Z9PHc1jDL654nVTNznSkV2haClhHkmWAYzLZsjBIFhghdiiqotlv3h7OCdL2bIJ7empOV Q95MuSYnqT/7uf8yNbM8euEWoBHC5OI6MYKjJ X-Google-Smtp-Source: AGHT+IEv1vAD5KmPx2DuK83e++oMbqGnWC2h94YZYICJhje+Ouaos+1vgy9d2n+Zn+VtRp04liNfK+AEGoM+Z6BbCWI= X-Received: by 2002:a05:6512:3492:b0:517:8ad8:c64 with SMTP id v18-20020a056512349200b005178ad80c64mr259130lfr.21.1714504772286; Tue, 30 Apr 2024 12:19:32 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-alpha@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20240403002053.2376017-1-almasrymina@google.com> <20240403002053.2376017-8-almasrymina@google.com> <8357256a-f0e9-4640-8fec-23341fc607db@davidwei.uk> <11f52113-7b67-4b45-ba1d-29b070050cec@kernel.dk> In-Reply-To: <11f52113-7b67-4b45-ba1d-29b070050cec@kernel.dk> From: Mina Almasry Date: Tue, 30 Apr 2024 12:19:17 -0700 Message-ID: Subject: Re: [RFC PATCH net-next v8 07/14] page_pool: devmem support To: Jens Axboe Cc: David Wei , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-alpha@vger.kernel.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, sparclinux@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-arch@vger.kernel.org, bpf@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Jonathan Corbet , Richard Henderson , Ivan Kokshaysky , Matt Turner , Thomas Bogendoerfer , "James E.J. Bottomley" , Helge Deller , Andreas Larsson , Jesper Dangaard Brouer , Ilias Apalodimas , Steven Rostedt , Masami Hiramatsu , Mathieu Desnoyers , Arnd Bergmann , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Eduard Zingerman , Song Liu , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , Steffen Klassert , Herbert Xu , David Ahern , Willem de Bruijn , Shuah Khan , Sumit Semwal , =?UTF-8?Q?Christian_K=C3=B6nig?= , Amritha Nambiar , Maciej Fijalkowski , Alexander Mikhalitsyn , Kaiyuan Zhang , Christian Brauner , Simon Horman , David Howells , Florian Westphal , Yunsheng Lin , Kuniyuki Iwashima , Arseniy Krasnov , Aleksander Lobakin , Michael Lass , Jiri Pirko , Sebastian Andrzej Siewior , Lorenzo Bianconi , Richard Gobert , Sridhar Samudrala , Xuan Zhuo , Johannes Berg , Abel Wu , Breno Leitao , Pavel Begunkov , Jason Gunthorpe , Shailend Chand , Harshitha Ramamurthy , Shakeel Butt , Jeroen de Borst , Praveen Kaligineedi , linux-mm@kvack.org, Matthew Wilcox Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Apr 30, 2024 at 11:55=E2=80=AFAM Jens Axboe wrote= : > > On 4/30/24 12:29 PM, Mina Almasry wrote: > > On Tue, Apr 30, 2024 at 6:46?AM Jens Axboe wrote: > >> > >> On 4/26/24 8:11 PM, Mina Almasry wrote: > >>> On Fri, Apr 26, 2024 at 5:18?PM David Wei wrote: > >>>> > >>>> On 2024-04-02 5:20 pm, Mina Almasry wrote: > >>>>> @@ -69,20 +106,26 @@ net_iov_binding(const struct net_iov *niov) > >>>>> */ > >>>>> typedef unsigned long __bitwise netmem_ref; > >>>>> > >>>>> +static inline bool netmem_is_net_iov(const netmem_ref netmem) > >>>>> +{ > >>>>> +#if defined(CONFIG_PAGE_POOL) && defined(CONFIG_DMA_SHARED_BUFFER) > >>>> > >>>> I am guessing you added this to try and speed up the fast path? It's > >>>> overly restrictive for us since we do not need dmabuf necessarily. I > >>>> spent a bit too much time wondering why things aren't working only t= o > >>>> find this :( > >>> > >>> My apologies, I'll try to put the changelog somewhere prominent, or > >>> notify you when I do something that I think breaks you. > >>> > >>> Yes, this is a by-product of a discussion with regards to the > >>> page_pool benchmark regressions due to adding devmem. There is some > >>> background on why this was added and the impact on the > >>> bench_page_pool_simple tests in the cover letter. > >>> > >>> For you, I imagine you want to change this to something like: > >>> > >>> #if defined(CONFIG_PAGE_POOL) > >>> #if defined(CONFIG_DMA_SHARED_BUFFER) || defined(CONFIG_IOURING) > >>> > >>> or something like that, right? Not sure if this is something I should > >>> do here or if something more appropriate to be in the patches you > >>> apply on top. > >> > >> In general, attempting to hide overhead behind config options is alway= s > >> a losing proposition. It merely serves to say "look, if these things > >> aren't enabled, the overhead isn't there", while distros blindly enabl= e > >> pretty much everything and then you're back where you started. > >> > > > > The history there is that this check adds 1 cycle regression to the > > page_pool fast path benchmark. The regression last I measured is 8->9 > > cycles, so in % wise it's a quite significant 12.5% (more details in > > the cover letter[1]). I doubt I can do much better than that to be > > honest. > > I'm all for cycle counting, and do it myself too, but is that even > measurable in anything that isn't a super targeted microbenchmark? Or > even in that? > Not as far as I can tell, no. This was purely to improve the page_pool benchmark. > > There was a desire not to pay this overhead in setups that will likely > > not care about devmem, like embedded devices maybe, or setups without > > GPUs. Adding a CONFIG check here seemed like very low hanging fruit, > > but yes it just hides the overhead in some configs, not really removes > > it. > > > > There was a discussion about adding this entire netmem/devmem work > > under a new CONFIG. There was pushback particularly from Willem that > > at the end of the day what is enabled on most distros is what matters > > and we added code churn and CONFIG churn for little value. > > > > If there is significant pushback to the CONFIG check I can remove it. > > I don't feel like it's critical, it just mirco-optimizes some setups > > that doesn't really care about this work area. > > That is true, but in practice it'll be enabled anyway. Seems like it's > not really worth it in this scenario. > OK, no pushback from me. I'll remove the CONFIG check in the next iteration= . --=20 Thanks, Mina