From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-io1-f43.google.com (mail-io1-f43.google.com [209.85.166.43]) (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 C1DA6184120 for ; Tue, 30 Apr 2024 18:55:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.166.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714503313; cv=none; b=gdj6IVkjPLeSXzy2xcGxxC79cTK9NRcFcN/rE8meUdtclPLs6AjxC5ONQGLQVZiPdCBRLNTslUDoPaXytt+gZp2f5bZhLg+evatX8Afs81kxVoco9IUAulZyijkDp36M/nusdr3NswGzBaOprD3s1W0sm1NO+KAP9qXXy1Cxd9Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714503313; c=relaxed/simple; bh=Y4zEuuEidL9HuzJO4epGfanxTN6QUEoob6ZCDJEvDGA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=D+s9XgNP7iuVdsUTgeQM8DG38+nWPV1PIRBypHHUr2qBwS1u5+fN8CKgvJEfiStByS+SlP6JMSC7D3oWvvKRkmGVsdChWPc8E2oC6X97vJcMQ6+YXjYKvqm09YUI1L9wPaKS7IsDSqpTGkLu9MUHTmYioTE2iiRXbXayl+jGa+o= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.dk; spf=pass smtp.mailfrom=kernel.dk; dkim=pass (2048-bit key) header.d=kernel-dk.20230601.gappssmtp.com header.i=@kernel-dk.20230601.gappssmtp.com header.b=fHqmNmfo; arc=none smtp.client-ip=209.85.166.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.dk Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=kernel.dk Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel-dk.20230601.gappssmtp.com header.i=@kernel-dk.20230601.gappssmtp.com header.b="fHqmNmfo" Received: by mail-io1-f43.google.com with SMTP id ca18e2360f4ac-7dec804bd19so12691239f.1 for ; Tue, 30 Apr 2024 11:55:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20230601.gappssmtp.com; s=20230601; t=1714503310; x=1715108110; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=SHR3RGRuxJEH/dBiKYgawHqY9Lnlphnx76oS7XwiA+c=; b=fHqmNmfo5j3nkc0/YlfKBue3Uo54rLdcUx3usEOkU3PR8LAWsZB5mGsa98YQUIEP4O NfUPjz2sWBba6nJMHrkxHT8QwnynajW9W7oimNjlrdurIiXHuFMKS+ogeCkHTkMcTGi7 ybm0qpbIpPiQ5fH2XMF3wNU4cc4faT7DKHnj/N8BdAgA+3uUeb/d/sWMrtIZK8qNSkpR yFkNOapl3EP94cWGzjMPhyJrZgbJococGHKTUZXHyVcAV52JpSZfzYfQQZVcQMov3CY8 mPRPS8dXdQeJYqKis1T/ZT/M8f3n/u4wLvIrlX6JCtuXTIi0mOo/gsQ6UgRSV8MmS+zS Gslg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714503310; x=1715108110; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=SHR3RGRuxJEH/dBiKYgawHqY9Lnlphnx76oS7XwiA+c=; b=IsGtB7vfZqw3b7HDO/Za7qh0PSo5QHrNR08np5dc9hzdgb/WsAppgUBP/6NncdvfCJ CdyS0IKCvgM50DQ57EdO2vdCNaJXVmeVm1TvcYPZBRyhFsID0JHQjA6iq6GU4b3zH/XN 8FOK+ErVVTeyWQn2uAYDtEYE60d0Pevy0PRoTTn1ge+x5rNa4SpivT3m8tbshFpTjRJq YEKzNCy/26eXUq9LGMKXFDWnQf8HXsWT+qI4KnqZ4VvVmbKDQ0uEOMuv+3wy7rQMczyp 1bxLjHMrKy1evmbjqyxyHcz5V9D0aqbAvnq+kLWwvXROXbXkjuU8gnPaTeosgBsjhs8g tyXA== X-Forwarded-Encrypted: i=1; AJvYcCVhogygdBJiHyut1cIZppMmydgWKXyCknlqlqS8DCf3fUeQXYznBe/b/E2QFlAJ2EX5PNg4nb6XHe+/12A963u+Vkm71SdBN2zopA== X-Gm-Message-State: AOJu0YzurEXGb6aeqy47E4K0gz5ZDx60fBizZOd1xk4kV7twnjC5nZFM 47yx9uAdB/MGtLbjIIjP60UOd+Y9jLfWRc8TsD9LQHMOPKcFacUUG8xwqlP7qUc= X-Google-Smtp-Source: AGHT+IFtf+wSV3MsQDY8kH1lbfF76eo7FuUDr/3x1d33gkfYgxzUOO8lZB4OHFgKKuLwCBSXBvT5jw== X-Received: by 2002:a6b:f110:0:b0:7de:e04b:42c3 with SMTP id e16-20020a6bf110000000b007dee04b42c3mr828170iog.0.1714503309571; Tue, 30 Apr 2024 11:55:09 -0700 (PDT) Received: from [192.168.1.116] ([96.43.243.2]) by smtp.gmail.com with ESMTPSA id v21-20020a05663812d500b00487f9ec16b9sm389802jas.173.2024.04.30.11.55.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 30 Apr 2024 11:55:08 -0700 (PDT) Message-ID: <11f52113-7b67-4b45-ba1d-29b070050cec@kernel.dk> Date: Tue, 30 Apr 2024 12:55:05 -0600 Precedence: bulk X-Mailing-List: linux-mips@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH net-next v8 07/14] page_pool: devmem support To: Mina Almasry 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 References: <20240403002053.2376017-1-almasrymina@google.com> <20240403002053.2376017-8-almasrymina@google.com> <8357256a-f0e9-4640-8fec-23341fc607db@davidwei.uk> Content-Language: en-US From: Jens Axboe In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 to >>>> 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 always >> a losing proposition. It merely serves to say "look, if these things >> aren't enabled, the overhead isn't there", while distros blindly enable >> 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? > 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. -- Jens Axboe