Message-ID: <7765df86-b08a-4f70-900d-4b4d85c07d49@suse.com>
Date: Wed, 18 Feb 2026 16:08:19 +0100
From: Petr Pavlu <petr.pavlu@...e.com>
To: Byungchul Park <byungchul@...com>
Cc: kernel_team@...ynix.com, torvalds@...ux-foundation.org,
 damien.lemoal@...nsource.wdc.com, linux-ide@...r.kernel.org,
 adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org, mingo@...hat.com,
 peterz@...radead.org, will@...nel.org, tglx@...utronix.de,
 rostedt@...dmis.org, joel@...lfernandes.org, sashal@...nel.org,
 daniel.vetter@...ll.ch, duyuyang@...il.com, johannes.berg@...el.com,
 tj@...nel.org, tytso@....edu, willy@...radead.org, david@...morbit.com,
 amir73il@...il.com, gregkh@...uxfoundation.org, kernel-team@....com,
 linux-mm@...ck.org, akpm@...ux-foundation.org, mhocko@...nel.org,
 minchan@...nel.org, hannes@...xchg.org, vdavydov.dev@...il.com,
 sj@...nel.org, jglisse@...hat.com, dennis@...nel.org, cl@...ux.com,
 penberg@...nel.org, rientjes@...gle.com, vbabka@...e.cz, ngupta@...are.org,
 linux-block@...r.kernel.org, josef@...icpanda.com,
 linux-fsdevel@...r.kernel.org, jack@...e.cz, jlayton@...nel.org,
 dan.j.williams@...el.com, hch@...radead.org, djwong@...nel.org,
 dri-devel@...ts.freedesktop.org, rodrigosiqueiramelo@...il.com,
 melissa.srw@...il.com, hamohammed.sa@...il.com, harry.yoo@...cle.com,
 chris.p.wilson@...el.com, gwan-gyeong.mun@...el.com,
 max.byungchul.park@...il.com, boqun.feng@...il.com, longman@...hat.com,
 yunseong.kim@...csson.com, ysk@...lloc.com, yeoreum.yun@....com,
 netdev@...r.kernel.org, matthew.brost@...el.com, her0gyugyu@...il.com,
 corbet@....net, catalin.marinas@....com, bp@...en8.de, x86@...nel.org,
 hpa@...or.com, luto@...nel.org, sumit.semwal@...aro.org,
 gustavo@...ovan.org, christian.koenig@....com, andi.shyti@...nel.org,
 arnd@...db.de, lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com,
 rppt@...nel.org, surenb@...gle.com, mcgrof@...nel.org, da.gomez@...nel.org,
 samitolvanen@...gle.com, paulmck@...nel.org, frederic@...nel.org,
 neeraj.upadhyay@...nel.org, joelagnelf@...dia.com, josh@...htriplett.org,
 urezki@...il.com, mathieu.desnoyers@...icios.com, jiangshanlai@...il.com,
 qiang.zhang@...ux.dev, juri.lelli@...hat.com, vincent.guittot@...aro.org,
 dietmar.eggemann@....com, bsegall@...gle.com, mgorman@...e.de,
 vschneid@...hat.com, chuck.lever@...cle.com, neil@...wn.name,
 okorniev@...hat.com, Dai.Ngo@...cle.com, tom@...pey.com, trondmy@...nel.org,
 anna@...nel.org, kees@...nel.org, bigeasy@...utronix.de,
 clrkwllms@...nel.org, mark.rutland@....com, ada.coupriediaz@....com,
 kristina.martsenko@....com, wangkefeng.wang@...wei.com, broonie@...nel.org,
 kevin.brodsky@....com, dwmw@...zon.co.uk, shakeel.butt@...ux.dev,
 ast@...nel.org, ziy@...dia.com, yuzhao@...gle.com,
 baolin.wang@...ux.alibaba.com, usamaarif642@...il.com,
 joel.granados@...nel.org, richard.weiyang@...il.com,
 geert+renesas@...der.be, tim.c.chen@...ux.intel.com, linux@...blig.org,
 alexander.shishkin@...ux.intel.com, lillian@...r-ark.net,
 chenhuacai@...nel.org, francesco@...la.it, guoweikang.kernel@...il.com,
 link@...o.com, jpoimboe@...nel.org, masahiroy@...nel.org,
 brauner@...nel.org, thomas.weissschuh@...utronix.de, oleg@...hat.com,
 mjguzik@...il.com, andrii@...nel.org, wangfushuai@...du.com,
 linux-doc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-media@...r.kernel.org, linaro-mm-sig@...ts.linaro.org,
 linux-i2c@...r.kernel.org, linux-arch@...r.kernel.org,
 linux-modules@...r.kernel.org, rcu@...r.kernel.org,
 linux-nfs@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
 2407018371@...com, dakr@...nel.org, miguel.ojeda.sandonis@...il.com,
 neilb@...mail.net, bagasdotme@...il.com, wsa+renesas@...g-engineering.com,
 dave.hansen@...el.com, geert@...ux-m68k.org, ojeda@...nel.org,
 alex.gaynor@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com,
 lossin@...nel.org, a.hindborg@...nel.org, aliceryhl@...gle.com,
 tmgross@...ch.edu, rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v18 34/42] dept: add module support for struct
 dept_event_site and dept_event_site_dep

