On Mon, 20 Jan 2025 21:03:05 +0100 David Hildenbrand <david@xxxxxxxxxx> wrote:
> On 20.01.25 20:58, Usama Arif wrote:
> >
> >
> > On 20/01/2025 19:30, SeongJae Park wrote:
> >> On Mon, 20 Jan 2025 20:23:20 +0100 David Hildenbrand <david@xxxxxxxxxx> wrote:
> >>
> >>> On 20.01.25 20:16, SeongJae Park wrote:
> >>>> On Mon, 20 Jan 2025 19:57:10 +0100 David Hildenbrand <david@xxxxxxxxxx> wrote:
> >>>>
> >>>>> On 20.01.25 19:19, Usama Arif wrote:
> >> [...]
> >>>>>> +#if defined(CONFIG_PGTABLE_HAS_HUGE_LEAVES)
> >>>>>> + case DAMOS_FILTER_TYPE_HUGEPAGE:
> >>>>>> + matched = folio_size(folio) == HPAGE_PMD_SIZE;
> >>>>>
> >>>>>
> >>>>> Can we directly embed in the name and the comments/docs that we are only
> >>>>> talking about PMD size (both, THP and hugetlb)?
> >>>>>
> >>>>> DAMOS_FILTER_TYPE_PMD_HUGEPAGE or sth. like that.
> >>>>
> >>>> Nice suggestion, thank you! And we might later add more filter types for
> >>>> different size huge pages. What about extending this to handle more general
> >>>> case, though? That is, we can let the filter receives a range of the folio
> >>>> size to match, like DAMOS_FILTER_TYPE_ADDR does. Then, the filter could be
> >>>> used for any size of interest.
> >>>
> >>> That would probably be future proof: either a range or explicitly
> >>> specified sizes (ranges?).
> >>
> >> DAMON supports installing multiple DAMOS filters. So multiple DAMOS filters
> >> that each matching single range can be used for the multiple sizes or ranges
> >> use case.
> >>
> >>
> >
> > Does creating something like schemes/<N>/access_pattern/page_size/{min,max}
> > sound good? with the default value being pmd size?
>
> "page_size" might be misleading.
Good point. I'm suggesting to add the files on another directory, and
apparently Usama agrees[1]. So the term "page_size" will not be used.
> Not sure if we want to use the word
> "folio_size" here, so far it's more an internal detail that evolved from
> compound pages.
>
> "hugepage_size" would at least match /sys/kernel/mm/hugepages/ and
> /sys/kernel/mm/transparent_hugepage/.
>
> But if you would also support "single page" == e.g., 4K, "hugepage"
> would be wrong.
Again, nice points, thank you for letting us aware of this. We could error
users if they try to set <=PAGE_SIZE filter range. FYI, DAMOS filters supports
making the filtering in/out action for not only condition-matching memory, but
also not-matching memory, so it will still be able to be used for filtering
in/out base pages.
That said, I now think "folio_size" might be a better term that allows simple
implementation and flexible usages. What do you think about changing the
filter type name from "hugepage" to "folio_size", and let users set the range
whatever they want? I still think "hugepage" type name is ok, but if there's
no objection about the naming, I'd slightly prefer more up to "folio_size".
[1] https://lore.kernel.org/db16fc35-58e7-453d-9fbb-318a88a98cd1@xxxxxxxxx
Thanks,
SJ
[...]
