On Tue 12-05-20 15:14:46, Huang, Ying wrote: > Michal Hocko <mhocko@xxxxxxxxxx> 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@xxxxxxxxx> > >> Cc: Michal Hocko <mhocko@xxxxxxxx> > >> Cc: Minchan Kim <minchan@xxxxxxxxxx> > >> Cc: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx> > >> Cc: Hugh Dickins <hughd@xxxxxxxxxx> > > > > To the change itself > > Acked-by: Michal Hocko <mhocko@xxxxxxxx> > > 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 SUSE Labs
