From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f41.google.com (mail-ej1-f41.google.com [209.85.218.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 9FAE3144D21 for ; Tue, 31 Dec 2024 07:31:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735630294; cv=none; b=p9PEvy7cJDZru2htMGYORWQwHS2kOe0QTalkIVG56WeZMnmxfcDFh3kcFFjflLXI0UwRUkX6zB52QYyl4FLSXoF1boW12KcdCfYEj+s/1QAXW+IgqbhbRI3aRm86v9wiZaWgJjqUhbQOELL3o/wzFB9Exh6JWv510EMeR8RUPzE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735630294; c=relaxed/simple; bh=kocXQ4vX+fsyfMpu7cj2L4odexaq1pRgsS3a37FCZPU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Ts7ru7wDH+qToPRZz/0h4L5ci+nBs3EmMRquKipdezjMpII1vKjwX3In9CUMIAO2WFdYTGjCTCndiEAxh7wfRPmcu3isd2Xm2R5qr4XU1iy8ujL56xkdU+sTr5NYbtPQi+A3m3zZt/onMl0qOhvliD8jR+WJ6ncR2gS024XE5Qc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=openvpn.net; spf=pass smtp.mailfrom=openvpn.com; dkim=pass (2048-bit key) header.d=openvpn.net header.i=@openvpn.net header.b=B9Ex07Og; arc=none smtp.client-ip=209.85.218.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=openvpn.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=openvpn.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=openvpn.net header.i=@openvpn.net header.b="B9Ex07Og" Received: by mail-ej1-f41.google.com with SMTP id a640c23a62f3a-aa69107179cso1613490866b.0 for ; Mon, 30 Dec 2024 23:31:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1735630291; x=1736235091; 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=DeVSalQ53GlGqy7ywxW1+UN+SYgePVnFipyEE8iNzLY=; b=B9Ex07Og40TbfgpL1KITMRL1YdLSCT4d50slIwAadXQnhibVAaJx3eR5fkQw/10/LP MqXIBiidZ9VY2K0qJsVz8FLKXAV70it1bxODZKGIGWjbWqUbuJDSuXZO9zV1qfj2damC jD+stPsFE5lOibJqa6HcP6EQ5dIprd/VUWacs5S7k1EfOrgs8/g6na5Fvskeb9daI4+k FIhG95WPZl7LdpE3P80R8vKYJ9dVNd01KYzeu4mMektdIauYojpKPJq0BmF5L6R3RLnC utN0lJ6jLcI4xX1+iDwZFlQ8DfBkcHWP1HcKAByGsP0/caBZ2nq44vhmO4oKHulIYtle 5W1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1735630291; x=1736235091; 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=DeVSalQ53GlGqy7ywxW1+UN+SYgePVnFipyEE8iNzLY=; b=CSM3uuOXMFPdNQrjTiwKp1Sy1uwZqW1RqQpbFsEtpHJJJonRIZ/FzjPH1zVyYdlB+D UVZ/M0BTK189VTZ+lbMcnqJv2jMJbNO+uUDOL+szmr0TnsYrbdwat1m9/tD6emf1XBXH ixE+2NLfGXS7weJV6TAMusy73MmNfXOFySrdzX8QJyWQIl00EqQMgL5qG6g3D7Lu32hO 8Nb4esSH9Cb84nFfHEpBf2TmOEfcY17kn7Wtjcg5ouVbMYVzEpxYuUPVfAv7EpAg1bOP vcKvo8NzeKjY2iORPIUw/Rj18kLDy2orVAnMLF15OcE0fhQpv7FFwGpz6fGafminw7NG bJFA== X-Forwarded-Encrypted: i=1; AJvYcCXXRxb3UpAEJI1OFhBhLli5pwYX/j6uqauuN57yb0OnO3Qwjo+jlkHcWoztgZazfa6sYTAsTxFC/r5J7ZI=@vger.kernel.org X-Gm-Message-State: AOJu0YwgUZpGZp9oap9modfk7YkQdJAlSDwDJJxfVdNFgJ1zZWf2iOrN 11zfj82XEa05toVNtwKIxBdyAYrlk4MtnHmSvkFtnH+PsaWuO0mCAU0uTEx+hhE= X-Gm-Gg: ASbGncuU4o6r7hOULsKn3hGOVzWC9zEzKFqN8CuyGpMz6MF4SQcVXG1wOjBKupAREzw LayXezimatbaBuU6sKEnkz7MQfGi3Lr4O0aXNhkLUir8lNYIHeGEBBsjd8BKlpmAOAPUIG3AuRD E6UXIZr/+43mDIeJtASsnTDt7oRGYSeVhe1pLTE9+JXW3htPjYett1iFM3g/9uvKhK0Zg6N1vmU D0xPccpDJCrAQra+BHZdXFfTe7hfsjP/N1FrYNANEWGnNPDsRUhR053YiXMXT2UoA== X-Google-Smtp-Source: AGHT+IHixdQps3Wsr2NFRaJfLKeVKOF4fumaMfhFlnKa/4hFejpWrGXeJUEKHnsQKRjjxtjT5Q2Raw== X-Received: by 2002:a17:907:7f8e:b0:aa6:9198:75a2 with SMTP id a640c23a62f3a-aac334e51afmr3388350766b.44.1735630290965; Mon, 30 Dec 2024 23:31:30 -0800 (PST) Received: from ?IPV6:2001:67c:2fbc:1000::1001? ([2001:67c:2fbc:1000::1001]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-aaf57c2e24esm246326566b.205.2024.12.30.23.31.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 30 Dec 2024 23:31:29 -0800 (PST) Message-ID: <12b0dab0-600b-4d0b-8b0c-881d92d8ae7c@openvpn.net> Date: Tue, 31 Dec 2024 08:31:27 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net-next v16 06/26] kref/refcount: implement kref_put_sock() To: Will Deacon Cc: netdev@vger.kernel.org, Eric Dumazet , Jakub Kicinski , Paolo Abeni , Donald Hunter , Shuah Khan , sd@queasysnail.net, ryazanov.s.a@gmail.com, Andrew Lunn , Simon Horman , linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, Xiao Liang , Peter Zijlstra , Boqun Feng , Mark Rutland , Andrew Morton References: <20241219-b4-ovpn-v16-0-3e3001153683@openvpn.net> <20241219-b4-ovpn-v16-6-3e3001153683@openvpn.net> <20241219172040.GA25368@willie-the-truck> Content-Language: en-US From: Antonio Quartulli In-Reply-To: <20241219172040.GA25368@willie-the-truck> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Will, Thanks a lot for chiming in and sorry for the delay. See below. On 19/12/24 18:20, Will Deacon wrote: [...] >> +/** >> + * refcount_dec_and_lock_sock - return holding locked sock if able to decrement >> + * refcount to 0 >> + * @r: the refcount >> + * @sock: the sock to be locked >> + * >> + * Similar to atomic_dec_and_lock(), it will WARN on underflow and fail to >> + * decrement when saturated at REFCOUNT_SATURATED. >> + * >> + * Provides release memory ordering, such that prior loads and stores are done >> + * before, and provides a control dependency such that free() must come after. >> + * See the comment on top. >> + * >> + * Return: true and hold sock if able to decrement refcount to 0, false >> + * otherwise >> + */ >> +bool refcount_dec_and_lock_sock(refcount_t *r, struct sock *sock) >> +{ >> + if (refcount_dec_not_one(r)) >> + return false; >> + >> + bh_lock_sock(sock); >> + if (!refcount_dec_and_test(r)) { >> + bh_unlock_sock(sock); >> + return false; >> + } >> + >> + return true; >> +} >> +EXPORT_SYMBOL(refcount_dec_and_lock_sock); > > It feels a little out-of-place to me having socket-specific functions in > lib/refcount.c. I'd suggest sticking this somewhere else _or_ maybe we > could generate this pattern of code: > > #define REFCOUNT_DEC_AND_LOCKNAME(lockname, locktype, lock, unlock) \ > static __always_inline \ > bool refcount_dec_and_lock_##lockname(refcount_t *r, locktype *l) \ > { \ > ... > > inside a generator macro in refcount.h, like we do for seqlocks in > linux/seqlock.h. The downside of that is the cost of inlining. Does your suggestion imply that I should convert already existing functions to this macro? In that case I believe the change would be too invasive and other devs may not like the inlining, as you pointed out. Secondly, I thought about moving this function to net/core/sock.c, but if you look at it, its logic is mostly about refcounting, with a little touch of sock. Hence, sock.c (or any other net file) does not seem appropriate either. I guess for the time being it is more convenient to keep this function, and is kref counterpart, inside 'ovpn'. They can be moved to better spots once they gained another user. Best Regards, -- Antonio Quartulli OpenVPN Inc.