Skip to contentSkip to conversation

mm, swap: Use prandom_u32_max()

PATCH In review Huang Ying 2 participants 5 messages May 12, 2020

Subsystems (2)
Memory management MM swap
lore
Share & tools
lore

Huang Ying

author
#0 lore raw

PATCH 1/1 mm, swap: Use prandom_u32_max()

To improve the code readability and get random number with higher quality.

Signed-off-by "Huang, Ying" <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Minchan Kim <[email protected]>
Cc Tim Chen <[email protected]>
Cc Hugh Dickins <[email protected]>
---
 mm/swapfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/swapfile.c +1 -1
index a0a123e59ce6..2ec8b21201d6 100644
@@ -3220,7 +3220,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 		 * select a random position to start with to help wear leveling
 		 * SSD
 		 */
-		p->cluster_next = 1 + (prandom_u32() % p->highest_bit);
+		p->cluster_next = 1 + prandom_u32_max(p->highest_bit);
 		nr_cluster = DIV_ROUND_UP(maxpages, SWAPFILE_CLUSTER);
 
 		cluster_info = kvcalloc(nr_cluster, sizeof(*cluster_info),
Signature · 1 line
2.26.2
Acked-by on v1 1/1
#1 lore raw
Quoted 2 lines · Huang Ying

To improve the code readability and get random number with higher quality.

I understand the readability argument but why should prandom_u32_max (which I was not aware of) provide a higher quality randomness?

Quoted 5 lines
Signed-off-by "Huang, Ying" <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Minchan Kim <[email protected]>
Cc Tim Chen <[email protected]>
Cc Hugh Dickins <[email protected]>

To the change itself

Acked-by Michal Hocko <[email protected]>
Quoted 19 lines
---
 mm/swapfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/swapfile.c +1 -1
index a0a123e59ce6..2ec8b21201d6 100644
@@ -3220,7 +3220,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 		 * select a random position to start with to help wear leveling
 		 * SSD
 		 */
-		p->cluster_next = 1 + (prandom_u32() % p->highest_bit);
+		p->cluster_next = 1 + prandom_u32_max(p->highest_bit);
 		nr_cluster = DIV_ROUND_UP(maxpages, SWAPFILE_CLUSTER);
 
 		cluster_info = kvcalloc(nr_cluster, sizeof(*cluster_info),
Signature · 1 line
2.26.2
Signature · 2 lines
Michal Hocko
SUSE Labs
author
#2 lore raw
Quoted 6 lines · Michal Hocko
Quoted 2 lines · Huang Ying

To improve the code readability and get random number with higher quality.

I understand the readability argument but why should prandom_u32_max (which I was not aware of) provide a higher quality randomness?

I am not expert on random number generator. I have heard about that the randomness of the low order bits of some random number generator isn't good enough. Anyway, by using the common implementation, the real random number generator expert can fix the possible issue once for all users.

Quoted 8 lines
Quoted 5 lines
Signed-off-by "Huang, Ying" <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Minchan Kim <[email protected]>
Cc Tim Chen <[email protected]>
Cc Hugh Dickins <[email protected]>

To the change itself

Acked-by Michal Hocko <[email protected]>

Thanks!

Best Regards,
Huang, Ying

Quoted 19 lines
Quoted 19 lines
---
 mm/swapfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/swapfile.c +1 -1
index a0a123e59ce6..2ec8b21201d6 100644
@@ -3220,7 +3220,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 		 * select a random position to start with to help wear leveling
 		 * SSD
 		 */
-		p->cluster_next = 1 + (prandom_u32() % p->highest_bit);
+		p->cluster_next = 1 + prandom_u32_max(p->highest_bit);
 		nr_cluster = DIV_ROUND_UP(maxpages, SWAPFILE_CLUSTER);
 
 		cluster_info = kvcalloc(nr_cluster, sizeof(*cluster_info),
Signature · 1 line
2.26.2
#3 lore raw
Quoted 6 lines · Ying
Quoted 4 lines · Michal Hocko
Quoted 2 lines · Huang Ying

To improve the code readability and get random number with higher quality.

Quoted 8 lines
Quoted 2 lines

I understand the readability argument but why should prandom_u32_max (which I was not aware of) provide a higher quality randomness?

I am not expert on random number generator. I have heard about that the randomness of the low order bits of some random number generator isn't good enough. Anyway, by using the common implementation, the real random number generator expert can fix the possible issue once for all users.

Please drop the quality argument if you cannot really justify it. This will likely just confuse future readers the same way it confused me here. Because prandom_u32_max uses the same source of randomness the only difference is the way how modulo vs. u64 overflow arithmetic is used for distributing values. I am not aware the later would be a way to achieve a higher quality randomness. If the interval distribution is better with the later then it would be great to have it documented.

Quoted 33 lines
Quoted 8 lines
Quoted 5 lines
Signed-off-by "Huang, Ying" <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Minchan Kim <[email protected]>
Cc Tim Chen <[email protected]>
Cc Hugh Dickins <[email protected]>

To the change itself

Acked-by Michal Hocko <[email protected]>

Thanks!

Best Regards,
Huang, Ying

Quoted 19 lines
Quoted 19 lines
---
 mm/swapfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/swapfile.c +1 -1
index a0a123e59ce6..2ec8b21201d6 100644
@@ -3220,7 +3220,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 		 * select a random position to start with to help wear leveling
 		 * SSD
 		 */
-		p->cluster_next = 1 + (prandom_u32() % p->highest_bit);
+		p->cluster_next = 1 + prandom_u32_max(p->highest_bit);
 		nr_cluster = DIV_ROUND_UP(maxpages, SWAPFILE_CLUSTER);
 
 		cluster_info = kvcalloc(nr_cluster, sizeof(*cluster_info),
Signature · 1 line
2.26.2
Signature · 2 lines
Michal Hocko
SUSE Labs
author
#4 lore raw
Quoted 16 lines · Michal Hocko
Quoted 6 lines · Ying
Quoted 4 lines · Michal Hocko
Quoted 2 lines · Huang Ying

To improve the code readability and get random number with higher quality.

Quoted 8 lines
Quoted 2 lines

I understand the readability argument but why should prandom_u32_max (which I was not aware of) provide a higher quality randomness?

I am not expert on random number generator. I have heard about that the randomness of the low order bits of some random number generator isn't good enough. Anyway, by using the common implementation, the real random number generator expert can fix the possible issue once for all users.

Quoted 8 lines

Please drop the quality argument if you cannot really justify it. This will likely just confuse future readers the same way it confused me here. Because prandom_u32_max uses the same source of randomness the only difference is the way how modulo vs. u64 overflow arithmetic is used for distributing values. I am not aware the later would be a way to achieve a higher quality randomness. If the interval distribution is better with the later then it would be great to have it documented.

OK. Fair enough.

Best Regards,
Huang, Ying

Quoted 33 lines
Quoted 33 lines
Quoted 8 lines
Quoted 5 lines
Signed-off-by "Huang, Ying" <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Minchan Kim <[email protected]>
Cc Tim Chen <[email protected]>
Cc Hugh Dickins <[email protected]>

To the change itself

Acked-by Michal Hocko <[email protected]>

Thanks!

Best Regards,
Huang, Ying

Quoted 19 lines
Quoted 19 lines
---
 mm/swapfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/swapfile.c +1 -1
index a0a123e59ce6..2ec8b21201d6 100644
@@ -3220,7 +3220,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 		 * select a random position to start with to help wear leveling
 		 * SSD
 		 */
-		p->cluster_next = 1 + (prandom_u32() % p->highest_bit);
+		p->cluster_next = 1 + prandom_u32_max(p->highest_bit);
 		nr_cluster = DIV_ROUND_UP(maxpages, SWAPFILE_CLUSTER);
 
 		cluster_info = kvcalloc(nr_cluster, sizeof(*cluster_info),
Signature · 1 line
2.26.2