On Wed, Feb 18, 2026 at 04:08:19PM +0100, Petr Pavlu wrote:
> 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 <[email protected]>
> >>
> >> 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))
I got what you meant. I will. Thanks.
Byungchul
> 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
