2022-01-06 10:40:24

by Baolin Wang

[permalink] [raw]
Subject: [PATCH] mm/damon: Remove redundant page validation

It will never get a NULL page by pte_page(), thus remove the redundant
page validation to fix below Smatch static checker warning.

mm/damon/vaddr.c:405 damon_hugetlb_mkold()
warn: 'page' can't be NULL.

Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Baolin Wang <[email protected]>
---
mm/damon/vaddr.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index 89b6468d..8a52e00 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -402,9 +402,6 @@ static void damon_hugetlb_mkold(pte_t *pte, struct mm_struct *mm,
pte_t entry = huge_ptep_get(pte);
struct page *page = pte_page(entry);

- if (!page)
- return;
-
get_page(page);

if (pte_young(entry)) {
@@ -564,9 +561,6 @@ static int damon_young_hugetlb_entry(pte_t *pte, unsigned long hmask,
goto out;

page = pte_page(entry);
- if (!page)
- goto out;
-
get_page(page);

if (pte_young(entry) || !page_is_idle(page) ||
--
1.8.3.1



2022-01-06 11:40:55

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH] mm/damon: Remove redundant page validation

Hi Baolin,


Thank you always for your great contribution! As always, trivial comments
below.

On Thu, 6 Jan 2022 18:40:11 +0800 Baolin Wang <[email protected]> wrote:

> It will never get a NULL page by pte_page(), thus remove the redundant
> page validation to fix below Smatch static checker warning.

I guess the issue has reported in the open place[1], right? I think it would
be better to add the link in this commit message, so that future readers can
read and learn from your discussion in the mail thread.

[1] https://lore.kernel.org/linux-mm/20220106091200.GA14564@kili/

>
> mm/damon/vaddr.c:405 damon_hugetlb_mkold()
> warn: 'page' can't be NULL.

Sorry for continuously obsessing about this trivial and personal taste, but...
Could you please indent quoting blocks like above (hopefully with 4 spaces)?
That makes the message much easier to read for me, and I'd like to keep the
commit messages for DAMON consistent and easy to read, at least for me.

Other than that,

Reviewed-by: SeongJae Park <[email protected]>


Thanks,
SJ

>
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Baolin Wang <[email protected]>
> ---
> mm/damon/vaddr.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 89b6468d..8a52e00 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -402,9 +402,6 @@ static void damon_hugetlb_mkold(pte_t *pte, struct mm_struct *mm,
> pte_t entry = huge_ptep_get(pte);
> struct page *page = pte_page(entry);
>
> - if (!page)
> - return;
> -
> get_page(page);
>
> if (pte_young(entry)) {
> @@ -564,9 +561,6 @@ static int damon_young_hugetlb_entry(pte_t *pte, unsigned long hmask,
> goto out;
>
> page = pte_page(entry);
> - if (!page)
> - goto out;
> -
> get_page(page);
>
> if (pte_young(entry) || !page_is_idle(page) ||
> --
> 1.8.3.1

2022-01-06 13:01:42

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH] mm/damon: Remove redundant page validation

Hi SeongJae,

On 1/6/2022 7:40 PM, SeongJae Park wrote:
> Hi Baolin,
>
>
> Thank you always for your great contribution! As always, trivial comments
> below.
>
> On Thu, 6 Jan 2022 18:40:11 +0800 Baolin Wang <[email protected]> wrote:
>
>> It will never get a NULL page by pte_page(), thus remove the redundant
>> page validation to fix below Smatch static checker warning.
>
> I guess the issue has reported in the open place[1], right? I think it would
> be better to add the link in this commit message, so that future readers can
> read and learn from your discussion in the mail thread.
>
> [1] https://lore.kernel.org/linux-mm/20220106091200.GA14564@kili/

OK.

>
>>
>> mm/damon/vaddr.c:405 damon_hugetlb_mkold()
>> warn: 'page' can't be NULL.
>
> Sorry for continuously obsessing about this trivial and personal taste, but...
> Could you please indent quoting blocks like above (hopefully with 4 spaces)?
> That makes the message much easier to read for me, and I'd like to keep the
> commit messages for DAMON consistent and easy to read, at least for me.

Sure, I will keep the rule of DAMON in mind.

Andrew, could you help to change the commit message format as SeongJae
suggested when you apply this patch? Thanks.

>
> Other than that,
>
> Reviewed-by: SeongJae Park <[email protected]>

Thanks.