Re: [RFC PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
Gutierrez Asier <[email protected]>
| Newsgroups | dev.linux.lists.damon,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
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. > >> >> Maybe this suggestion should go in a different patch. > > I agree. > > Thanks, > SJ > > [...] -- Asier Gutierrez Huawei