From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DFA032E8881 for ; Fri, 9 Jan 2026 06:48:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767941287; cv=none; b=WFOjnAB5oTwAY2MJdfkvcdgxrMYs90LtWIFzzK9g5KFHlPkTdGRdooyGQg7hoNMtqyHCN2AmxsCc9GF5JWD1WSISZgpv33vDp8nQX21k4Vl+LIU7Evkp4Y3ES3NIGnpJYZEoctlBKGu/RCJbl1A4cayr5H//L1Pq+BqqXgRjaxY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767941287; c=relaxed/simple; bh=1t7HpXtfhndr//EM310eF9nsZ7x3boyagBYp2aL6CmE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Sz3qaUMH9N0JiZQJIiGnI5CSsVtJLWpG2ReNUqodBiQXwlMBTK5NqzlBCEj8h59X26VMrxIgqWfgMc33lV5waVxEFrw3KB7uKpb0N2O0ECc0UKrFjc9bBUCvTpdHCcbLvFYy0kHyY7DYUp52yCF8RY7M8dCdaR5H1xJEI4sgqhs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=KHIvue3i; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=hNrLHS8B; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="KHIvue3i"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="hNrLHS8B" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1767941285; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=V/9K6fpSsNoEJIRz5SgfQXESe4sUZm/fKrYlDOxA0/w=; b=KHIvue3iDREjoahJArvBYcN3Ew+N3X2bPqVtqXUP2eVLqPfteITWjzr6cpKhccYrSm6rmC 7Uai9xJI3M672mCDryduHL5+JRr+MoxPLCHVfCfmeDSEQQVii1FICwdyyORIB0qsHxvbI5 5M7nZUBYhuTw9SPv10Vx6RJEZ6byDac= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-154-_RaARv32MC6C9JJyGKDBGg-1; Fri, 09 Jan 2026 01:48:03 -0500 X-MC-Unique: _RaARv32MC6C9JJyGKDBGg-1 X-Mimecast-MFC-AGG-ID: _RaARv32MC6C9JJyGKDBGg_1767941282 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-4775f51ce36so34860645e9.1 for ; Thu, 08 Jan 2026 22:48:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1767941282; x=1768546082; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=V/9K6fpSsNoEJIRz5SgfQXESe4sUZm/fKrYlDOxA0/w=; b=hNrLHS8Bbuz66rHhTSxrCNAH4Zit6MMh3RODkZR1hzQfuuKgvE3l9iDKSgDQEoMnF1 0kjuYV5W3vBAesBNSsC72Suh0QvYvUBzmmLniLcg2K+zkpeSzMfpm3i0yFU/Kb+2i4pj F0973yJ+rbBMeBnDLvW7a1iPDfJBg7+lUgCHwnrW8pvK8Sgns1EfgMlirhRPUI7HGEn/ 6QJtcK1y0NymSJH5iJ3W2qvx7svKAtHgrY0pcd/9yOM2nr0P4oUPVdti6RqqJ3/xjW6q Gm1SJfDf7CCSrbd1v47LUYvxjkkkDieRsknyocUaiO9U0jIWhBaav7+ZQ/baBw6An32B Qs0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767941282; x=1768546082; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=V/9K6fpSsNoEJIRz5SgfQXESe4sUZm/fKrYlDOxA0/w=; b=jFeQOGXFFgoMGJ/BWb8cc3roqJdfIlI77sGVWYES6K855B0c6NikQO8fjcnlT5t6JO /Ka8x2AwMpkat78NjT2dT8fgLs7CKApeTkyiXf4JUCiZuVoz287og+yY5pdpv2Hps+Zj Os5rwf8PXdEp5N0mUaUNRT/oF3xYdNAFcc0pkKsJQl+bGQ3e3vmGW7nzSsoBVzD4fCWg mN2zW++iPm26f/6itUsqnK8kdaoo3DFgNVc6DJIB0QqxXVjVqgM9ozMvkLF/imWkWJqR lhrQiuCpLE0VQA9c2jWQcGl1Y9YLOj2h9NQO/ODv2+nq0Pw7Q3+o0lR9qn15uPoExNAS qdfw== X-Forwarded-Encrypted: i=1; AJvYcCXiG22zKvwma0sHSmq9Ikx1fEpXddHVADMUdbOD/Ki7k9YYFAuoGfN/cF2sq2ml2IukFRMaWNDv/oA2cEo=@vger.kernel.org X-Gm-Message-State: AOJu0YzS3kkl0nrzQ9IsQ1A/e8uoIt8IfR/iiwEVEk+iUD9zeFY3p58C dlM4AQWqmlOfpkgXw/k3qz6FqnBCWILpnsfMVTKo1OcYCv6Y+alHvqEUUk5GS1Q1+jjEFYvLoy7 5U8kbAIib9PUi8BcXq9VnnndIt83T8/8n7nwiaTzXbX94/ct8D0EteP7VtMe/MHLpJg== X-Gm-Gg: AY/fxX5gJvY3wTK3DrVdfWPpYETb9aK7885r3EdyXN7ajunGRkmuPX4i1QXIFb5kP6t QQMDRMrPjNMOQVpyNJVzu08BD8UZWRtQUg9FxzHXyT+oXceHpAseoL61zl4FjNLwSyeRnL7ih6r doY2mUaP6VwgdeF/1EyfhfcnssH1pX4vYR9lXTAYQzsW3mvPS0bhfRhlXXWf03MI62QEM2S1xGq +iQCUi/qGNgiqz6lWv3xWBWccHGA5pcjEDJ8Ju7KrooZJzEZx5r9XAhqZx2R9yNCxzO5Kc/OdQv WBpxrd7IwuBh7LEjTCckWDVNHtNEb8G9OjxO8oGzqmWNDJiRVE4JuheNvAxfzOiOqNdGhH9SPXh IchfvdBiz6Fw+k5SBnkUMT/8X+/09CkXZ5w== X-Received: by 2002:a05:600c:83c9:b0:45d:5c71:769a with SMTP id 5b1f17b1804b1-47d84b3b650mr97118305e9.26.1767941281931; Thu, 08 Jan 2026 22:48:01 -0800 (PST) X-Google-Smtp-Source: AGHT+IE5mwMLnpPXtS3sSstkOI4XZ+lgxWp4YHE6MscmmctxPLldO2W4ddVXCOcgeyZ9c3lCcV9aiA== X-Received: by 2002:a05:600c:83c9:b0:45d:5c71:769a with SMTP id 5b1f17b1804b1-47d84b3b650mr97117985e9.26.1767941281330; Thu, 08 Jan 2026 22:48:01 -0800 (PST) Received: from redhat.com (IGLD-80-230-31-118.inter.net.il. [80.230.31.118]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-47d7f410c86sm194224835e9.3.2026.01.08.22.47.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Jan 2026 22:48:00 -0800 (PST) Date: Fri, 9 Jan 2026 01:47:57 -0500 From: "Michael S. Tsirkin" To: Jason Wang Cc: Simon Schippers , willemdebruijn.kernel@gmail.com, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, eperezma@redhat.com, leiyang@redhat.com, stephen@networkplumber.org, jon@nutanix.com, tim.gebauer@tu-dortmund.de, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, virtualization@lists.linux.dev Subject: Re: [PATCH net-next v7 2/9] ptr_ring: add helper to detect newly freed space on consume Message-ID: <20260109014322-mutt-send-email-mst@kernel.org> References: <20260107210448.37851-1-simon.schippers@tu-dortmund.de> <20260107210448.37851-3-simon.schippers@tu-dortmund.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Fri, Jan 09, 2026 at 02:01:54PM +0800, Jason Wang wrote: > On Thu, Jan 8, 2026 at 3:21 PM Simon Schippers > wrote: > > > > On 1/8/26 04:23, Jason Wang wrote: > > > On Thu, Jan 8, 2026 at 5:06 AM Simon Schippers > > > wrote: > > >> > > >> This proposed function checks whether __ptr_ring_zero_tail() was invoked > > >> within the last n calls to __ptr_ring_consume(), which indicates that new > > >> free space was created. Since __ptr_ring_zero_tail() moves the tail to > > >> the head - and no other function modifies either the head or the tail, > > >> aside from the wrap-around case described below - detecting such a > > >> movement is sufficient to detect the invocation of > > >> __ptr_ring_zero_tail(). > > >> > > >> The implementation detects this movement by checking whether the tail is > > >> at most n positions behind the head. If this condition holds, the shift > > >> of the tail to its current position must have occurred within the last n > > >> calls to __ptr_ring_consume(), indicating that __ptr_ring_zero_tail() was > > >> invoked and that new free space was created. > > >> > > >> This logic also correctly handles the wrap-around case in which > > >> __ptr_ring_zero_tail() is invoked and the head and the tail are reset > > >> to 0. Since this reset likewise moves the tail to the head, the same > > >> detection logic applies. > > >> > > >> Co-developed-by: Tim Gebauer > > >> Signed-off-by: Tim Gebauer > > >> Signed-off-by: Simon Schippers > > >> --- > > >> include/linux/ptr_ring.h | 13 +++++++++++++ > > >> 1 file changed, 13 insertions(+) > > >> > > >> diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h > > >> index a5a3fa4916d3..7cdae6d1d400 100644 > > >> --- a/include/linux/ptr_ring.h > > >> +++ b/include/linux/ptr_ring.h > > >> @@ -438,6 +438,19 @@ static inline int ptr_ring_consume_batched_bh(struct ptr_ring *r, > > >> return ret; > > >> } > > >> > > >> +/* Returns true if the consume of the last n elements has created space > > >> + * in the ring buffer (i.e., a new element can be produced). > > >> + * > > >> + * Note: Because of batching, a successful call to __ptr_ring_consume() / > > >> + * __ptr_ring_consume_batched() does not guarantee that the next call to > > >> + * __ptr_ring_produce() will succeed. > > > > > > This sounds like a bug that needs to be fixed, as it requires the user > > > to know the implementation details. For example, even if > > > __ptr_ring_consume_created_space() returns true, __ptr_ring_produce() > > > may still fail? > > > > No, it should not fail in that case. > > If you only call consume and after that try to produce, *then* it is > > likely to fail because __ptr_ring_zero_tail() is only invoked once per > > batch. > > Well, this makes the helper very hard for users. > > So I think at least the documentation should specify the meaning of > 'n' here. Right. Documenting parameters is good. > For example, is it the value returned by > ptr_ring_consume_batched()(), and is it required to be called > immediately after ptr_ring_consume_batched()? If it is, the API is > kind of tricky to be used, we should consider to merge two helpers > into a new single helper to ease the user. I think you are right partially it's more a question of documentation and naming. It's not that it's hard to use: follow up patches use it without issues - it's that neither documentatin nor naming explain how. let's try to document, first of all: if it does not guarantee that produce will succeed, then what *is* the guarantee this API gives? > > What's more, there would be false positives. Considering there's not > many entries in the ring, just after the first zeroing, > __ptr_ring_consume_created_space() will return true, this will lead to > unnecessary wakeups. well optimizations are judged on their performance not on theoretical analysis. in this instance, this should be rare enough. > > And last, the function will always succeed if n is greater than the batch. > > > > > > > > > Maybe revert fb9de9704775d? > > > > I disagree, as I consider this to be one of the key features of ptr_ring. > > Nope, it's just an optimization and actually it changes the behaviour > that might be noticed by the user. > > Before the patch, ptr_ring_produce() is guaranteed to succeed after a > ptr_ring_consume(). After that patch, it's not. We don't see complaint > because the implication is not obvious (e.g more packet dropping). > > > > > That said, there are several other implementation details that users need > > to be aware of. > > > > For example, __ptr_ring_empty() must only be called by the consumer. This > > was for example the issue in dc82a33297fc ("veth: apply qdisc > > backpressure on full ptr_ring to reduce TX drops") and the reason why > > 5442a9da6978 ("veth: more robust handing of race to avoid txq getting > > stuck") exists. > > At least the behaviour is documented. This is not the case for the > implications of fb9de9704775d. > > Thanks > > > > > > > > > >> + */ > > >> +static inline bool __ptr_ring_consume_created_space(struct ptr_ring *r, > > >> + int n) > > >> +{ > > >> + return r->consumer_head - r->consumer_tail < n; > > >> +} > > >> + > > >> /* Cast to structure type and call a function without discarding from FIFO. > > >> * Function must return a value. > > >> * Callers must take consumer_lock. > > >> -- > > >> 2.43.0 > > >> > > > > > > Thanks > > > > >