Re: [PATCH v3 v3 2/2] ext4: allow clearing mballoc stats through mb_stats

liubaolin
2026-04-27T01:30:09Z

Dear Baokun,
Thank you and Ted for your review and suggestions.
I will incorporate your suggestions and submit the v4 patch as soon
as possible.

Thanks,
Baolin

在 2026/4/24 17:34, Baokun Li 写道:
>
> On 2026/4/24 16:09, liubaolin wrote:
>>
>>
>> 在 2026/4/24 0:19, Theodore Tso 写道:
>>> On Wed, Apr 22, 2026 at 09:50:25AM +0800, Baolin Liu wrote:
>>>> From: Baolin Liu <liubaolin@kylinos.cn>
>>>>
>>>> Make /proc/fs/ext4/<dev>/mb_stats writable and clear the runtime
>>>> mballoc statistics when 0 is written.
>>>
>>> At the moment to enable mb_stats the system administrator needs to
>>> write "1" to /sys/fs/ext4/<dev>/mb_stats, and writing "0" to the sysfs
>>> file will pauce the statistics colleciton (but not clear the
>>> statistics).  Adding a way to clear the statistics by writing to the
>>> procfs file might be confusing to users.
>>>
>>> So.... as a suggestion, if you're adding to the ability to write to
>>> /proc/fs/.../mb_stats, what if we make things work by
>>>
>>>     * Write 1 to /proc/fs/.../mb_stats to  enable statistics collection
>>>     * Write 0 to /proc/fs/.../mb_stats to  disable statistics collection
>>>     * Write -1 to /proc/fs/.../mb_stats to clear statistics counters
>>>
>>> And then deprecate the /sys/fs/.../mb_stats variable (but we probably
>>> won't be able to remove it for at least a year or two).
>>>
>>>                                         - Ted
>> Dear Ted, Baokun,
>>    Thank you for your review and suggestions.
>>    Since you mentioned that /sys/fs/.../mb_stats cannot be deleted in
>> the short term,
>>    I plan to modify and submit a v4 patch according to the following
>> strategy.
>>
>>    1. Change `/proc/fs/.../mb_stats` to read-write mode.
>>     * Read `/proc/fs/.../mb_stats` to show statistics counters.
>>     * Write 0 to `/proc/fs/.../mb_stats` to disable statistics
>> collection.
>>     * Write 1 to `/proc/fs/.../mb_stats` to enable statistics collection.
>>     * Write 2 to `/proc/fs/.../mb_stats` to clear statistics counters.
>>
>>    2. Do not delete the `/sys/fs/.../mb_stats` node for now; implement
>> the same write control logic.
>>     * Write 0 to `/sys/fs/.../mb_stats` to disable statistics collection.
>>     * Write 1 to `/sys/fs/.../mb_stats` to enable statistics collection.
>>     * Write 2 to `/sys/fs/.../mb_stats` to clear statistics counters.
>>
>>     Delete `/sys/fs/.../mb_stats` later when it is possible to delete it.
>>
>>    3. Modify the relevant documentation for `mb_stats`.
>>     Documentation/ABI/testing/sysfs-fs-ext4
>>     Documentation/admin-guide/ext4.rst
>>     Documentation/filesystems/proc.rst
>>
>>    Compared to your suggestion, I recommend using the value 2 for the
>> clear operation because s_mb_stats is an unsigned int variable, and
>> using -1 requires changing the variable type.
>>    I suggest avoiding changing the s_mb_stats variable type unless
>> absolutely necessary.
>>
>>    Do you think this modification is appropriate?
>>    If there are no problems, I will start modifying the code and
>> submit the v4 patch as soon as possible.
>
> For the clear command, we only handle it without storing it, so s_mb_stats
> remains unchanged and still stores only 0 and non-zero values to represent
> disabled and enabled, respectively. Otherwise, you will have to deal with
> a large number of s_mb_stats checks
>
> That means the /sys/fs/.../mb_stats interface does not need to support
> clearing, but it might make sense to add a deprecation warning there.
>
> Then in `/proc/fs/.../mb_stats`, writing 0 or a positive number passes
> it to s_mb_stats, writing -1 performs a reset, and other negative values
> return -EINVAL.
>
>
> Cheers,
> Baokun
>