staging: android: lowmemorykiller.c: Fix checkpatch warning

PATCH In review Anson Jacob 5 participants 7 messages Aug 25, 2016 – Sep 1, 2016 Staging

lore

Share & tools

lore

Anson Jacob

author

2016-08-25 15:10 UTC #0 lore raw

PATCH 1/1 staging: android: lowmemorykiller.c: Fix checkpatch warning

Fix checkpatch.pl 'line over 80 characters' warning

Signed-off-by Anson Jacob <[email protected]>

---
 drivers/staging/android/lowmemorykiller.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/staging/android/lowmemorykiller.c +2 -2

index 45a1b4e..80d7adf 100644
@@ -92,8 +92,8 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 	int array_size = ARRAY_SIZE(lowmem_adj);
 	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
 	int other_file = global_node_page_state(NR_FILE_PAGES) -
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+				global_node_page_state(NR_SHMEM) -
+				total_swapcache_pages();
 
 	if (lowmem_adj_size < array_size)
 		array_size = lowmem_adj_size;

Signature · 1 line

2.7.4

Andrey Utkin

2016-08-25 15:28 UTC #1 lore raw

Quoted 12 lines · Anson Jacob

Fix checkpatch.pl 'line over 80 characters' warning
Signed-off-by Anson Jacob <[email protected]>
---
 drivers/staging/android/lowmemorykiller.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/android/lowmemorykiller.c +0 -0
index 45a1b4e..80d7adf 100644

drivers/staging/android/lowmemorykiller.c static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc) · line 99

Quoted 10 lines

drivers/staging/android/lowmemorykiller.c +2 -2

@@ -92,8 +92,8 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 	int array_size = ARRAY_SIZE(lowmem_adj);
 	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
 	int other_file = global_node_page_state(NR_FILE_PAGES) -
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+				global_node_page_state(NR_SHMEM) -
+				total_swapcache_pages();
 
 	if (lowmem_adj_size < array_size)
 		array_size = lowmem_adj_size;

Indeed original alignment on opening brace here doesn't make sense. I don't mind if your patch gets accepted, but out of curiosity I have opened this file in vim with kernel coding style plugin loaded
(https://github.com/vivien/vim-linux-coding-style), selected the affected lines, and reindented the selection by pressing 'gq'. It ended up with this, being both shorter and simpler:

drivers/staging/android/lowmemorykiller.c +1 -2

index 45a1b4e..3aef467 100644
@@ -92,8 +92,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 	int array_size = ARRAY_SIZE(lowmem_adj);
 	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
 	int other_file = global_node_page_state(NR_FILE_PAGES) -
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+		global_node_page_state(NR_SHMEM) - total_swapcache_pages();
 
 	if (lowmem_adj_size < array_size)
 		array_size = lowmem_adj_size;

Greg Kroah-Hartman

maintainer

2016-09-01 15:36 UTC #2 lore raw

Quoted 36 lines · Andrey Utkin

Quoted 12 lines · Anson Jacob
Fix checkpatch.pl 'line over 80 characters' warning
Signed-off-by Anson Jacob <[email protected]>
---
 drivers/staging/android/lowmemorykiller.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/android/lowmemorykiller.c +0 -0
index 45a1b4e..80d7adf 100644
drivers/staging/android/lowmemorykiller.c static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc) · line 99
Quoted 10 lines
drivers/staging/android/lowmemorykiller.c +2 -2
@@ -92,8 +92,8 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 	int array_size = ARRAY_SIZE(lowmem_adj);
 	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
 	int other_file = global_node_page_state(NR_FILE_PAGES) -
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+				global_node_page_state(NR_SHMEM) -
+				total_swapcache_pages();
 
 	if (lowmem_adj_size < array_size)
 		array_size = lowmem_adj_size;
Indeed original alignment on opening brace here doesn't make sense. I don't mind if your patch gets accepted, but out of curiosity I have opened this file in vim with kernel coding style plugin loaded
(https://github.com/vivien/vim-linux-coding-style), selected the affected lines, and reindented the selection by pressing 'gq'. It ended up with this, being both shorter and simpler:
drivers/staging/android/lowmemorykiller.c +0 -0
index 45a1b4e..3aef467 100644

drivers/staging/android/lowmemorykiller.c static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc) · line 95

Quoted 6 lines

drivers/staging/android/lowmemorykiller.c +1 -2

@@ -92,8 +92,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 	int array_size = ARRAY_SIZE(lowmem_adj);
 	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
 	int other_file = global_node_page_state(NR_FILE_PAGES) -
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+		global_node_page_state(NR_SHMEM) - total_swapcache_pages();

The original patch here is nicer to read, don't you think?

thanks,

greg k-h

Joe Perches

