From patchwork Fri Mar 2 19:53:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 10255647 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8E9B56037F for ; Fri, 2 Mar 2018 19:54:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 80B102845E for ; Fri, 2 Mar 2018 19:54:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 741A02846C; Fri, 2 Mar 2018 19:54:38 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 048F02845E for ; Fri, 2 Mar 2018 19:54:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932401AbeCBTyY (ORCPT ); Fri, 2 Mar 2018 14:54:24 -0500 Received: from out01.mta.xmission.com ([166.70.13.231]:38724 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932387AbeCBTyX (ORCPT ); Fri, 2 Mar 2018 14:54:23 -0500 Received: from in02.mta.xmission.com ([166.70.13.52]) by out01.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.87) (envelope-from ) id 1erql3-0006rN-QM; Fri, 02 Mar 2018 12:54:21 -0700 Received: from 174-19-85-160.omah.qwest.net ([174.19.85.160] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.87) (envelope-from ) id 1erql3-0005vv-3M; Fri, 02 Mar 2018 12:54:21 -0700 From: ebiederm@xmission.com (Eric W. Biederman) To: Linus Torvalds Cc: Miklos Szeredi , Linux Kernel Mailing List , Linux Containers , linux-fsdevel , Alban Crequy , Seth Forshee , Sargun Dhillon , Dongsu Park , "Serge E. Hallyn" References: <87po4rz4ui.fsf_-_@xmission.com> <20180226235302.12708-3-ebiederm@xmission.com> <87r2p7rvn5.fsf@xmission.com> <87tvu3qg2b.fsf@xmission.com> Date: Fri, 02 Mar 2018 13:53:45 -0600 In-Reply-To: (Linus Torvalds's message of "Mon, 26 Feb 2018 19:41:21 -0800") Message-ID: <87r2p2b6eu.fsf_-_@xmission.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1erql3-0005vv-3M; ; ; mid=<87r2p2b6eu.fsf_-_@xmission.com>; ; ; hst=in02.mta.xmission.com; ; ; ip=174.19.85.160; ; ; frm=ebiederm@xmission.com; ; ; spf=neutral X-XM-AID: U2FsdGVkX1/4t4hSeajxuYNMhrp57GHLiqpesI6YFTw= X-SA-Exim-Connect-IP: 174.19.85.160 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: [RFC][PATCH] fs/posix_acl: Update the comments and support lightweight cache skipping X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The code has been missing a way for a ->get_acl method to not cache a return value without risking invalidating a cached value that was set while get_acl() was returning. Add that support by implementing to_uncachable_acl, to_cachable_acl, is_uncacheable_acl, and dealing with uncachable acls in get_acl(). Update the comments so that they are a little clearer about what is going on in get_acl() Signed-off-by: "Eric W. Biederman" --- Linus my issue with the forget_cached_acl case was really that it was too big of a hammer. If you care about caching acls only somtimes forget_cached_acl called from ->get_acl can stomp that acl you explicitly cached with set_cached_acl. With this change I can unify the legacy horrible fuse posix acl case that requires not caching acls with a single if statement in the get_acl method. AKA: + if (!IS_ERR(acl) && !fc->posix_acl) + acl = to_uncacheable_acl(acl); return acl; That code I know is locally correct even if later fuse decides to cache negative acls when the underlying filesystem does not support xattrs. fs/posix_acl.c | 56 ++++++++++++++++++++++++++++++++++------------- include/linux/posix_acl.h | 17 ++++++++++++++ 2 files changed, 58 insertions(+), 15 deletions(-) diff --git a/fs/posix_acl.c b/fs/posix_acl.c index 2fd0fde16fe1..e58a68e18603 100644 --- a/fs/posix_acl.c +++ b/fs/posix_acl.c @@ -96,12 +96,16 @@ struct posix_acl *get_acl(struct inode *inode, int type) { void *sentinel; struct posix_acl **p; - struct posix_acl *acl; + struct posix_acl *acl, *to_cache; /* * The sentinel is used to detect when another operation like * set_cached_acl() or forget_cached_acl() races with get_acl(). * It is guaranteed that is_uncached_acl(sentinel) is true. + * + * This is sufficient to prevent races between ->set_acl + * calling set_cached_acl (outside of filesystem specific + * locking) and get_acl() caching the returned acl. */ acl = get_cached_acl(inode, type); @@ -126,12 +130,18 @@ struct posix_acl *get_acl(struct inode *inode, int type) /* fall through */ ; /* - * Normally, the ACL returned by ->get_acl will be cached. - * A filesystem can prevent that by calling - * forget_cached_acl(inode, type) in ->get_acl. + * Normally, the ACL returned by ->get_acl() will be cached. + * + * A filesystem can prevent the acl returned by ->get_acl() + * from being cached by wrapping it with to_uncachable_acl(). + * + * A filesystem can at anytime effect the cache directly and + * cause in process calls of get_acl() not to update the cache + * by calling forget_cache_acl(inode, type) or + * set_cached_acl(inode, type, acl). * - * If the filesystem doesn't have a get_acl() function at all, we'll - * just create the negative cache entry. + * If the filesystem doesn't have a ->get_acl() function at + * all, we'll just create the negative cache entry. */ if (!inode->i_op->get_acl) { set_cached_acl(inode, type, NULL); @@ -139,21 +149,37 @@ struct posix_acl *get_acl(struct inode *inode, int type) } acl = inode->i_op->get_acl(inode, type); + + /* To keep the logic simple default to not caching an acl when + * the sentinel is cleared. + */ + to_cache = ACL_NOT_CACHED; if (IS_ERR(acl)) { - /* - * Remove our sentinel so that we don't block future attempts - * to cache the ACL. + /* Clears the sentinel so that we don't block future + * attempts to cache the ACL, and return an error. */ - cmpxchg(p, sentinel, ACL_NOT_CACHED); - return acl; + } + else if (is_uncacheable_acl(acl)) { + /* Clears the sentinel so that we don't block future + * attempts to cache the ACL, and return a valid ACL. + */ + acl = to_cacheable_acl(acl); + } + else { + to_cache = acl; + posix_acl_dup(to_cache); } /* - * Cache the result, but only if our sentinel is still in place. + * Remove the sentinel and replace it with the value that + * needs to be cached, but only if the sentinel is still in + * place. */ - posix_acl_dup(acl); - if (unlikely(cmpxchg(p, sentinel, acl) != sentinel)) - posix_acl_release(acl); + if (unlikely(cmpxchg(p, sentinel, to_cache) != sentinel)) { + if (!is_uncached_acl(to_cache)) + posix_acl_release(to_cache); + } + return acl; } EXPORT_SYMBOL(get_acl); diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h index 540595a321a7..3be8929b9f48 100644 --- a/include/linux/posix_acl.h +++ b/include/linux/posix_acl.h @@ -56,6 +56,23 @@ posix_acl_release(struct posix_acl *acl) kfree_rcu(acl, a_rcu); } +/* + * Allow for acls returned from ->get_acl() to not be cached. + */ +static inline bool is_uncacheable_acl(struct posix_acl *acl) +{ + return ((unsigned long)acl) & 1UL; +} + +static inline struct posix_acl *to_uncacheable_acl(struct posix_acl *acl) +{ + return (struct posix_acl *)(((unsigned long)acl) | 1UL); +} + +static inline struct posix_acl *to_cacheable_acl(struct posix_acl *acl) +{ + return (struct posix_acl *)(((unsigned long)acl) & ~1UL); +} /* posix_acl.c */