From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BBE8A1E98E0 for ; Sat, 12 Apr 2025 23:55:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=18.9.28.11 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744502155; cv=none; b=Ab4TYiddyxEEYAn9GNr+7ddyYxie/9pvzLihZK2buDc0EfIpp10epPcYIpdcOu9Nl69NH0Dyetewj5EIaGfY/i9zsSzoqfEl2U3W5BD6S79MRMpfNaq9VM6GB2X4GYF60Cm64jHNupGbB5zvCXeZiOfRKOIRSah2c4urVAu1wPg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744502155; c=relaxed/simple; bh=abXuAYgvwr1Ws5Hl6ZuttWEMtuZQf666Ofjlrrpz3Ak=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Rv68v4s2vBugCwRJl6LIj1etCiKT0nbOfYtuZJJCI8VUdLiLx3dfXX/oEusz30ONI6Ocv/W8RZhCVevhV2W52f06cpbimBzCUT0sSaTHFGlYfyIKO+05DO7TXxMyP49Uvwz/2+Dci2R0c5YujIHGAqfYh/3Da7m6Lg5Ted1dRek= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=mit.edu; spf=pass smtp.mailfrom=mit.edu; arc=none smtp.client-ip=18.9.28.11 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=mit.edu Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=mit.edu Received: from trampoline.thunk.org (pool-173-48-82-137.bstnma.fios.verizon.net [173.48.82.137]) (authenticated bits=0) (User authenticated as tytso@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 53CNtZA6010237 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 12 Apr 2025 19:55:36 -0400 Received: by trampoline.thunk.org (Postfix, from userid 15806) id 74D772E00E9; Sat, 12 Apr 2025 19:55:35 -0400 (EDT) Date: Sat, 12 Apr 2025 19:55:35 -0400 From: "Theodore Ts'o" To: Linus Torvalds Cc: Mateusz Guzik , Christian Brauner , Al Viro , linux-fsdevel , Jan Kara , Ext4 Developers List Subject: Re: generic_permission() optimization Message-ID: <20250412235535.GH13132@mit.edu> References: <20241031-klaglos-geldmangel-c0e7775d42a7@brauner> <20250412215257.GF13132@mit.edu> Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Sat, Apr 12, 2025 at 03:36:00PM -0700, Linus Torvalds wrote: > Indeed. I sent a query to the ext4 list (and I think you) about > whether my test was even the right one. Sorry, I must have not seen that message; at least, I don't have any memory of it. > Also, while I did a "getfattr -dR" to see if there are any *existing* > attributes (and couldn't find any), I also assume that if a file has > ever *had* any attributes, the filesystem may have the attribute block > allocated even if it's now empty. Well, getfattr will only show user xattrs. It won't show security.* xattr's that might have been set by SELinux, or a system.posix_acl_access xattr. > I assume there's some trivial e2fstools thing to show things like > that, but it needs more ext4 specific knowledge than I have. Yes, we can test for this using the debugfs command. For exaple: root@kvm-xfstests:~# debugfs /dev/vdc debugfs 1.47.2-rc1 (28-Nov-2024) debugfs: stat <13> Inode: 13 Type: regular Mode: 0644 Flags: 0x80000 Generation: 1672288850 Version: 0x00000000:00000003 User: 0 Group: 0 Project: 0 Size: 286 File ACL: 0 Links: 1 Blockcount: 8 Fragment: Address: 0 Number: 0 Size: 0 ctime: 0x67faf5d0:30d0b2e4 -- Sat Apr 12 19:22:56 2025 atime: 0x67faf571:7064bd50 -- Sat Apr 12 19:21:21 2025 mtime: 0x67faf571:71236aa8 -- Sat Apr 12 19:21:21 2025 crtime: 0x67faf571:7064bd50 -- Sat Apr 12 19:21:21 2025 Size of extra inode fields: 32 Extended attributes: system.posix_acl_access (28) = 01 00 00 00 01 00 06 00 02 00 04 00 b7 7a 00 00 04 00 04 00 10 00 04 00 20 00 04 00 Inode checksum: 0xc8f7f1a7 EXTENTS: (0):33792 (If you know the pathname instead of the inode number, you can also give that to debugfs's stat command, e.g., "stat /lost+found") I tested it with a simple variant of your patch, and seems to do the right thing. Mateusz, if you want, try the following patch, and then mount your test file system with "mount -o debug". (The test_opt is to avoid a huge amount of noise on your root file system; you can skip it if it's more trouble than it's worth.) The patch has a reversed seense of the test, so it will print a message for every one where cache_no_acl *wouldn't* be called. You casn then use debugfs's "stat " to verify whether it has some kind of extended attribute. - Ted diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index f386de8c12f6..3e0ba7c4723a 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -5109,6 +5109,11 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino, goto bad_inode; brelse(iloc.bh); + if (test_opt(sb, DEBUG) && + (ext4_test_inode_state(inode, EXT4_STATE_XATTR) || + ei->i_file_acl)) + ext4_msg(sb, KERN_DEBUG, "has xattr ino %lu", inode->i_ino); + unlock_new_inode(inode); return inode;