On 20/01/2025 20:12, SeongJae Park wrote:
> On Mon, 20 Jan 2025 19:58:24 +0000 Usama Arif <usamaarif642@xxxxxxxxx> 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?
>
> For user-space ABI, like DAMOS_FILTER_TYPE_ADDR, let's use
> 'schemes/<N>/filters/<F>/' directory. File names 'min' and 'max' look good to
> me.
>
> For kernel-space API, again, like DAMOS_FILTER_TYPE_ADDR, let's use the union
> in 'struct damos_filter'.
>
> If you will do the extension together with this patch, I think the default
> value is not really needed. It will only add a bit of complexity. So if you
> will do so, I'd recommend not having the default value.
>
> Also, if you will revise this patch series for the extension, could you please
> split and post the first patch of this series as a separate one? I think the
> fix is important and has no reason to be tied with this patch.
>
Yeah I think it might be best if we get a version with flexible folio sizes merged
from the start, especially as it involves creating user ABI. I will try and send
something tomorrow.
For the first patch, do I need to resend? Or maybe Andrew could merge whats in
this v3 for patch 1, we discard patch 2 and I just send the hugepage
filter implementation separately as a new series..
