Details
-
Improvement
-
Resolution: Unresolved
-
Minor
-
None
-
None
-
3
-
9223372036854775807
Description
It should be possible to cache negative xattr entries (i.e. xattr names that do not exist) for an inode on the client if it has the MDS_INODELOCK_XATTR lock locally, since a new xattr cannot be created on the inode without the MDS cancelling this lock.
There are a few xattrs that are handled differently. See ll_xattr_cache_refill():
- XATTR_NAME_ACL_ACCESS is cached under the MDS_INODELOCK_LOOKUP lock since it is permission related
- XATTR_NAME_SOM shouldn't be cached since it may change without a lock
- XATTR_NAME_SELINUX, XATTR_NAME_SMACK are cached in the kernel already since they shouldn't be modified for files after creation
The negative cached ll_xattr_entry could store "xe_value = ERR_PTR(-ENODATA)" (or presumably whatever other error was returned for the MDS_GETXATTR for that xattr name to indicate the xattr doesn't exist.
There is also a discussion in LU-10276 to store some xattrs under a different DLM lock, which would cause problems for using listxattr() for cached xattrs that are not actually in the client cache. In such cases it would be possible to store "xe_value = ERR_PTR(-EREMOTE)" in the xattr cache, so that they still appear in the listxattr() output but the values are not actually stored in the cache.
It appears that the lack of some xattrs in the client cache is already a problem for listxattr() today, since security.selinux is not being listed on the client, even though it is stored on the inode:
# getfattr -d -m - -e hex /myth/tmp getfattr: Removing leading '/' from absolute path names # file: myth/tmp trusted.link=0xdff1ea11010000002d000000000000000000000000000000001500000000002080011c08b68000000000746d70 trusted.lma=0x0000000000000000625f030002000000b07f000000000000 # debugfs -c -R "stat ROOT/tmp" /dev/vgmyth/lvmythmdt0.ssd debugfs 1.47.0-wc4 (22-Jun-2023) : Extended attributes: lma: fid=[0x200035f62:0x7fb0:0x0] compat=0 incompat=0 security.selinux (37) = "unconfined_u:object_r:unlabeled_t:s0\000" linkea: idx=0 parent=[0x208001:0x1c08b680:0x0] name='tmp'
For the security.selinux xattr this isn't a huge problem, since most applications will just request the xattr without first listing if it exists, but it should properly be listed when requested (e.g for backups and similar).