On 2/13/26 6:50 AM, Byungchul Park wrote:
> On Wed, Jan 07, 2026 at 01:19:00PM +0100, Petr Pavlu wrote:
>> On 12/5/25 8:18 AM, Byungchul Park wrote:
>>> struct dept_event_site and struct dept_event_site_dep have been
>>> introduced to track dependencies between multi event sites for a single
>>> wait, that will be loaded to data segment.  Plus, a custom section,
>>> '.dept.event_sites', also has been introduced to keep pointers to the
>>> objects to make sure all the event sites defined exist in code.
>>>
>>> dept should work with the section and segment of module.  Add the
>>> support to handle the section and segment properly whenever modules are
>>> loaded and unloaded.
>>>
>>> Signed-off-by: Byungchul Park <byungchul@...com>
>>
>> Below are a few comments from the module loader perspective.
> 
> Sorry about the late reply.  I've been going through some major life
> changes lately. :(
> 
> Thank you sooooo~ much for your helpful feedback.  I will leave my
> opinion below.
> 
[...]
>>> diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c
>>> index b14400c4f83b..07d883579269 100644
>>> --- a/kernel/dependency/dept.c
>>> +++ b/kernel/dependency/dept.c
>>> @@ -984,6 +984,9 @@ static void bfs(void *root, struct bfs_ops *ops, void *in, void **out)
>>>   * event sites.
>>>   */
>>>
>>> +static LIST_HEAD(dept_event_sites);
>>> +static LIST_HEAD(dept_event_site_deps);
>>> +
>>>  /*
>>>   * Print all events in the circle.
>>>   */
>>> @@ -2043,6 +2046,33 @@ static void del_dep_rcu(struct rcu_head *rh)
>>>       preempt_enable();
>>>  }
>>>
>>> +/*
>>> + * NOTE: Must be called with dept_lock held.
>>> + */
>>> +static void disconnect_event_site_dep(struct dept_event_site_dep *esd)
>>> +{
>>> +     list_del_rcu(&esd->dep_node);
>>> +     list_del_rcu(&esd->dep_rev_node);
>>> +}
>>> +
>>> +/*
>>> + * NOTE: Must be called with dept_lock held.
>>> + */
>>> +static void disconnect_event_site(struct dept_event_site *es)
>>> +{
>>> +     struct dept_event_site_dep *esd, *next_esd;
>>> +
>>> +     list_for_each_entry_safe(esd, next_esd, &es->dep_head, dep_node) {
>>> +             list_del_rcu(&esd->dep_node);
>>> +             list_del_rcu(&esd->dep_rev_node);
>>> +     }
>>> +
>>> +     list_for_each_entry_safe(esd, next_esd, &es->dep_rev_head, dep_rev_node) {
>>> +             list_del_rcu(&esd->dep_node);
>>> +             list_del_rcu(&esd->dep_rev_node);
>>> +     }
>>> +}
>>> +
>>>  /*
>>>   * NOTE: Must be called with dept_lock held.
>>>   */
>>> @@ -2384,6 +2414,8 @@ void dept_free_range(void *start, unsigned int sz)
>>>  {
>>>       struct dept_task *dt = dept_task();
>>>       struct dept_class *c, *n;
>>> +     struct dept_event_site_dep *esd, *next_esd;
>>> +     struct dept_event_site *es, *next_es;
>>>       unsigned long flags;
>>>
>>>       if (unlikely(!dept_working()))
>>> @@ -2405,6 +2437,24 @@ void dept_free_range(void *start, unsigned int sz)
>>>       while (unlikely(!dept_lock()))
>>>               cpu_relax();
>>>
>>> +     list_for_each_entry_safe(esd, next_esd, &dept_event_site_deps, all_node) {
>>> +             if (!within((void *)esd, start, sz))
>>> +                     continue;
>>> +
>>> +             disconnect_event_site_dep(esd);
>>> +             list_del(&esd->all_node);
>>> +     }
>>> +
>>> +     list_for_each_entry_safe(es, next_es, &dept_event_sites, all_node) {
>>> +             if (!within((void *)es, start, sz) &&
>>> +                 !within(es->name, start, sz) &&
>>> +                 !within(es->func_name, start, sz))
>>> +                     continue;
>>> +
>>> +             disconnect_event_site(es);
>>> +             list_del(&es->all_node);
>>> +     }
>>> +
>>>       list_for_each_entry_safe(c, n, &dept_classes, all_node) {
>>>               if (!within((void *)c->key, start, sz) &&
>>>                   !within(c->name, start, sz))
>>> @@ -3337,6 +3387,7 @@ void __dept_recover_event(struct dept_event_site_dep *esd,
>>>
>>>       list_add(&esd->dep_node, &es->dep_head);
>>>       list_add(&esd->dep_rev_node, &rs->dep_rev_head);
>>> +     list_add(&esd->all_node, &dept_event_site_deps);
>>>       check_recover_dl_bfs(esd);
>>>  unlock:
>>>       dept_unlock();
>>> @@ -3347,6 +3398,23 @@ EXPORT_SYMBOL_GPL(__dept_recover_event);
>>>
>>>  #define B2KB(B) ((B) / 1024)
>>>
>>> +void dept_mark_event_site_used(void *start, void *end)
>>
>> Nit: I suggest that dept_mark_event_site_used() take pointers to
>> dept_event_site_init, which would catch the type mismatch with
> 
> IMO, this is the easiest way to get all the pointers from start to the
> end, or I can't get the number of the pointers.  It's similar to the
> initcalls section for device drivers.

This was a minor suggestion.. The idea is to simply change the function
signature to:

void dept_mark_event_site_used(struct dept_event_site_init **start,
			       struct dept_event_site_init **end))

This way, the compiler can provide proper type checking to ensure that
correct pointers are passed to dept_mark_event_site_used(). It would
catch the type mismatch with module::dept_event_sites.

> 
>> module::dept_event_sites.
>>
>>> +{
>>> +     struct dept_event_site_init **evtinitpp;
>>> +
>>> +     for (evtinitpp = (struct dept_event_site_init **)start;
>>> +          evtinitpp < (struct dept_event_site_init **)end;
>>> +          evtinitpp++) {
>>> +             (*evtinitpp)->evt_site->used = true;
>>> +             (*evtinitpp)->evt_site->func_name = (*evtinitpp)->func_name;
>>> +             list_add(&(*evtinitpp)->evt_site->all_node, &dept_event_sites);
>>> +
>>> +             pr_info("dept_event_site %s@%s is initialized.\n",
>>> +                             (*evtinitpp)->evt_site->name,
>>> +                             (*evtinitpp)->evt_site->func_name);
>>> +     }
>>> +}
>>> +
>>>  extern char __dept_event_sites_start[], __dept_event_sites_end[];
>>
>> Related to the above, __dept_event_sites_start and
>> __dept_event_sites_end can already be properly typed here.
> 
> How can I get the number of the pointers?

Similarly here, changing the code to:

extern struct dept_event_site_init *__dept_event_sites_start[], *__dept_event_sites_end[];

It is the same for the initcalls you mentioned. The declarations of
their start/end symbols are also already properly typed as
initcall_entry_t[] in include/linux/init.h.

-- 
Thanks,
Petr

Powered by blists - more mailing lists