| Message ID | 20200512064147.514493-1-ying.huang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm, swap: Use prandom_u32_max() | expand |
On Tue 12-05-20 14:41:46, Huang Ying wrote: > 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? > Signed-off-by: "Huang, Ying" <ying.huang@intel.com> > Cc: Michal Hocko <mhocko@suse.com> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Tim Chen <tim.c.chen@linux.intel.com> > Cc: Hugh Dickins <hughd@google.com> To the change itself Acked-by: Michal Hocko <mhocko@suse.com> > --- > mm/swapfile.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/swapfile.c b/mm/swapfile.c > index a0a123e59ce6..2ec8b21201d6 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -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), > -- > 2.26.2
Michal Hocko <mhocko@kernel.org> writes: > On Tue 12-05-20 14:41:46, Huang Ying wrote: >> 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. >> Signed-off-by: "Huang, Ying" <ying.huang@intel.com> >> Cc: Michal Hocko <mhocko@suse.com> >> Cc: Minchan Kim <minchan@kernel.org> >> Cc: Tim Chen <tim.c.chen@linux.intel.com> >> Cc: Hugh Dickins <hughd@google.com> > > To the change itself > Acked-by: Michal Hocko <mhocko@suse.com> Thanks! Best Regards, Huang, Ying >> --- >> mm/swapfile.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/mm/swapfile.c b/mm/swapfile.c >> index a0a123e59ce6..2ec8b21201d6 100644 >> --- a/mm/swapfile.c >> +++ b/mm/swapfile.c >> @@ -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), >> -- >> 2.26.2
On Tue 12-05-20 15:14:46, Huang, Ying wrote: > Michal Hocko <mhocko@kernel.org> writes: > > > On Tue 12-05-20 14:41:46, Huang Ying wrote: > >> 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. 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. > >> Signed-off-by: "Huang, Ying" <ying.huang@intel.com> > >> Cc: Michal Hocko <mhocko@suse.com> > >> Cc: Minchan Kim <minchan@kernel.org> > >> Cc: Tim Chen <tim.c.chen@linux.intel.com> > >> Cc: Hugh Dickins <hughd@google.com> > > > > To the change itself > > Acked-by: Michal Hocko <mhocko@suse.com> > > Thanks! > > Best Regards, > Huang, Ying > > >> --- > >> mm/swapfile.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/mm/swapfile.c b/mm/swapfile.c > >> index a0a123e59ce6..2ec8b21201d6 100644 > >> --- a/mm/swapfile.c > >> +++ b/mm/swapfile.c > >> @@ -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), > >> -- > >> 2.26.2
Michal Hocko <mhocko@kernel.org> writes: > On Tue 12-05-20 15:14:46, Huang, Ying wrote: >> Michal Hocko <mhocko@kernel.org> writes: >> >> > On Tue 12-05-20 14:41:46, Huang Ying wrote: >> >> 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. > > 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 >> >> Signed-off-by: "Huang, Ying" <ying.huang@intel.com> >> >> Cc: Michal Hocko <mhocko@suse.com> >> >> Cc: Minchan Kim <minchan@kernel.org> >> >> Cc: Tim Chen <tim.c.chen@linux.intel.com> >> >> Cc: Hugh Dickins <hughd@google.com> >> > >> > To the change itself >> > Acked-by: Michal Hocko <mhocko@suse.com> >> >> Thanks! >> >> Best Regards, >> Huang, Ying >> >> >> --- >> >> mm/swapfile.c | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/mm/swapfile.c b/mm/swapfile.c >> >> index a0a123e59ce6..2ec8b21201d6 100644 >> >> --- a/mm/swapfile.c >> >> +++ b/mm/swapfile.c >> >> @@ -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), >> >> -- >> >> 2.26.2
diff --git a/mm/swapfile.c b/mm/swapfile.c index a0a123e59ce6..2ec8b21201d6 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -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),
To improve the code readability and get random number with higher quality. Signed-off-by: "Huang, Ying" <ying.huang@intel.com> Cc: Michal Hocko <mhocko@suse.com> Cc: Minchan Kim <minchan@kernel.org> Cc: Tim Chen <tim.c.chen@linux.intel.com> Cc: Hugh Dickins <hughd@google.com> --- mm/swapfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)