Re: [PATCH v2] common/rc: support f2fs in _require_fanotify_ioerrors()

[Chao Yu <chao@kernel.org>]

On 7/17/26 18:47, Zorro Lang wrote:
> On Wed, Jul 08, 2026 at 11:25:37AM +0800, Chao Yu wrote:
>> On 7/7/26 18:13, Anand Suveer Jain wrote:
>>> On 6/7/26 14:43, Chao Yu wrote:
>>>> f2fs has supported to report fserror, this patch adds support for f2fs
>>>> in _require_fanotify_ioerrors(), so that we can run generic/791 on f2fs.
>>>>
>>>> Cc: Darrick J. Wong <djwong@kernel.org>
>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>> ---
>>>>  common/rc | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/common/rc b/common/rc
>>>> index 494725547..f51ad59f3 100644
>>>> --- a/common/rc
>>>> +++ b/common/rc
>>>> @@ -6257,6 +6257,9 @@ _require_fanotify_ioerrors()
>>>>  		x="$(_get_fs_sysfs_attr $TEST_DEV err_report_sec)"
>>>>  		test -n "$x" && return 0
>>>>  		;;
>>>
>>>
>>>> +	f2fs)
>>>> +		x="$(cat /sys/fs/f2fs/features/fserror)"
>>>> +		test -n "$x" && return 0
>>>
>>> Don't we need to check if /sys/fs/f2fs/features/fserror
>>> is present in the running kernel (there are helpers)
>>
>> Hi Anand,
>>
>> You mean _has_fs_sysfs_attr()? however, it forces to check first parameter $dev
>> w/ block device type:
>>
>> 	if [ ! -b "$dev" -o -z "$attr" ];then
>> 		_fail "Usage: _has_fs_sysfs_attr <mounted_device> <attr>"
>> 	fi
> 
> Hi Chao and Anand,
> 
> The *_fs_sysfs related helpers were brought in by me in:
>   commit 2a7b77abcd1dde735186b363da24540706c43b73
>   Author: Zorro Lang <zlang@redhat.com>
>   Date:   Tue Jul 5 17:30:24 2016 +0800
> 
>     xfs: configurable behavior on errors at unmount time
> 
> At that time, I didn't think about the "features/" directory.
> 
> Now, let me rethink this set of helpers. I feel that the *_fs_sysfs
> family of functions shouldn't be restricted to the $dev directory,
> we should remove this restriction.
> 
> Following that, we bring in *_fs_sysfs_device (appending the _device suffix)
> helpers which call *_fs_sysfs(). Also change the test cases who call these
> functions.
> 
> Finally, we can introduce the *_fs_sysfs_features functions (or simply use
> *_fs_sysfs directly).
> 
> Also, we have both *_fs_sysfs_attr and *_fs_sysfs, and the logic between
> them is somewhat confusing and easy to mix up.

Zorro,

It makes sense, and good news to their user. Thanks for the proposal.

> 
> I need to write a series of patches to clean up these logic. For this current
> patch, we can start with a small-scale change first. What do you think?

So I will use this first:

f2fs)
    local feat_file="/sys/fs/f2fs/features/fserror"
    if [ -f "$feat_file" ] && [ "$(cat "$feat_file")" = "supported" ]; then
        return 0
    fi
    ;;

And then waiting for your cleanup? or if you want to clean up this, please do it
directly while merging.

Thanks,

> 
> Thanks,
> Zorro
> 
>>
>>
>>> before reading it?
>>
>> How about this?
>>
>> f2fs)
>>     local feat_file="/sys/fs/f2fs/features/fserror"
>>     if [ -f "$feat_file" ] && [ "$(cat "$feat_file")" = "supported" ]; then
>>         return 0
>>     fi
>>     ;;
>>
>>>
>>>
>>>
>>>>  	esac
>>>>  
>>>>  	_notrun "$FSTYP does not support fanotify ioerrors"
>>>
>>