Re: [PATCH v3 2/2] mm/damon: introduce DAMOS filter type hugepage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 21.01.25 18:52, SeongJae Park wrote:
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.

The issue with "folio" is that it is mostly a kernel-internal name for how we currently manage metadata for all pages and compound pages. In the future, some of what we call folio today will no longer be called folios ... as soon as we dynamically allocate "struct folio".

--
Cheers,

David / dhildenb





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux