Re: [PATCH] mm/khugepaged: add tracepoint to collapse_file()

From: Zach O'Keefe
Date: Mon Oct 24 2022 - 15:11:37 EST


Thanks for your mail, Gautam.

> I try to keep dereferences out of the calling path as much as possible
> (adds to I$ at the call site).

This was probably due to the way I handled
trace_mm_khugepaged_scan_file(). Perhaps that can be cleaned up at the
same time as this patch, for consistency.

Also, no qualms about adding this tracepoint; there are a few scan
result codes that overlap between hpage_collapse_scan_file() and those
possibly returned in collapse_file() such that, if we only have the
one tracepoint in hpage_collapse_scan_file(), it could be ambiguous
what callsite the error path stemmed from. Luckily this hasn't been an
issue thus far.

Lastly, a few other items we might care about capturing:

- is_shmem (perhaps the filename is enough to know this - but I know
at least once during development I was caught off-guard b/c a mount I
thought to be file-backed turned out to be tmpfs (and something I
didn't think to question until I had wasted some time on other
paths)).
- index

Best,
Zach


> Could you just pass in file, and then have:
>
> __string(filename, file->f_path.dentry->d_iname)
>
> [..]
>
> __assign_string(filename, file->f_path.dentry->d_iname);
>
>
> If you are paranoid, you can have the above also be:
>
> file ? file->f_path.dentry ? file->f_path.dentry->d_iname : "(null)" : "(null)")
>
>
> -- Steve
>
>
> > + nr, result);
> > return result;