From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from shards.monkeyblade.net ([184.105.139.130]:57956 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932699AbeCEVe3 (ORCPT ); Mon, 5 Mar 2018 16:34:29 -0500 Date: Mon, 05 Mar 2018 16:34:28 -0500 (EST) Message-Id: <20180305.163428.1811826895016433265.davem@davemloft.net> To: john.fastabend@gmail.com Cc: ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org, davejwatson@fb.com Subject: Re: [bpf-next PATCH 02/16] sockmap: convert refcnt to an atomic refcnt From: David Miller In-Reply-To: <20180305195106.6612.55622.stgit@john-Precision-Tower-5810> References: <20180305194616.6612.36343.stgit@john-Precision-Tower-5810> <20180305195106.6612.55622.stgit@john-Precision-Tower-5810> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: From: John Fastabend Date: Mon, 05 Mar 2018 11:51:06 -0800 > The sockmap refcnt up until now has been wrapped in the > sk_callback_lock(). So its not actually needed any locking of its > own. The counter itself tracks the lifetime of the psock object. > Sockets in a sockmap have a lifetime that is independent of the > map they are part of. This is possible because a single socket may > be in multiple maps. When this happens we can only release the > psock data associated with the socket when the refcnt reaches > zero. There are three possible delete sock reference decrement > paths first through the normal sockmap process, the user deletes > the socket from the map. Second the map is removed and all sockets > in the map are removed, delete path is similar to case 1. The third > case is an asyncronous socket event such as a closing the socket. The > last case handles removing sockets that are no longer available. > For completeness, although inc does not pose any problems in this > patch series, the inc case only happens when a psock is added to a > map. > > Next we plan to add another socket prog type to handle policy and > monitoring on the TX path. When we do this however we will need to > keep a reference count open across the sendmsg/sendpage call and > holding the sk_callback_lock() here (on every send) seems less than > ideal, also it may sleep in cases where we hit memory pressure. > Instead of dealing with these issues in some clever way simply make > the reference counting a refcnt_t type and do proper atomic ops. > > Signed-off-by: John Fastabend Acked-by: David S. Miller