Re: [PATCH v3 1/4] tools/lib/mm: add shared file helpers

From: Mike Rapoport

Date: Tue May 26 2026 - 08:34:59 EST


On Tue, May 26, 2026 at 05:38:29PM +0530, Sarthak Sharma wrote:
> On 5/26/26 2:31 PM, Mike Rapoport wrote:
> > On Mon, May 25, 2026 at 11:59:32AM +0530, Sarthak Sharma wrote:
> >> On 5/24/26 10:36 PM, Mike Rapoport wrote:
> >>>
> >>> and while EXIT_FAILURE == KSFT_FAIL I'm not sure it's robust enough.
> >>
> >> I used EXIT_FAILURE here because the helper is moving out of selftests
> >> and should not include kselftest.h anymore. The helper already
> >> terminated the process on these paths, so I tried to preserve that
> >> behavior while removing the ksft dependency.
> >
> > In mm selftests a failure to update a /proc or /sysfs file meant there is
> > no point to continue the test. But if we make it a generic helper for
> > potentially broader use than mm selftests, exit() on failure is too harsh.
>
> Okay yeah, this makes sense.
>
> >
> >> We can change this to return errors instead of calling exit() and update
> >> the selftest callers to report failures through the ksft_* helpers. I
> >> agree this is cleaner, but it would grow the series a bit.
> >>
> >> If you feel strongly, I can include these changes in v4. Otherwise I
> >> feel we can handle it separately later to avoid growing this series.
> >
> > There are not that many callers of write_file() and write_num().
> > I think a patch that makes them return an error rather than exit() can go
> > before moving these functions to lib.
> >
>
> So I will add a patch before the move that makes read_file(),
> write_file(), read_num() and write_num() return errors instead of
> exiting and update the existing selftest callers to report those
> failures via ksft_* helpers.
>
> For hugepage_settings.c, I’d prefer to keep the existing fail fast
> behaviour in this series. After this series, the users are still mm
> selftests and the new tools/mm/gup_bench tool and for those users a
> failure to read/write THP or HugeTLB state is fatal to the operation
> being attempted.
>
> Converting the full hugepage_settings API to return errors would be a
> larger follow-up, because many of its helpers are used throughout mm
> selftests. I can handle that in a separate series unless you think it
> should be folded into this one as well.

Keeping fail fast in hugepage_settings for now makes sense, they are not as
generic as write_file().

--
Sincerely yours,
Mike.