Re: [PATCH] btrfs: drop unused memparse() parameter

David Sterba
2023-12-07T12:22:30Z

On Thu, Dec 07, 2023 at 01:01:50PM +1030, Qu Wenruo wrote:
>
>
> On 2023/12/5 21:43, David Disseldorp wrote:
> > The @retptr parameter for memparse() is optional.
> > btrfs_devinfo_scrub_speed_max_store() doesn't use it for any input
> > validation, so the parameter can be dropped.
>
> To me, I believe it's better to completely get rid of memparse().
>
> As you already found out, some suffix, especially "e|E" can screw up the
> result.
> E.g. "25e" would be interpreted as 25 with "e" as suffix, which is fine
> according to the rule. (without prefix, the base is 10, so only "25" is
> valid. Then the remaining part is interpreted as suffix).
>
> And since btrfs is not going to do pretty size output for sysfs (as most
> sysfs is not directly for end users, and we need accurate output), to be
> consistent there isn't much need for suffix handling either.
>
> So can't we just replace memparse() with kstrtoull()?

The value that can be read from the sysfs file is in bytes and it's so
that applications do not need to interpret it, like multiplying with
1024. We'll probably never return the pretty values with suffixes in
sysfs files.

However, on the input side the suffixes are a convenience, setting to
limit the throughput as '32m' is better than typing '32000000' and
counting zeros or $((32*1024*1024)) or 33554432.

This is why memparse is there and kstrtoull does not do that.