2016-09-01 21:55 UTC #3 lore raw

Quoted 33 lines · Greg Kroah-Hartman

Quoted 32 lines · Andrey Utkin
Quoted 20 lines · Anson Jacob
Fix checkpatch.pl 'line over 80 characters' warning
Signed-off-by Anson Jacob <[email protected]>
---
drivers/staging/android/lowmemorykiller.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/android/lowmemorykiller.c +0 -0
index 45a1b4e..80d7adf 100644
@@ -92,8 +92,8 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
  	int array_size = ARRAY_SIZE(lowmem_adj);
  	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
int other_file = global_node_page_state(NR_FILE_PAGES) -
drivers/staging/android/lowmemorykiller.c +2 -2
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+				global_node_page_state(NR_SHMEM) -
+				total_swapcache_pages();
Quoted 3 lines

if (lowmem_adj_size < array_size)
array_size = lowmem_adj_size;
Indeed original alignment on opening brace here doesn't make sense. I don't mind if your patch gets accepted, but out of curiosity I have opened this file in vim with kernel coding style plugin loaded
(https://github.com/vivien/vim-linux-coding-style), selected the affected lines, and reindented the selection by pressing 'gq'. It ended up with this, being both shorter and simpler:

Quoted 12 lines

drivers/staging/android/lowmemorykiller.c
Quoted 11 lines
drivers/staging/android/lowmemorykiller.c +0 -0
index 45a1b4e..3aef467 100644
@@ -92,8 +92,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
  	int array_size = ARRAY_SIZE(lowmem_adj);
  	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
int other_file = global_node_page_state(NR_FILE_PAGES) -
drivers/staging/android/lowmemorykiller.c +1 -2
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+		global_node_page_state(NR_SHMEM) - total_swapcache_pages();
The original patch here is nicer to read, don't you think?

Strictly, these should probably be unsigned long instead of int as that's the return type of global_[node_]page_state().

Maybe something like the below, but then maybe other_free/file should become unsigned long too.
---
drivers/staging/android/lowmemorykiller.c | 9 +++++----

 1 file changed, 5 insertions(+), 4 deletions(-)

drivers/staging/android/lowmemorykiller.c +5 -4

index 45a1b4e..e8e1eaf 100644
@@ -90,10 +90,11 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 	int selected_tasksize = 0;
 	short selected_oom_score_adj;
 	int array_size = ARRAY_SIZE(lowmem_adj);
-	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
-	int other_file = global_node_page_state(NR_FILE_PAGES) -
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+	unsigned long nr_free = global_page_state(NR_FREE_PAGES);
+	unsigned long nr_file = global_node_page_state(NR_FILE_PAGES);
+	unsigned long nr_shmem = global_node_page_state(NR_SHMEM);
+	int other_free = nr_free - totalreserve_pages;
+	int other_file = nr_file - nr_shmem - total_swapcache_pages();
 
 	if (lowmem_adj_size < array_size)
 		array_size = lowmem_adj_size;

Greg Kroah-Hartman

maintainer

2016-09-01 21:02 UTC #4 lore raw

Quoted 66 lines · Joe Perches

Quoted 33 lines · Greg Kroah-Hartman
Quoted 32 lines · Andrey Utkin
Quoted 20 lines · Anson Jacob
Fix checkpatch.pl 'line over 80 characters' warning
Signed-off-by Anson Jacob <[email protected]>
---
drivers/staging/android/lowmemorykiller.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/android/lowmemorykiller.c +0 -0
index 45a1b4e..80d7adf 100644
@@ -92,8 +92,8 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
  	int array_size = ARRAY_SIZE(lowmem_adj);
  	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
int other_file = global_node_page_state(NR_FILE_PAGES) -
drivers/staging/android/lowmemorykiller.c +2 -2
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+				global_node_page_state(NR_SHMEM) -
+				total_swapcache_pages();
Quoted 3 lines

if (lowmem_adj_size < array_size)
array_size = lowmem_adj_size;
Indeed original alignment on opening brace here doesn't make sense. I don't mind if your patch gets accepted, but out of curiosity I have opened this file in vim with kernel coding style plugin loaded
(https://github.com/vivien/vim-linux-coding-style), selected the affected lines, and reindented the selection by pressing 'gq'. It ended up with this, being both shorter and simpler:
Quoted 12 lines
drivers/staging/android/lowmemorykiller.c static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
Quoted 11 lines
drivers/staging/android/lowmemorykiller.c +0 -0
index 45a1b4e..3aef467 100644
@@ -92,8 +92,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
  	int array_size = ARRAY_SIZE(lowmem_adj);
  	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
int other_file = global_node_page_state(NR_FILE_PAGES) -
drivers/staging/android/lowmemorykiller.c +1 -2
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+		global_node_page_state(NR_SHMEM) - total_swapcache_pages();
The original patch here is nicer to read, don't you think?
Strictly, these should probably be unsigned long instead of int as that's the return type of global_[node_]page_state().
Maybe something like the below, but then maybe other_free/file should become unsigned long too.
---
drivers/staging/android/lowmemorykiller.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/staging/android/lowmemorykiller.c +0 -2
index 45a1b4e..e8e1eaf 100644
@@ -90,10 +90,11 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 	int selected_tasksize = 0;
 	short selected_oom_score_adj;
 	int array_size = ARRAY_SIZE(lowmem_adj);
-	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
-	int other_file = global_node_page_state(NR_FILE_PAGES) -

drivers/staging/android/lowmemorykiller.c static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc) · line 100

Quoted 10 lines

drivers/staging/android/lowmemorykiller.c +5 -2

-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+	unsigned long nr_free = global_page_state(NR_FREE_PAGES);
+	unsigned long nr_file = global_node_page_state(NR_FILE_PAGES);
+	unsigned long nr_shmem = global_node_page_state(NR_SHMEM);
+	int other_free = nr_free - totalreserve_pages;
+	int other_file = nr_file - nr_shmem - total_swapcache_pages();
 
 	if (lowmem_adj_size < array_size)
 		array_size = lowmem_adj_size;

I like it, much easier to follow. Care to resend it in a patch form I can apply it in?

thanks,

greg k-h

Joe Perches

2016-09-01 21:13 UTC #5 lore raw

Quoted 5 lines · Greg Kroah-Hartman

Quoted 4 lines · Joe Perches
Quoted 3 lines · Greg Kroah-Hartman
Quoted 2 lines · Andrey Utkin
Quoted 1 line · Anson Jacob
Fix checkpatch.pl 'line over 80 characters' warning

[]

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
drivers/staging/android/lowmemorykiller.c
Quoted 1 line
drivers/staging/android/lowmemorykiller.c +0 -0

[]

Quoted 18 lines

Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 11 lines
drivers/staging/android/lowmemorykiller.c +0 -0
@@ -92,8 +92,8 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
  	int array_size = ARRAY_SIZE(lowmem_adj);
  	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
int other_file = global_node_page_state(NR_FILE_PAGES) -
drivers/staging/android/lowmemorykiller.c +2 -2
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+				global_node_page_state(NR_SHMEM) -
+				total_swapcache_pages();
if (lowmem_adj_size < array_size)
array_size = lowmem_adj_size;
Indeed original alignment on opening brace here doesn't make sense. I don't mind if your patch gets accepted, but out of curiosity I have opened this file in vim with kernel coding style plugin loaded
(https://github.com/vivien/vim-linux-coding-style), selected the affected lines, and reindented the selection by pressing 'gq'. It ended up with this, being both shorter and simpler:

Quoted 1 line

Quoted 1 line
Quoted 1 line
drivers/staging/android/lowmemorykiller.c
Quoted 1 line
drivers/staging/android/lowmemorykiller.c +0 -0

[]

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
drivers/staging/android/lowmemorykiller.c static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
Quoted 7 lines
drivers/staging/android/lowmemorykiller.c +0 -0
@@ -92,8 +92,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
  	int array_size = ARRAY_SIZE(lowmem_adj);
  	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
int other_file = global_node_page_state(NR_FILE_PAGES) -
drivers/staging/android/lowmemorykiller.c +1 -2
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+		global_node_page_state(NR_SHMEM) - total_swapcache_pages();
The original patch here is nicer to read, don't you think?

Quoted 5 lines

Quoted 5 lines
Strictly, these should probably be unsigned long instead of int as that's the return type of global_[node_]page_state().
Maybe something like the below, but then maybe other_free/file should become unsigned long too.

[]

Quoted 1 line

drivers/staging/android/lowmemorykiller.c
Quoted 1 line
drivers/staging/android/lowmemorykiller.c +0 -0

[]

Quoted 10 lines

drivers/staging/android/lowmemorykiller.c +2 -4

@@ -90,10 +90,11 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
  	int selected_tasksize = 0;
  	short selected_oom_score_adj;
  	int array_size = ARRAY_SIZE(lowmem_adj);
-	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
-	int other_file = global_node_page_state(NR_FILE_PAGES) -
-						global_node_page_state(NR_SHMEM) -
-						total_swapcache_pages();
+	unsigned long nr_free = global_page_state(NR_FREE_PAGES);
+	unsigned long nr_file = global_node_page_state(NR_FILE_PAGES);

Quoted 8 lines

Quoted 6 lines
drivers/staging/android/lowmemorykiller.c +3 -0
+	unsigned long nr_shmem = global_node_page_state(NR_SHMEM);
+	int other_free = nr_free - totalreserve_pages;
+	int other_file = nr_file - nr_shmem - total_swapcache_pages();
if (lowmem_adj_size < array_size)
array_size = lowmem_adj_size;
I like it, much easier to follow. Care to resend it in a patch form I can apply it in?

I think the android folk should figure out what the right thing to do with the int/unsigned long calculation is first.

Arve? Riley?

Arve Hjønnevåg

2016-09-01 21:19 UTC #6 lore raw

Quoted 64 lines · Joe Perches

Quoted 5 lines · Greg Kroah-Hartman
Quoted 4 lines · Joe Perches
Quoted 3 lines · Greg Kroah-Hartman
Quoted 2 lines · Andrey Utkin
Quoted 1 line · Anson Jacob
Fix checkpatch.pl 'line over 80 characters' warning
[]
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
drivers/staging/android/lowmemorykiller.c
Quoted 1 line
drivers/staging/android/lowmemorykiller.c +0 -0
[]
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
drivers/staging/android/lowmemorykiller.c static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc) · line 99
Quoted 11 lines
drivers/staging/android/lowmemorykiller.c +2 -2
@@ -92,8 +92,8 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
      int array_size = ARRAY_SIZE(lowmem_adj);
      int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
      int other_file = global_node_page_state(NR_FILE_PAGES) -
-                                             global_node_page_state(NR_SHMEM) -
-                                             total_swapcache_pages();
+                             global_node_page_state(NR_SHMEM) -
+                             total_swapcache_pages();
 
      if (lowmem_adj_size < array_size)
              array_size = lowmem_adj_size;
Indeed original alignment on opening brace here doesn't make sense. I don't mind if your patch gets accepted, but out of curiosity I have opened this file in vim with kernel coding style plugin loaded
(https://github.com/vivien/vim-linux-coding-style), selected the affected lines, and reindented the selection by pressing 'gq'. It ended up with this, being both shorter and simpler:
Quoted 1 line
Quoted 1 line
Quoted 1 line
drivers/staging/android/lowmemorykiller.c
Quoted 1 line
drivers/staging/android/lowmemorykiller.c +0 -0
[]
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/staging/android/lowmemorykiller.c static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc) · line 95
Quoted 7 lines
drivers/staging/android/lowmemorykiller.c +1 -2
@@ -92,8 +92,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
        int array_size = ARRAY_SIZE(lowmem_adj);
        int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
        int other_file = global_node_page_state(NR_FILE_PAGES) -
-                                               global_node_page_state(NR_SHMEM) -
-                                               total_swapcache_pages();
+               global_node_page_state(NR_SHMEM) - total_swapcache_pages();
The original patch here is nicer to read, don't you think?
Quoted 5 lines
Quoted 5 lines
Strictly, these should probably be unsigned long instead of int as that's the return type of global_[node_]page_state().
Maybe something like the below, but then maybe other_free/file should become unsigned long too.
[]
Quoted 1 line
drivers/staging/android/lowmemorykiller.c
Quoted 1 line
drivers/staging/android/lowmemorykiller.c +0 -0
[]
Quoted 14 lines
Quoted 14 lines
drivers/staging/android/lowmemorykiller.c +5 -4
@@ -90,10 +90,11 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
    int selected_tasksize = 0;
    short selected_oom_score_adj;
    int array_size = ARRAY_SIZE(lowmem_adj);
-   int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
-   int other_file = global_node_page_state(NR_FILE_PAGES) -
-                                           global_node_page_state(NR_SHMEM) -
-                                           total_swapcache_pages();
+   unsigned long nr_free = global_page_state(NR_FREE_PAGES);
+   unsigned long nr_file = global_node_page_state(NR_FILE_PAGES);
+   unsigned long nr_shmem = global_node_page_state(NR_SHMEM);
+   int other_free = nr_free - totalreserve_pages;
+   int other_file = nr_file - nr_shmem - total_swapcache_pages();
 
Quoted 4 lines
drivers/staging/android/lowmemorykiller.c static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc) · line 100
Quoted 2 lines
drivers/staging/android/lowmemorykiller.c +0 -0
    if (lowmem_adj_size < array_size)
            array_size = lowmem_adj_size;
I like it, much easier to follow. Care to resend it in a patch form I can apply it in?

Quoted 5 lines

I think the android folk should figure out what the right thing to do with the int/unsigned long calculation is first.
Arve? Riley?

I think the end result is the same if you store it in an int. If you store the result in a long it will work better, but I don't know of anyone using lowmemorykilller on a system with more than 8TB of memory.

Signature · 1 line

Arve Hjønnevåg