Re: [PATCH v4 4/6] mm/damon: introduce DAMOS filter type hugepage

[SeongJae Park <sj@kernel.org>]

On Wed, 5 Feb 2025 13:52:37 +0000 Usama Arif <usamaarif642@gmail.com> wrote:

> 
> 
> On 04/02/2025 23:12, SeongJae Park wrote:
> > On Mon,  3 Feb 2025 22:55:31 +0000 Usama Arif <usamaarif642@gmail.com> wrote:
> > 
> >> This is to gather statistics to check if memory regions of specific
> >> access tempratures are backed by hugepages of a size in a specific
> >> range
> > 
> > nit.  A period is missed?
> > 
> >> This filter can help to observe and prove the effectivenes of
> >> different schemes for shrinking/collapsing hugepages.
> >>
> >> Signed-off-by: Usama Arif <usamaarif642@gmail.com>
> >> ---
[...]
> >> --- a/mm/damon/sysfs-schemes.c
> >> +++ b/mm/damon/sysfs-schemes.c
> >> @@ -330,6 +330,7 @@ static const char * const damon_sysfs_scheme_filter_type_strs[] = {
> >>  	"anon",
> >>  	"memcg",
> >>  	"young",
> >> +	"hugepage",
> > 
> > hugepage_size?
> > 
> 
> So from https://lore.kernel.org/all/20250121200510.43645-1-sj@kernel.org/, my understanding
> was you didn't have a preference between "hugepage" and "hugepage_size"?

You're right.  And sorry, I changed my mind.

> 
> I do prefer hugepage over hugepage_size. This is because this is a type of filter, I would
> say hugepage_size is not a type, but hugepage is. For e.g. we have anon as type and not 
> anon_number. And the "size" part is clear from the min and max which is documented in patch
> 5 and 6.

I agree your points. The meaning of "min" and "max" is well documented.  But,
not everyone reads the documentation, and human's memory is volatile.  Also we
might want to further extend DAMOS fitler for other hugepage properties, say,
zero-filled hugepage-internal base pages ratio.  So now I prefer
"hugepage_size".

> 
> TBH my preference would be "folio". What I think we should do is let users input any value
> for min and max, as long as max > min, even if its just 4K page. If the user just wants
> to know how much of the region is 4K page, they will get it with this.

We can still do this, since DAMOS filters work for not only matching memory but
also non-matching memory.  It's not very intuitive, but at least makes a sense
in my opinion.

> But as David said,
> folio is a kernel concept so might not be best to expose it as sysfs to userspace?
> 
> As folio might not be an option, my preference is going with hugepage, but as its not
> an implementation detail and just naming, I am ok to change it to hugepage_size if
> you have a strong preference for it.

Yes, I'd suggest calling it "hugepage_size".  I don't find a problem it will
cause, other than the length.  Please let me know if I'm missing something,
though!

Again, sorry for changing my mind after the last discussion.


Thanks,
SJ

[...]