From patchwork Thu Oct 31 04:16:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 13857550 Received: from mail-ed1-f41.google.com (mail-ed1-f41.google.com [209.85.208.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 AFA1015D1 for ; Thu, 31 Oct 2024 04:16:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730348206; cv=none; b=XzPfq7xPogOfxWQhz8WzDuv9mTIWiPooHjsQdPROLFIxpgL5JJJOt++yEH3+KWnTEUiUKrISfsyBhAWXq65/0RlkEuBpRbTpmEDeoM55XKiDsiKQGnK2OQqqBluKXAaKQALMMqkVHhCij34+HGdsFM0XSTk6SCG+u8VssLkZ1vM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730348206; c=relaxed/simple; bh=xdTMB5mH53eRmMhNGiRJf3MOmUeu1DjlabGahGpGkSE=; h=MIME-Version:From:Date:Message-ID:Subject:To:Cc:Content-Type; b=RvinlMn2eDf3VtRvTomZShI38wz13Ih+Swvn646+y7Nc6WLWQ1nQyToS5RMPOD9OSGBY/5T0laRqtH6+oT8FCfe1w/bcrDrwQmN9t+pSbl1V3OHgJWXzizMbaxDPtO2IEwQo82PGspQcdkeuIps2snHRSeIn7qwB/BCD8MTLWA0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org; spf=pass smtp.mailfrom=linuxfoundation.org; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b=gnkUv/Dv; arc=none smtp.client-ip=209.85.208.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linuxfoundation.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b="gnkUv/Dv" Received: by mail-ed1-f41.google.com with SMTP id 4fb4d7f45d1cf-5cb6704ff6bso716170a12.3 for ; Wed, 30 Oct 2024 21:16:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; t=1730348202; x=1730953002; darn=vger.kernel.org; h=cc:to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=PjUUN+pUu7aAtkLIEjQSxN2sW69Zq1jbLWcrZXOkFzM=; b=gnkUv/Dv3W7AB9w3KaVhbQFDVp5QniC0X5qU0YrWM3sA5zPljXhlG5QI7G5HaEDFg6 LYc6b+GYydJxpZjooNpg7YviR079chvjxfOe1hSMHOxvatTzHi6cyZ9eGuH4CaFSc4tE IgQ+HgI3IRjSm4DLnjpEL8ThtTMff2dL77D3w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730348202; x=1730953002; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=PjUUN+pUu7aAtkLIEjQSxN2sW69Zq1jbLWcrZXOkFzM=; b=mW2XU/gBuXswn39+WL881XeA0bimupxpBZ7Q+pQ8DNYzgnpwcOmkDkssIX/yjlK/Uo Z/n7WMjfbMUgJ/yffdEZFnpt2D4UfNP8LHQbmzmCRq6ML4fUDzDdhZwy/pTvNRQZS34O 7kZPmxko79MezXNjsmwe3QQEE+DU6M+QZJXygnv3Fzj+ljT7c+k1Ph4XKueMSlsb2Ubj biP6G0khmxE4ySSiwN38uwAs3nHEcM6JKUxuy66c1ptAIUJHtES8KJuGgd/s/NY7Hw8V 6FBlQhs2R+pj5YL5VmtDnnaOi+fl7FvECvmCBJEVkZJ+N9EdUpz4sfnMgVeyHwBV0ePj XIyg== X-Gm-Message-State: AOJu0YzoxZpwG7rRZh7d6QxPyVu6BEfopGpIBSDdtbJiZA5vX36JTp2b dVKFJAzIp3lKfzeT3VwvhtumD+uFay0DFemP22ASJrZUq7D8k0deZ9Tu0RaqsRzAuAhfOa7F9SA jBlg= X-Google-Smtp-Source: AGHT+IHxWXc7/f6u20FNK+fL9s5pFbGyulbs+GzYw6kALANTDEMrF19p/jryOnmFyzJ6wak+VoRtyg== X-Received: by 2002:a17:906:f5a6:b0:a9a:f84:fefd with SMTP id a640c23a62f3a-a9de5edbf04mr1752209866b.36.1730348201554; Wed, 30 Oct 2024 21:16:41 -0700 (PDT) Received: from mail-ej1-f44.google.com (mail-ej1-f44.google.com. [209.85.218.44]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a9e564c5348sm23822766b.49.2024.10.30.21.16.39 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 30 Oct 2024 21:16:40 -0700 (PDT) Received: by mail-ej1-f44.google.com with SMTP id a640c23a62f3a-a93c1cc74fdso59471366b.3 for ; Wed, 30 Oct 2024 21:16:39 -0700 (PDT) X-Received: by 2002:a17:907:6d09:b0:a99:f887:ec09 with SMTP id a640c23a62f3a-a9de5ecade8mr1642383366b.35.1730348199083; Wed, 30 Oct 2024 21:16:39 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Linus Torvalds Date: Wed, 30 Oct 2024 18:16:22 -1000 X-Gmail-Original-Message-ID: Message-ID: Subject: generic_permission() optimization To: Christian Brauner , Al Viro Cc: linux-fsdevel So call me crazy, but due to entirely unrelated changes (the x86 barrier_nospec optimization) I was doing some profiles to check that everything looked fine. And just looking at kernel profiles, I noticed that "generic_permission()" wasn't entirely in the noise. It was right there along with strncpy_from_user() etc. Which is a bit surprising, because it should be really cheap to check basic Unix permissions? It's all really just "acl_permission_check()" and that code is actually fairly optimized, except that the whole vfsuid = i_uid_into_vfsuid(idmap, inode); to check whether we're the owner is *not* cheap. It causes a call to make_vfsuid(), and it's just messy. Which made me wonder: we already have code that says "if the Group and Other permission bits are the same wrt the mask we are checking, don't bother doing the expensive group checks". Why not extend that to "if any of the UGO choices are ok with the permissions, why bother looking up ownership at all?" Now, there is one reason to look up the owner: POSIX ACL's don't matter to owners, but do matter to others. But we could check for the cached case of "no POSIX ACLs" very cheaply, and only do it for that case. IOW, a patch something like the attached. No, I didn't really check whether it makes any difference at all. But my gut feel is that a *lot* of permission checks succeed for any user, with things like system directories are commonly drwxr-xr-x, so if you want to check read or execute permissions, it really doesn't matter whether you are the User, the Group, or Other. So thus the code does that unsigned int all; all = mode & (mode >> 3); // combine g into o all &= mode >> 6; // ... and u into o so now the low three bits of 'all' are the bits that *every* case has set. And then if (!(mask & ~all & 7)) return 0; basically says "if what we are asking for is not zero in any of those bits, we're good". And it's entirely possible that I'm missing something silly and am being just stupid. Somebody hit me with the clue bat if so. Linus Acked-by: Al Viro fs/namei.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/fs/namei.c b/fs/namei.c index 4a4a22a08ac2..6aeabde0ec9f 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -326,6 +326,25 @@ static int check_acl(struct mnt_idmap *idmap, return -EAGAIN; } +/* + * Very quick optimistic "we know we have no ACL's" check. + * + * Note that this is purely for ACL_TYPE_ACCESS, and purely + * for the "we have cached that there are no ACLs" case. + * + * If this returns true, we know there are no ACLs. But if + * it returns false, we might still not have ACLs (it could + * be the is_uncached_acl() case). + */ +static inline bool no_acl_inode(struct inode *inode) +{ +#ifdef CONFIG_FS_POSIX_ACL + return likely(!READ_ONCE(inode->i_acl)); +#else + return true; +#endif +} + /** * acl_permission_check - perform basic UNIX permission checking * @idmap: idmap of the mount the inode was found from @@ -348,6 +367,19 @@ static int acl_permission_check(struct mnt_idmap *idmap, unsigned int mode = inode->i_mode; vfsuid_t vfsuid; + /* + * Common cheap case: everybody has the requested + * rights, and there are no ACLs to check. No need + * to do any owner/group checks. + */ + if (no_acl_inode(inode)) { + unsigned int all; + all = mode & (mode >> 3); // combine g into o + all &= mode >> 6; // ... and u into o + if (!(mask & ~all & 7)) + return 0; + } + /* Are we the owner? If so, ACL's don't matter */ vfsuid = i_uid_into_vfsuid(idmap, inode); if (likely(vfsuid_eq_kuid(vfsuid, current_fsuid()))) {