Skip to content

cherry-pick statx_permcheck for rhel9.6#157

Merged
hbirth merged 2 commits into
DDNStorage:redfs-rhel9_6-570.12.1from
cding-ddn:statx_permcheck-redfs-rhel9_6-570.12.1
May 25, 2026
Merged

cherry-pick statx_permcheck for rhel9.6#157
hbirth merged 2 commits into
DDNStorage:redfs-rhel9_6-570.12.1from
cding-ddn:statx_permcheck-redfs-rhel9_6-570.12.1

Conversation

@cding-ddn
Copy link
Copy Markdown
Collaborator

No description provided.

@cding-ddn cding-ddn requested review from bsbernd and hbirth May 25, 2026 08:39
@hbirth
Copy link
Copy Markdown
Collaborator

hbirth commented May 25, 2026

looks like this has some compile issues

fs/fuse/inode.c: In function 'fuse_change_attributes_common_sx':
fs/fuse/inode.c:287:17: error: implicit declaration of function 'inode_set_atime'; did you mean 'inode_set_ctime'? [-Werror=implicit-function-declaration]
287 | inode_set_atime(inode, attr->atime, attr->atimensec);
| ^~~~~~~~~~~~~~~
| inode_set_ctime
fs/fuse/inode.c:292:17: error: implicit declaration of function 'inode_set_mtime'; did you mean 'inode_set_ctime'? [-Werror=implicit-function-declaration]
292 | inode_set_mtime(inode, attr->mtime, attr->mtimensec);
| ^~~~~~~~~~~~~~~
| inode_set_ctime
CC [M] fs/smb/client/link.o
cc1: all warnings being treated as errors
make[2]: *** [scripts/Makefile.build:249: fs/fuse/inode.o] Error 1
make[1]: *** [scripts/Makefile.build:478: fs/fuse] Error 2
make[1]: *** Waiting for unfinished jobs....

bsbernd added 2 commits May 25, 2026 09:53
In preparation for allowing partial attribute updates via statx
(where only a subset of STATX_BASIC_STATS may be requested and
returned), modify fuse_change_attributes_common() to be more
selective about what it updates.

Currently, fuse_change_attributes_common() unconditionally:
1. Clears ALL of STATX_BASIC_STATS from inval_mask
2. Updates fi->i_time (extending the attribute timeout)

For some fuse-servers it might be benefitial to reduce the number
of queries attributes and and attribute mask is one of the
features of statx. With the all or nothing handling
of fuse_change_attributes_common() that statx feature is
impossible to be used.

This commit adds the logic to:
1. Track which attributes were actually returned (via sx->mask for
   statx, or assume all STATX_BASIC_STATS for getattr)
2. Only clear those specific attributes from inval_mask
3. Only update fi->i_time when it's safe: when cache_mask is empty
   OR when all cache_mask attributes were included in the response

The condition in fuse_do_statx() still requires ALL STATX_BASIC_STATS,
so this commit has no functional change. A follow up commit will relax
that condition to enable partial updates.

Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
(cherry picked from commit 8034f26)
…attributes

For permission checks via inode_permission(), we only need mode, uid, and
gid attributes. Previously, we requested all STATX_BASIC_STATS, which was
inefficient.

This commit enables the optimization by:
1. Requesting only STATX_MODE | STATX_UID | STATX_GID for permission checks
2. Relaxing the condition in fuse_do_statx() from requiring all basic stats
   to accepting any subset of basic stats
3. Adding validation that the server returns at least what was requested

The preparation commit ensures partial updates work correctly by only
updating returned attributes and managing timeouts appropriately.

Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
(cherry picked from commit 09ed47b)
@cding-ddn cding-ddn force-pushed the statx_permcheck-redfs-rhel9_6-570.12.1 branch from 91e3a26 to f8a66b0 Compare May 25, 2026 09:53
Copy link
Copy Markdown
Collaborator

@hbirth hbirth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ... this is a cherry pick

@hbirth hbirth merged commit 7fcc07e into DDNStorage:redfs-rhel9_6-570.12.1 May 25, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants