From patchwork Thu Feb 27 21:17:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Simmons X-Patchwork-Id: 11410835 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8450C924 for ; Thu, 27 Feb 2020 21:47:58 +0000 (UTC) Received: from pdx1-mailman02.dreamhost.com (pdx1-mailman02.dreamhost.com [64.90.62.194]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6CDEA24690 for ; Thu, 27 Feb 2020 21:47:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6CDEA24690 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lustre-devel-bounces@lists.lustre.org Received: from pdx1-mailman02.dreamhost.com (localhost [IPv6:::1]) by pdx1-mailman02.dreamhost.com (Postfix) with ESMTP id 82BD834B560; Thu, 27 Feb 2020 13:38:06 -0800 (PST) X-Original-To: lustre-devel@lists.lustre.org Delivered-To: lustre-devel-lustre.org@pdx1-mailman02.dreamhost.com Received: from smtp3.ccs.ornl.gov (smtp3.ccs.ornl.gov [160.91.203.39]) by pdx1-mailman02.dreamhost.com (Postfix) with ESMTP id 552063489F6 for ; Thu, 27 Feb 2020 13:21:26 -0800 (PST) Received: from star.ccs.ornl.gov (star.ccs.ornl.gov [160.91.202.134]) by smtp3.ccs.ornl.gov (Postfix) with ESMTP id 57998A154; Thu, 27 Feb 2020 16:18:20 -0500 (EST) Received: by star.ccs.ornl.gov (Postfix, from userid 2004) id 55F0246A; Thu, 27 Feb 2020 16:18:20 -0500 (EST) From: James Simmons To: Andreas Dilger , Oleg Drokin , NeilBrown Date: Thu, 27 Feb 2020 16:17:48 -0500 Message-Id: <1582838290-17243-601-git-send-email-jsimmons@infradead.org> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1582838290-17243-1-git-send-email-jsimmons@infradead.org> References: <1582838290-17243-1-git-send-email-jsimmons@infradead.org> Subject: [lustre-devel] [PATCH 600/622] lustre: handle: remove locking from class_handle2object() X-BeenThere: lustre-devel@lists.lustre.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "For discussing Lustre software development." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lustre Development List MIME-Version: 1.0 Errors-To: lustre-devel-bounces@lists.lustre.org Sender: "lustre-devel" From: Mr NeilBrown There is limited value in this locking and test on h_in. If the lookup could have run in parallel with class_handle_unhash_nolock() and seen "h_in == 0", then it could equally well have run moments earlier and not seen it - no locking would prevent that, so the caller much be prepared to have an object returned which has already been unhashed by the time it sees the object. In other words, any interlock between unhash and lookup must be provided at a higher level than where this code is trying to handle it. The locking *does* prevent the refcount from being incremented if the object has already been removed from the list. As the final reference is always dropped after that removal, it indirectly stops the refcount from being incremented after the final reference is dropped. This can be more directly achieved by using refcount_inc_not_zero(). So remove the locking, and replace it with refcount_inc_not_zero(). WC-bug-id: https://jira.whamcloud.com/browse/LU-12542 Lustre-commit: e2458a94a6a2 ("LU-12542 handle: remove locking from class_handle2object()") Signed-off-by: Mr NeilBrown Reviewed-on: https://review.whamcloud.com/35861 Reviewed-by: Mike Pershin Reviewed-by: Petros Koutoupis Reviewed-by: Oleg Drokin Signed-off-by: James Simmons --- fs/lustre/obdclass/lustre_handles.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fs/lustre/obdclass/lustre_handles.c b/fs/lustre/obdclass/lustre_handles.c index 6989a60..acee2db 100644 --- a/fs/lustre/obdclass/lustre_handles.c +++ b/fs/lustre/obdclass/lustre_handles.c @@ -149,15 +149,12 @@ void *class_handle2object(u64 cookie, const char *owner) if (h->h_cookie != cookie || h->h_owner != owner) continue; - spin_lock(&h->h_lock); - if (likely(h->h_in != 0)) { - refcount_inc(&h->h_ref); + if (refcount_inc_not_zero(&h->h_ref)) { CDEBUG(D_INFO, "GET %s %p refcount=%d\n", h->h_owner, h, refcount_read(&h->h_ref)); retval = h; } - spin_unlock(&h->h_lock); break; } rcu_read_unlock();