| Message ID | 20211202075859.51341-1-xhao@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RESEND,MERGE,REQUEST] mm/damon: move damon_rand() definition into damon.h | expand |
On Thu, 2 Dec 2021 15:58:59 +0800 Xin Hao <xhao@linux.alibaba.com> wrote: > damon_rand() is called in three files:damon/core.c, damon/ > paddr.c, damon/vaddr.c, i think there is no need to redefine > this twice, So move it to damon.h will be a good choice. Fair enough. > +/* Get a random number in [l, r) */ > +#define damon_rand(l, r) (l + prandom_u32_max(r - l)) This shouldn't be implemented as a macro, for all the usual reasons (typechecking, side-effects, ugliness, code-documentation site, etc). In particular, this expression references `l' twice, so calling damon_rand(a++, b); will increment `a' twice, surprise surprise. Can someone please send a followup patch to convert this (and any similar damon macros) into (probably static inlined) C functions?
On Thu, 2 Dec 2021 15:12:13 -0800 Andrew Morton <akpm@linux-foundation.org> wrote: > On Thu, 2 Dec 2021 15:58:59 +0800 Xin Hao <xhao@linux.alibaba.com> wrote: > > > damon_rand() is called in three files:damon/core.c, damon/ > > paddr.c, damon/vaddr.c, i think there is no need to redefine > > this twice, So move it to damon.h will be a good choice. > > Fair enough. > > > +/* Get a random number in [l, r) */ > > +#define damon_rand(l, r) (l + prandom_u32_max(r - l)) > > This shouldn't be implemented as a macro, for all the usual reasons > (typechecking, side-effects, ugliness, code-documentation site, etc). Yes, agreed. > > In particular, this expression references `l' twice, so calling > > damon_rand(a++, b); > > will increment `a' twice, surprise surprise. Thank you for this nice explanation. > > Can someone please send a followup patch to convert this (and any > similar damon macros) into (probably static inlined) C functions? Sure, I will do, unless someone shows interest here. Thanks, SJ
Hi park: Are you fixing this potential bug? If not, I am willing to modify, i will send a fixed patch ？
Hi Xin, On Mon, 6 Dec 2021 13:20:02 +0800 Xin Hao <xhao@linux.alibaba.com> wrote: > Hi park: > > Are you fixing this potential bug? If not, I am willing to modify, > i will send a fixed patch ？ I couldn't find a time slot for this yet. It would be great if you could send a patch. Thanks, SJ > > -- > Best Regards! > Xin Hao >
On 12/6/21 2:28 PM, SeongJae Park wrote: > Hi Xin, > > On Mon, 6 Dec 2021 13:20:02 +0800 Xin Hao <xhao@linux.alibaba.com> wrote: > >> Hi park: >> >> Are you fixing this potential bug? If not, I am willing to modify, >> i will send a fixed patch ？ > I couldn't find a time slot for this yet. It would be great if you could send > a patch. Ok, i will do, thanks! > > > Thanks, > SJ > >> -- >> Best Regards! >> Xin Hao >>
diff --git a/include/linux/damon.h b/include/linux/damon.h index 00ad96f2ec10..c6df025d8704 100644 --- a/include/linux/damon.h +++ b/include/linux/damon.h @@ -11,12 +11,16 @@ #include <linux/mutex.h> #include <linux/time64.h> #include <linux/types.h> +#include <linux/random.h> /* Minimal region size. Every damon_region is aligned by this. */ #define DAMON_MIN_REGION PAGE_SIZE /* Max priority score for DAMON-based operation schemes */ #define DAMOS_MAX_SCORE (99) +/* Get a random number in [l, r) */ +#define damon_rand(l, r) (l + prandom_u32_max(r - l)) + /** * struct damon_addr_range - Represents an address region of [@start, @end). * @start: Start address of the region (inclusive). diff --git a/mm/damon/core.c b/mm/damon/core.c index 4d2c3a0c7c8a..bdec32ef78c0 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -11,7 +11,6 @@ #include <linux/delay.h> #include <linux/kthread.h> #include <linux/mm.h> -#include <linux/random.h> #include <linux/slab.h> #include <linux/string.h> @@ -23,9 +22,6 @@ #define DAMON_MIN_REGION 1 #endif -/* Get a random number in [l, r) */ -#define damon_rand(l, r) (l + prandom_u32_max(r - l)) - static DEFINE_MUTEX(damon_lock); static int nr_running_ctxs; diff --git a/mm/damon/prmtv-common.h b/mm/damon/prmtv-common.h index 61f27037603e..e790cb5f8fe0 100644 --- a/mm/damon/prmtv-common.h +++ b/mm/damon/prmtv-common.h @@ -6,10 +6,6 @@ */ #include <linux/damon.h> -#include <linux/random.h> - -/* Get a random number in [l, r) */ -#define damon_rand(l, r) (l + prandom_u32_max(r - l)) struct page *damon_get_page(unsigned long pfn);