Re: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
SJ Park <[email protected]>
| Newsgroups | dev.linux.lists.damon,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On Thu, 13 Aug 2026 17:27:51 +0300 Gutierrez Asier <[email protected]> wrote: > > > On 8/13/2026 4:55 PM, SJ Park wrote: > > Hi Asier, > > > > > > Thank you for your review! > > > > On Thu, 13 Aug 2026 11:33:01 +0300 Gutierrez Asier <[email protected]> wrote: > > > >> Hi SJ, > >> > >> On 8/13/2026 6:49 AM, SJ Park wrote: > >>> When NULL damon_ctx pointer parameter is passed, damon_call() could do > >>> NULL dereference. The caller is responsible to avoid that. It is easy > >>> to forget, and there are many damon_call() callers. Meanwhile, > >>> damon_call() is never meant to be performance critical. It uses mutex > >>> and completion. Add the NULL pointer check inside damon_call() so that > >>> callers can pass the parameter without NULL checks. > >>> > >>> Signed-off-by: SJ Park <[email protected]> > >>> --- > >>> mm/damon/core.c | 2 ++ > >>> 1 file changed, 2 insertions(+) > >>> > >>> diff --git a/mm/damon/core.c b/mm/damon/core.c > >>> index 92631a36d7b51..5882f9c94c47f 100644 > >>> --- a/mm/damon/core.c > >>> +++ b/mm/damon/core.c > >>> @@ -2188,6 +2188,8 @@ int damon_kdamond_pid(struct damon_ctx *ctx) > >>> */ > >>> int damon_call(struct damon_ctx *ctx, struct damon_call_control *control) > >>> { > >>> + if (!ctx) > >>> + return -EINVAL; > >>> if (!control->repeat) > >>> init_completion(&control->completion); > >>> control->canceled = false; > >> > >> I know that this is a minor optimization, but how about moving INIT_LIST_HEAD(&control->list) > >> to right before list_add_tail? > >> > >> It doesn't make sense to initialize the list before checking call_controls_obsolete. > > > > I agree it might be more efficient. > > > > I think my intention of the code was to to keep the call_controls_lock critical > > section minimal with access to things that really protected by the mutex. > > Moving the simple initialization should be fine. As you mentioned, it should > > be minor change. I'm bit concerned if it will confuse people if the list head > > is also protected by the mutex, though. What do you think? > Good point, I didn't think about it. > > Given that damon_call is not called often, I believe user experience should be > more important than efficiency. Anyway, you have the last word. Thank you for the quick answer, Asier. I understand you are find with keeping this as is. Let's keep this as is for now. Thanks, SJ [...]