[PATCH] mm/page_owner.c: allow page_owner with given start_pfn/count

[PATCH] mm/page_owner.c: allow page_owner with given start_pfn/count

[Kassey Li]

by default, page_owner iterates all page from min_low_pfn to
max_pfn, this cost too much time if we want an alternative pfn range.

with this patch it allows user to set pfn range to dump the page_onwer.

Signed-off-by: Kassey Li <quic_yingangl@quicinc.com>
---
 mm/page_owner.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/mm/page_owner.c b/mm/page_owner.c
index e4c6f3f1695b..79c634d11ee6 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -41,6 +41,39 @@ static depot_stack_handle_t dummy_handle;
 static depot_stack_handle_t failure_handle;
 static depot_stack_handle_t early_handle;
 
+static u64 start_pfn;
+static u64 end_pfn;
+
+static int debugfs_start_pfn_u64_set(void *data, u64 val)
+{
+	if (val < min_low_pfn || val > max_pfn)
+		return -EINVAL;
+	start_pfn = val;
+	return 0;
+}
+static int debugfs_start_pfn_u64_get(void *data, u64 *val)
+{
+	*val = start_pfn;
+	return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(fops_start_pfn, debugfs_start_pfn_u64_get,
+	debugfs_start_pfn_u64_set, "%lld\n");
+
+static int debugfs_end_pfn_u64_set(void *data, u64 val)
+{
+	if (val > max_pfn || val < start_pfn)
+		return -EINVAL;
+	end_pfn = val;
+	return 0;
+}
+static int debugfs_end_pfn_u64_get(void *data, u64 *val)
+{
+	*val = end_pfn;
+	return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(fops_end_pfn, debugfs_end_pfn_u64_get,
+	debugfs_end_pfn_u64_set, "%lld\n");
+
 static void init_early_allocated_pages(void);
 
 static int __init early_page_owner_param(char *buf)
@@ -497,7 +530,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 		return -EINVAL;
 
 	page = NULL;
-	pfn = min_low_pfn + *ppos;
+	pfn = start_pfn + *ppos;
 
 	/* Find a valid PFN or the start of a MAX_ORDER_NR_PAGES area */
 	while (!pfn_valid(pfn) && (pfn & (MAX_ORDER_NR_PAGES - 1)) != 0)
@@ -506,7 +539,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	drain_all_pages(NULL);
 
 	/* Find an allocated page */
-	for (; pfn < max_pfn; pfn++) {
+	for (; pfn < end_pfn; pfn++) {
 		/*
 		 * If the new page is in a new MAX_ORDER_NR_PAGES area,
 		 * validate the area as existing, skip it if not
@@ -561,7 +594,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 			continue;
 
 		/* Record the next PFN to read in the file offset */
-		*ppos = (pfn - min_low_pfn) + 1;
+		*ppos = (pfn - start_pfn) + 1;
 
 		return print_page_owner(buf, count, pfn, page,
 				page_owner, handle);
@@ -671,6 +704,12 @@ static int __init pageowner_init(void)
 
 	debugfs_create_file("page_owner", 0400, NULL, NULL,
 			    &proc_page_owner_operations);
+	debugfs_create_file("page_owner_start_pfn", 0644, NULL, NULL,
+				&fops_start_pfn);
+	debugfs_create_file("page_owner_end_pfn", 0644, NULL, NULL,
+				&fops_end_pfn);
+	start_pfn = min_low_pfn;
+	end_pfn = max_pfn;
 
 	return 0;
 }
-- 
2.17.1

Re: [PATCH] mm/page_owner.c: allow page_owner with given start_pfn/count

[Matthew Wilcox]

On Fri, Jul 22, 2022 at 11:08:10PM +0800, Kassey Li wrote:
> by default, page_owner iterates all page from min_low_pfn to
> max_pfn, this cost too much time if we want an alternative pfn range.
> 
> with this patch it allows user to set pfn range to dump the page_onwer.

This is a really bad UI.  If two users try to do different ranges at the
same time, it'll go wrong.  What use cases are you actually trying to
solve?

Re: [PATCH] mm/page_owner.c: allow page_owner with given start_pfn/count

[Kassey Li]

hi, Matthew:
	sorry for the delay, I just started to learn how to upstream patch, and 
setup my Thunderbird with plain text only.
	you are right, two users will cause problem here.
	the uses case is dump CMA area to understand the page usage in a given 
cma pool. 2nd, dump whole memory page owner is very time cost, mostly 
our android device has 8G memory now.
	I will research and check again, if you have more idea on this , please 
kindly to share.

BR
Kassey

On 7/22/2022 11:38 PM, Matthew Wilcox wrote:
> On Fri, Jul 22, 2022 at 11:08:10PM +0800, Kassey Li wrote:
>> by default, page_owner iterates all page from min_low_pfn to
>> max_pfn, this cost too much time if we want an alternative pfn range.
>>
>> with this patch it allows user to set pfn range to dump the page_onwer.
> 
> This is a really bad UI.  If two users try to do different ranges at the
> same time, it'll go wrong.  What use cases are you actually trying to
> solve?

Re: [PATCH] mm/page_owner.c: allow page_owner with given start_pfn/count

[Vlastimil Babka (SUSE)]

On 7/25/22 10:39, Kassey Li wrote:
> hi, Matthew:
>     sorry for the delay, I just started to learn how to upstream patch, and
> setup my Thunderbird with plain text only.
>     you are right, two users will cause problem here.
>     the uses case is dump CMA area to understand the page usage in a given
> cma pool. 2nd, dump whole memory page owner is very time cost, mostly our
> android device has 8G memory now.
>     I will research and check again, if you have more idea on this , please
> kindly to share.

You could try employing lseek() to specify the start pfn, and as for end
pfn, the process can just stop reading and close when it has seen enough?

> BR
> Kassey
> 
> On 7/22/2022 11:38 PM, Matthew Wilcox wrote:
>> On Fri, Jul 22, 2022 at 11:08:10PM +0800, Kassey Li wrote:
>>> by default, page_owner iterates all page from min_low_pfn to
>>> max_pfn, this cost too much time if we want an alternative pfn range.
>>>
>>> with this patch it allows user to set pfn range to dump the page_onwer.
>>
>> This is a really bad UI.  If two users try to do different ranges at the
>> same time, it'll go wrong.  What use cases are you actually trying to
>> solve?
> 

Re: [PATCH] mm/page_owner.c: allow page_owner with given start_pfn/count

[Kassey Li]

On 7/26/2022 10:03 PM, Vlastimil Babka (SUSE) wrote:
> On 7/25/22 10:39, Kassey Li wrote:
>> hi, Matthew:
>>      sorry for the delay, I just started to learn how to upstream patch, and
>> setup my Thunderbird with plain text only.
>>      you are right, two users will cause problem here.
>>      the uses case is dump CMA area to understand the page usage in a given
>> cma pool. 2nd, dump whole memory page owner is very time cost, mostly our
>> android device has 8G memory now.
>>      I will research and check again, if you have more idea on this , please
>> kindly to share.
> 
> You could try employing lseek() to specify the start pfn, and as for end
> pfn, the process can just stop reading and close when it has seen enough?

lseek is a good idea.
read_page_owner start with below
	pfn = min_low_pfn + *ppos;
so we need to export the min_low_pfn to user then decide the ppos to seek.
(my_cma.base_pfn - min_low_pfn) is the ppos we want to set.

is there concern to export min_low_pfn  ?
or use a mutex lock for my previous debugfs version patch ?

> 
>> BR
>> Kassey
>>
>> On 7/22/2022 11:38 PM, Matthew Wilcox wrote:
>>> On Fri, Jul 22, 2022 at 11:08:10PM +0800, Kassey Li wrote:
>>>> by default, page_owner iterates all page from min_low_pfn to
>>>> max_pfn, this cost too much time if we want an alternative pfn range.
>>>>
>>>> with this patch it allows user to set pfn range to dump the page_onwer.
>>>
>>> This is a really bad UI.  If two users try to do different ranges at the
>>> same time, it'll go wrong.  What use cases are you actually trying to
>>> solve?
>>
> 

Re: [PATCH] mm/page_owner.c: allow page_owner with given start_pfn/count

[Vlastimil Babka (SUSE)]

On 7/27/22 09:44, Kassey Li wrote:
> 
> 
> On 7/26/2022 10:03 PM, Vlastimil Babka (SUSE) wrote:
>> On 7/25/22 10:39, Kassey Li wrote:
>>> hi, Matthew:
>>>      sorry for the delay, I just started to learn how to upstream patch, and
>>> setup my Thunderbird with plain text only.
>>>      you are right, two users will cause problem here.
>>>      the uses case is dump CMA area to understand the page usage in a given
>>> cma pool. 2nd, dump whole memory page owner is very time cost, mostly our
>>> android device has 8G memory now.
>>>      I will research and check again, if you have more idea on this , please
>>> kindly to share.
>>
>> You could try employing lseek() to specify the start pfn, and as for end
>> pfn, the process can just stop reading and close when it has seen enough?
> 
> lseek is a good idea.
> read_page_owner start with below
>     pfn = min_low_pfn + *ppos;
> so we need to export the min_low_pfn to user then decide the ppos to seek.
> (my_cma.base_pfn - min_low_pfn) is the ppos we want to set.

Hm could we just pfn = *ppos and then anything below min_low_pfn is skipped
internally? So we don't need to teach userspace min_low_pfn.

> is there concern to export min_low_pfn  ?
> or use a mutex lock for my previous debugfs version patch ?
> 
>>
>>> BR
>>> Kassey
>>>
>>> On 7/22/2022 11:38 PM, Matthew Wilcox wrote:
>>>> On Fri, Jul 22, 2022 at 11:08:10PM +0800, Kassey Li wrote:
>>>>> by default, page_owner iterates all page from min_low_pfn to
>>>>> max_pfn, this cost too much time if we want an alternative pfn range.
>>>>>
>>>>> with this patch it allows user to set pfn range to dump the page_onwer.
>>>>
>>>> This is a really bad UI.  If two users try to do different ranges at the
>>>> same time, it'll go wrong.  What use cases are you actually trying to
>>>> solve?
>>>
>>

Re: [PATCH] mm/page_owner.c: allow page_owner with given start_pfn/count

[Kassey Li]

On 7/27/2022 6:59 PM, Vlastimil Babka (SUSE) wrote:
> On 7/27/22 09:44, Kassey Li wrote:
>>
>>
>> On 7/26/2022 10:03 PM, Vlastimil Babka (SUSE) wrote:
>>> On 7/25/22 10:39, Kassey Li wrote:
>>>> hi, Matthew:
>>>>       sorry for the delay, I just started to learn how to upstream patch, and
>>>> setup my Thunderbird with plain text only.
>>>>       you are right, two users will cause problem here.
>>>>       the uses case is dump CMA area to understand the page usage in a given
>>>> cma pool. 2nd, dump whole memory page owner is very time cost, mostly our
>>>> android device has 8G memory now.
>>>>       I will research and check again, if you have more idea on this , please
>>>> kindly to share.
>>>
>>> You could try employing lseek() to specify the start pfn, and as for end
>>> pfn, the process can just stop reading and close when it has seen enough?
>>
>> lseek is a good idea.
>> read_page_owner start with below
>>      pfn = min_low_pfn + *ppos;
>> so we need to export the min_low_pfn to user then decide the ppos to seek.
>> (my_cma.base_pfn - min_low_pfn) is the ppos we want to set.
> 
> Hm could we just pfn = *ppos and then anything below min_low_pfn is skipped
> internally? So we don't need to teach userspace min_low_pfn.
	that makes sense.
	I send out a new path "mm/page_owner.c: add llseek for page_owner" 
according your suggest, please help to review again, thanks.
> 
>> is there concern to export min_low_pfn  ?
>> or use a mutex lock for my previous debugfs version patch ?
>>
>>>
>>>> BR
>>>> Kassey
>>>>
>>>> On 7/22/2022 11:38 PM, Matthew Wilcox wrote:
>>>>> On Fri, Jul 22, 2022 at 11:08:10PM +0800, Kassey Li wrote:
>>>>>> by default, page_owner iterates all page from min_low_pfn to
>>>>>> max_pfn, this cost too much time if we want an alternative pfn range.
>>>>>>
>>>>>> with this patch it allows user to set pfn range to dump the page_onwer.
>>>>>
>>>>> This is a really bad UI.  If two users try to do different ranges at the
>>>>> same time, it'll go wrong.  What use cases are you actually trying to
>>>>> solve?
>>>>
>>>
>