[PATCH] mm: Do not use double negation for testing page flags

[PATCH] mm: Do not use double negation for testing page flags

[Minchan Kim]

With the discussion[1], I found it seems there are every PageFlags
functions return bool at this moment so we don't need double
negation any more.
Although it's not a problem to keep it, it makes future users
confused to use dobule negation for them, too.

Remove such possibility.

[1] https://marc.info/?l=linux-kernel&m=148881578820434

Frankly sepaking, I like every PageFlags return bool instead of int.
It will make it clear. AFAIR, Chen Gang had tried it but don't know
why it was not merged at that time.

http://lkml.kernel.org/r/1469336184-1904-1-git-send-email-chengang@emindsoft.com.cn

Cc: Vlastimil Vlastimil Babka <vbabka@suse.cz>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Chen Gang <gang.chen.5i5j@gmail.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/khugepaged.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 88e4b17..7cb9c88 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -548,7 +548,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 		 * The page must only be referenced by the scanned process
 		 * and page swap cache.
 		 */
-		if (page_count(page) != 1 + !!PageSwapCache(page)) {
+		if (page_count(page) != 1 + PageSwapCache(page)) {
 			unlock_page(page);
 			result = SCAN_PAGE_COUNT;
 			goto out;
@@ -1181,7 +1181,7 @@ static int khugepaged_scan_pmd(struct mm_struct *mm,
 		 * The page must only be referenced by the scanned process
 		 * and page swap cache.
 		 */
-		if (page_count(page) != 1 + !!PageSwapCache(page)) {
+		if (page_count(page) != 1 + PageSwapCache(page)) {
 			result = SCAN_PAGE_COUNT;
 			goto out_unmap;
 		}
-- 
2.7.4

[PATCH] mm: Do not use double negation for testing page flags

[Minchan Kim]

With the discussion[1], I found it seems there are every PageFlags
functions return bool at this moment so we don't need double
negation any more.
Although it's not a problem to keep it, it makes future users
confused to use dobule negation for them, too.

Remove such possibility.

[1] https://marc.info/?l=linux-kernel&m=148881578820434

Frankly sepaking, I like every PageFlags return bool instead of int.
It will make it clear. AFAIR, Chen Gang had tried it but don't know
why it was not merged at that time.

http://lkml.kernel.org/r/1469336184-1904-1-git-send-email-chengang@emindsoft.com.cn

Cc: Vlastimil Vlastimil Babka <vbabka@suse.cz>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Chen Gang <gang.chen.5i5j@gmail.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/khugepaged.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 88e4b17..7cb9c88 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -548,7 +548,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 		 * The page must only be referenced by the scanned process
 		 * and page swap cache.
 		 */
-		if (page_count(page) != 1 + !!PageSwapCache(page)) {
+		if (page_count(page) != 1 + PageSwapCache(page)) {
 			unlock_page(page);
 			result = SCAN_PAGE_COUNT;
 			goto out;
@@ -1181,7 +1181,7 @@ static int khugepaged_scan_pmd(struct mm_struct *mm,
 		 * The page must only be referenced by the scanned process
 		 * and page swap cache.
 		 */
-		if (page_count(page) != 1 + !!PageSwapCache(page)) {
+		if (page_count(page) != 1 + PageSwapCache(page)) {
 			result = SCAN_PAGE_COUNT;
 			goto out_unmap;
 		}
-- 
2.7.4

Re: [PATCH] mm: Do not use double negation for testing page flags

[Anshuman Khandual]

On 03/07/2017 12:06 PM, Minchan Kim wrote:
> With the discussion[1], I found it seems there are every PageFlags
> functions return bool at this moment so we don't need double
> negation any more.
> Although it's not a problem to keep it, it makes future users
> confused to use dobule negation for them, too.
> 
> Remove such possibility.

A quick search of '!!Page' in the source tree does not show any other
place having this double negation. So I guess this is all which need
to be fixed.

Re: [PATCH] mm: Do not use double negation for testing page flags

[Anshuman Khandual]

On 03/07/2017 12:06 PM, Minchan Kim wrote:
> With the discussion[1], I found it seems there are every PageFlags
> functions return bool at this moment so we don't need double
> negation any more.
> Although it's not a problem to keep it, it makes future users
> confused to use dobule negation for them, too.
> 
> Remove such possibility.

A quick search of '!!Page' in the source tree does not show any other
place having this double negation. So I guess this is all which need
to be fixed.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Do not use double negation for testing page flags

[Johannes Weiner]

On Tue, Mar 07, 2017 at 03:36:37PM +0900, Minchan Kim wrote:
> With the discussion[1], I found it seems there are every PageFlags
> functions return bool at this moment so we don't need double
> negation any more.
> Although it's not a problem to keep it, it makes future users
> confused to use dobule negation for them, too.
> 
> Remove such possibility.
> 
> [1] https://marc.info/?l=linux-kernel&m=148881578820434
> 
> Frankly sepaking, I like every PageFlags return bool instead of int.
> It will make it clear. AFAIR, Chen Gang had tried it but don't know
> why it was not merged at that time.
> 
> http://lkml.kernel.org/r/1469336184-1904-1-git-send-email-chengang@emindsoft.com.cn
> 
> Cc: Vlastimil Vlastimil Babka <vbabka@suse.cz>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Chen Gang <gang.chen.5i5j@gmail.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Re: [PATCH] mm: Do not use double negation for testing page flags

[Johannes Weiner]

On Tue, Mar 07, 2017 at 03:36:37PM +0900, Minchan Kim wrote:
> With the discussion[1], I found it seems there are every PageFlags
> functions return bool at this moment so we don't need double
> negation any more.
> Although it's not a problem to keep it, it makes future users
> confused to use dobule negation for them, too.
> 
> Remove such possibility.
> 
> [1] https://marc.info/?l=linux-kernel&m=148881578820434
> 
> Frankly sepaking, I like every PageFlags return bool instead of int.
> It will make it clear. AFAIR, Chen Gang had tried it but don't know
> why it was not merged at that time.
> 
> http://lkml.kernel.org/r/1469336184-1904-1-git-send-email-chengang@emindsoft.com.cn
> 
> Cc: Vlastimil Vlastimil Babka <vbabka@suse.cz>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Chen Gang <gang.chen.5i5j@gmail.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Do not use double negation for testing page flags

[Minchan Kim]

Hi Anshuman,

On Tue, Mar 07, 2017 at 09:31:18PM +0530, Anshuman Khandual wrote:
> On 03/07/2017 12:06 PM, Minchan Kim wrote:
> > With the discussion[1], I found it seems there are every PageFlags
> > functions return bool at this moment so we don't need double
> > negation any more.
> > Although it's not a problem to keep it, it makes future users
> > confused to use dobule negation for them, too.
> > 
> > Remove such possibility.
> 
> A quick search of '!!Page' in the source tree does not show any other
> place having this double negation. So I guess this is all which need
> to be fixed.

Yeb. That's the why my patch includes only khugepagd part but my
concern is PageFlags returns int type not boolean so user might
be confused easily and tempted to use dobule negation.

Other side is they who create new custom PageXXX(e.g., PageMovable)
should keep it in mind that they should return 0 or 1 although
fucntion prototype's return value is int type. It shouldn't be
documented nowhere. Although we can add a little description
somewhere in page-flags.h, I believe changing to boolean is more
clear/not-error-prone so Chen's work is enough worth, I think.

Re: [PATCH] mm: Do not use double negation for testing page flags

[Minchan Kim]

Hi Anshuman,

On Tue, Mar 07, 2017 at 09:31:18PM +0530, Anshuman Khandual wrote:
> On 03/07/2017 12:06 PM, Minchan Kim wrote:
> > With the discussion[1], I found it seems there are every PageFlags
> > functions return bool at this moment so we don't need double
> > negation any more.
> > Although it's not a problem to keep it, it makes future users
> > confused to use dobule negation for them, too.
> > 
> > Remove such possibility.
> 
> A quick search of '!!Page' in the source tree does not show any other
> place having this double negation. So I guess this is all which need
> to be fixed.

Yeb. That's the why my patch includes only khugepagd part but my
concern is PageFlags returns int type not boolean so user might
be confused easily and tempted to use dobule negation.

Other side is they who create new custom PageXXX(e.g., PageMovable)
should keep it in mind that they should return 0 or 1 although
fucntion prototype's return value is int type. It shouldn't be
documented nowhere. Although we can add a little description
somewhere in page-flags.h, I believe changing to boolean is more
clear/not-error-prone so Chen's work is enough worth, I think.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Do not use double negation for testing page flags

[Vlastimil Babka]

On 03/08/2017 06:25 AM, Minchan Kim wrote:
> Hi Anshuman,
> 
> On Tue, Mar 07, 2017 at 09:31:18PM +0530, Anshuman Khandual wrote:
>> On 03/07/2017 12:06 PM, Minchan Kim wrote:
>>> With the discussion[1], I found it seems there are every PageFlags
>>> functions return bool at this moment so we don't need double
>>> negation any more.
>>> Although it's not a problem to keep it, it makes future users
>>> confused to use dobule negation for them, too.
>>>
>>> Remove such possibility.
>>
>> A quick search of '!!Page' in the source tree does not show any other
>> place having this double negation. So I guess this is all which need
>> to be fixed.
> 
> Yeb. That's the why my patch includes only khugepagd part but my
> concern is PageFlags returns int type not boolean so user might
> be confused easily and tempted to use dobule negation.
> 
> Other side is they who create new custom PageXXX(e.g., PageMovable)
> should keep it in mind that they should return 0 or 1 although
> fucntion prototype's return value is int type.

> It shouldn't be
> documented nowhere.

Was this double negation intentional? :P

> Although we can add a little description
> somewhere in page-flags.h, I believe changing to boolean is more
> clear/not-error-prone so Chen's work is enough worth, I think.

Agree, unless some arches benefit from the int by performance
for some reason (no idea if it's possible).

Anyway, to your original patch:

Acked-by: Vlastimil Babka <vbabka@suse.cz>

Re: [PATCH] mm: Do not use double negation for testing page flags

[Vlastimil Babka]

On 03/08/2017 06:25 AM, Minchan Kim wrote:
> Hi Anshuman,
> 
> On Tue, Mar 07, 2017 at 09:31:18PM +0530, Anshuman Khandual wrote:
>> On 03/07/2017 12:06 PM, Minchan Kim wrote:
>>> With the discussion[1], I found it seems there are every PageFlags
>>> functions return bool at this moment so we don't need double
>>> negation any more.
>>> Although it's not a problem to keep it, it makes future users
>>> confused to use dobule negation for them, too.
>>>
>>> Remove such possibility.
>>
>> A quick search of '!!Page' in the source tree does not show any other
>> place having this double negation. So I guess this is all which need
>> to be fixed.
> 
> Yeb. That's the why my patch includes only khugepagd part but my
> concern is PageFlags returns int type not boolean so user might
> be confused easily and tempted to use dobule negation.
> 
> Other side is they who create new custom PageXXX(e.g., PageMovable)
> should keep it in mind that they should return 0 or 1 although
> fucntion prototype's return value is int type.

> It shouldn't be
> documented nowhere.

Was this double negation intentional? :P

> Although we can add a little description
> somewhere in page-flags.h, I believe changing to boolean is more
> clear/not-error-prone so Chen's work is enough worth, I think.

Agree, unless some arches benefit from the int by performance
for some reason (no idea if it's possible).

Anyway, to your original patch:

Acked-by: Vlastimil Babka <vbabka@suse.cz>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Do not use double negation for testing page flags

[Michal Hocko]

On Wed 08-03-17 08:51:23, Vlastimil Babka wrote:
> On 03/08/2017 06:25 AM, Minchan Kim wrote:
[...]
> > Although we can add a little description
> > somewhere in page-flags.h, I believe changing to boolean is more
> > clear/not-error-prone so Chen's work is enough worth, I think.
> 
> Agree, unless some arches benefit from the int by performance
> for some reason (no idea if it's possible).

I have a vague recollection somebody tried to change this to bool and
the resulting code was larger on some architecture. Do not remember any
details though

Btw. feel free to add
Acked-by: Michal Hocko <mhocko@suse.com>

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: Do not use double negation for testing page flags

[Michal Hocko]

On Wed 08-03-17 08:51:23, Vlastimil Babka wrote:
> On 03/08/2017 06:25 AM, Minchan Kim wrote:
[...]
> > Although we can add a little description
> > somewhere in page-flags.h, I believe changing to boolean is more
> > clear/not-error-prone so Chen's work is enough worth, I think.
> 
> Agree, unless some arches benefit from the int by performance
> for some reason (no idea if it's possible).

I have a vague recollection somebody tried to change this to bool and
the resulting code was larger on some architecture. Do not remember any
details though

Btw. feel free to add
Acked-by: Michal Hocko <mhocko@suse.com>

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Do not use double negation for testing page flags

[Minchan Kim]

Hi Vlastimil,

On Wed, Mar 08, 2017 at 08:51:23AM +0100, Vlastimil Babka wrote:
> On 03/08/2017 06:25 AM, Minchan Kim wrote:
> > Hi Anshuman,
> > 
> > On Tue, Mar 07, 2017 at 09:31:18PM +0530, Anshuman Khandual wrote:
> >> On 03/07/2017 12:06 PM, Minchan Kim wrote:
> >>> With the discussion[1], I found it seems there are every PageFlags
> >>> functions return bool at this moment so we don't need double
> >>> negation any more.
> >>> Although it's not a problem to keep it, it makes future users
> >>> confused to use dobule negation for them, too.
> >>>
> >>> Remove such possibility.
> >>
> >> A quick search of '!!Page' in the source tree does not show any other
> >> place having this double negation. So I guess this is all which need
> >> to be fixed.
> > 
> > Yeb. That's the why my patch includes only khugepagd part but my
> > concern is PageFlags returns int type not boolean so user might
> > be confused easily and tempted to use dobule negation.
> > 
> > Other side is they who create new custom PageXXX(e.g., PageMovable)
> > should keep it in mind that they should return 0 or 1 although
> > fucntion prototype's return value is int type.
> 
> > It shouldn't be
> > documented nowhere.
> 
> Was this double negation intentional? :P

Nice catch!
It seems you have a crystal ball. ;-)

> 
> > Although we can add a little description
> > somewhere in page-flags.h, I believe changing to boolean is more
> > clear/not-error-prone so Chen's work is enough worth, I think.
> 
> Agree, unless some arches benefit from the int by performance
> for some reason (no idea if it's possible).
> 
> Anyway, to your original patch:
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!

Re: [PATCH] mm: Do not use double negation for testing page flags

[Minchan Kim]

Hi Vlastimil,

On Wed, Mar 08, 2017 at 08:51:23AM +0100, Vlastimil Babka wrote:
> On 03/08/2017 06:25 AM, Minchan Kim wrote:
> > Hi Anshuman,
> > 
> > On Tue, Mar 07, 2017 at 09:31:18PM +0530, Anshuman Khandual wrote:
> >> On 03/07/2017 12:06 PM, Minchan Kim wrote:
> >>> With the discussion[1], I found it seems there are every PageFlags
> >>> functions return bool at this moment so we don't need double
> >>> negation any more.
> >>> Although it's not a problem to keep it, it makes future users
> >>> confused to use dobule negation for them, too.
> >>>
> >>> Remove such possibility.
> >>
> >> A quick search of '!!Page' in the source tree does not show any other
> >> place having this double negation. So I guess this is all which need
> >> to be fixed.
> > 
> > Yeb. That's the why my patch includes only khugepagd part but my
> > concern is PageFlags returns int type not boolean so user might
> > be confused easily and tempted to use dobule negation.
> > 
> > Other side is they who create new custom PageXXX(e.g., PageMovable)
> > should keep it in mind that they should return 0 or 1 although
> > fucntion prototype's return value is int type.
> 
> > It shouldn't be
> > documented nowhere.
> 
> Was this double negation intentional? :P

Nice catch!
It seems you have a crystal ball. ;-)

> 
> > Although we can add a little description
> > somewhere in page-flags.h, I believe changing to boolean is more
> > clear/not-error-prone so Chen's work is enough worth, I think.
> 
> Agree, unless some arches benefit from the int by performance
> for some reason (no idea if it's possible).
> 
> Anyway, to your original patch:
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>