mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
@ 2026-09-01  9:18 Zqiang
  2026-09-01 17:12 ` Alexander Aring
  2026-09-02 13:21 ` Alexander Aring
  0 siblings, 2 replies; 13+ messages in thread
From: Zqiang @ 2026-09-01  9:18 UTC (permalink / raw)
  To: aahringo, teigland; +Cc: gfs2, linux-kernel, qiang.zhang

The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
srcu protected connection and node hash tables and hand each
element to call_srcu() for deferred freeing (connection_release()
and midcomms_node_release()). call_srcu() is asynchronous: the
callbacks are invoked only after an SRCU grace period, which may
happen after the exit function has already returned.

These exit functions are reached from exit_dlm() on module unload.
Once they return, module teardown continues and the module text
may be unloaded while call_srcu() callbacks are still pending. When
such a callback finally runs, it executes freed module code and
touches the static SRCU domains that are being torn down, resulting
in a use-after-free.

Add an srcu_barrier() after the call_srcu() loop in each exit function
to wait for all outstanding callbacks of the respective SRCU domain to
complete before returning. In dlm_midcomms_exit() the barrier is issued
before dlm_lowcomms_exit() so that node callbacks are drained prior to
tearing down the lower layer.

Signed-off-by: Zqiang <qiang.zhang@linux.dev>
---
 fs/dlm/lowcomms.c | 1 +
 fs/dlm/midcomms.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 2aff1c7c17de..ea8353c4638d 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -1984,4 +1984,5 @@ void dlm_lowcomms_exit(void)
 		}
 	}
 	srcu_read_unlock(&connections_srcu, idx);
+	srcu_barrier(&connections_srcu);
 }
diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
index 8964164600d2..045431524494 100644
--- a/fs/dlm/midcomms.c
+++ b/fs/dlm/midcomms.c
@@ -1178,6 +1178,7 @@ void dlm_midcomms_exit(void)
 		}
 	}
 	srcu_read_unlock(&nodes_srcu, idx);
+	srcu_barrier(&nodes_srcu);
 
 	dlm_lowcomms_exit();
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-01  9:18 [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths Zqiang
@ 2026-09-01 17:12 ` Alexander Aring
  2026-09-02 13:21 ` Alexander Aring
  1 sibling, 0 replies; 13+ messages in thread
From: Alexander Aring @ 2026-09-01 17:12 UTC (permalink / raw)
  To: Zqiang; +Cc: teigland, gfs2, linux-kernel

Hi,

On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
>
> The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> srcu protected connection and node hash tables and hand each
> element to call_srcu() for deferred freeing (connection_release()
> and midcomms_node_release()). call_srcu() is asynchronous: the
> callbacks are invoked only after an SRCU grace period, which may
> happen after the exit function has already returned.
>
> These exit functions are reached from exit_dlm() on module unload.
> Once they return, module teardown continues and the module text
> may be unloaded while call_srcu() callbacks are still pending. When
> such a callback finally runs, it executes freed module code and
> touches the static SRCU domains that are being torn down, resulting
> in a use-after-free.
>
> Add an srcu_barrier() after the call_srcu() loop in each exit function
> to wait for all outstanding callbacks of the respective SRCU domain to
> complete before returning. In dlm_midcomms_exit() the barrier is issued
> before dlm_lowcomms_exit() so that node callbacks are drained prior to
> tearing down the lower layer.
>
> Signed-off-by: Zqiang <qiang.zhang@linux.dev>

Acked-by: Alexander Aring <aahringo@redhat.com>

- Alex


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-01  9:18 [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths Zqiang
  2026-09-01 17:12 ` Alexander Aring
@ 2026-09-02 13:21 ` Alexander Aring
  2026-09-02 14:15   ` Zqiang
  1 sibling, 1 reply; 13+ messages in thread
From: Alexander Aring @ 2026-09-02 13:21 UTC (permalink / raw)
  To: Zqiang; +Cc: teigland, gfs2, linux-kernel, rcu

Hi,

On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
>
> The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> srcu protected connection and node hash tables and hand each
> element to call_srcu() for deferred freeing (connection_release()
> and midcomms_node_release()). call_srcu() is asynchronous: the
> callbacks are invoked only after an SRCU grace period, which may
> happen after the exit function has already returned.
>
> These exit functions are reached from exit_dlm() on module unload.
> Once they return, module teardown continues and the module text
> may be unloaded while call_srcu() callbacks are still pending. When
> such a callback finally runs, it executes freed module code and
> touches the static SRCU domains that are being torn down, resulting
> in a use-after-free.
>

I thought again about this and in my opinion this is not possible as
it is already being handled by DEFINE_STATIC_SRCU() with a cleanup
handling when the module is unloaded.
I know that srcu subsystem does a lot of magic with modules init/exit
functionality to call init_srcu_struct() and cleanup_srcu_struct().
See

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v7.3-rc1#n2711

so this patch should be reverted. If they don't use a barrier there,
the fix should be in the SRCU subsystem, but I believe the current
SRCU implementation already handles this.

- Alex

> Add an srcu_barrier() after the call_srcu() loop in each exit function
> to wait for all outstanding callbacks of the respective SRCU domain to
> complete before returning. In dlm_midcomms_exit() the barrier is issued
> before dlm_lowcomms_exit() so that node callbacks are drained prior to
> tearing down the lower layer.
>
> Signed-off-by: Zqiang <qiang.zhang@linux.dev>
> ---
>  fs/dlm/lowcomms.c | 1 +
>  fs/dlm/midcomms.c | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> index 2aff1c7c17de..ea8353c4638d 100644
> --- a/fs/dlm/lowcomms.c
> +++ b/fs/dlm/lowcomms.c
> @@ -1984,4 +1984,5 @@ void dlm_lowcomms_exit(void)
>                 }
>         }
>         srcu_read_unlock(&connections_srcu, idx);
> +       srcu_barrier(&connections_srcu);
>  }
> diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
> index 8964164600d2..045431524494 100644
> --- a/fs/dlm/midcomms.c
> +++ b/fs/dlm/midcomms.c
> @@ -1178,6 +1178,7 @@ void dlm_midcomms_exit(void)
>                 }
>         }
>         srcu_read_unlock(&nodes_srcu, idx);
> +       srcu_barrier(&nodes_srcu);
>
>         dlm_lowcomms_exit();
>  }
> --
> 2.17.1
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-02 13:21 ` Alexander Aring
@ 2026-09-02 14:15   ` Zqiang
  2026-09-02 14:29     ` Zqiang
  2026-09-02 14:30     ` Alexander Aring
  0 siblings, 2 replies; 13+ messages in thread
From: Zqiang @ 2026-09-02 14:15 UTC (permalink / raw)
  To: Alexander Aring; +Cc: teigland, gfs2, linux-kernel, rcu

> 
> Hi,
> 
> On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
> 
> > 
> > The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> >  srcu protected connection and node hash tables and hand each
> >  element to call_srcu() for deferred freeing (connection_release()
> >  and midcomms_node_release()). call_srcu() is asynchronous: the
> >  callbacks are invoked only after an SRCU grace period, which may
> >  happen after the exit function has already returned.
> > 
> >  These exit functions are reached from exit_dlm() on module unload.
> >  Once they return, module teardown continues and the module text
> >  may be unloaded while call_srcu() callbacks are still pending. When
> >  such a callback finally runs, it executes freed module code and
> >  touches the static SRCU domains that are being torn down, resulting
> >  in a use-after-free.
> > 
> I thought again about this and in my opinion this is not possible as
> it is already being handled by DEFINE_STATIC_SRCU() with a cleanup
> handling when the module is unloaded.

When the moudle unload, the srcu_module_going() will call cleanup_srcu_struct()
and free_percpu(ssp->sda) to release resource. but we not call srcu_barrier(),
the srcu_barrier() should be called before cleanup_srcu_struct().


> I know that srcu subsystem does a lot of magic with modules init/exit
> functionality to call init_srcu_struct() and cleanup_srcu_struct().
> See
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v7.3-rc1#n2711
> 
> so this patch should be reverted. If they don't use a barrier there,
> the fix should be in the SRCU subsystem, but I believe the current
> SRCU implementation already handles this.

The srcu_barrier() need to be called, there are some description
from Documentation/RCU/rcubarrier.rst:

rcu_barrier()
-------------

This situation can be handled by the rcu_barrier() primitive.  Rather
than waiting for a grace period to elapse, rcu_barrier() waits for all
outstanding RCU callbacks to complete.  Please note that rcu_barrier()
does **not** imply synchronize_rcu(), in particular, if there are no RCU
callbacks queued anywhere, rcu_barrier() is within its rights to return
immediately, without waiting for anything, let alone a grace period.

Pseudo-code using rcu_barrier() is as follows:

   1. Prevent any new RCU callbacks from being posted.
   2. Execute rcu_barrier().
   3. Allow the module to be unloaded.

There is also an srcu_barrier() function for SRCU, and you of course
must match the flavor of srcu_barrier() with that of call_srcu().
If your module uses multiple srcu_struct structures, then it must also
use multiple invocations of srcu_barrier() when unloading that module.
For example, if it uses call_rcu(), call_srcu() on srcu_struct_1, and
call_srcu() on srcu_struct_2, then the following three lines of code
will be required when unloading::

  1  rcu_barrier();
  2  srcu_barrier(&srcu_struct_1);
  3  srcu_barrier(&srcu_struct_2);
....

Thanks
Zqiang

> 
> - Alex
> 
> > 
> > Add an srcu_barrier() after the call_srcu() loop in each exit function
> >  to wait for all outstanding callbacks of the respective SRCU domain to
> >  complete before returning. In dlm_midcomms_exit() the barrier is issued
> >  before dlm_lowcomms_exit() so that node callbacks are drained prior to
> >  tearing down the lower layer.
> > 
> >  Signed-off-by: Zqiang <qiang.zhang@linux.dev>
> >  ---
> >  fs/dlm/lowcomms.c | 1 +
> >  fs/dlm/midcomms.c | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> >  diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> >  index 2aff1c7c17de..ea8353c4638d 100644
> >  --- a/fs/dlm/lowcomms.c
> >  +++ b/fs/dlm/lowcomms.c
> >  @@ -1984,4 +1984,5 @@ void dlm_lowcomms_exit(void)
> >  }
> >  }
> >  srcu_read_unlock(&connections_srcu, idx);
> >  + srcu_barrier(&connections_srcu);
> >  }
> >  diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
> >  index 8964164600d2..045431524494 100644
> >  --- a/fs/dlm/midcomms.c
> >  +++ b/fs/dlm/midcomms.c
> >  @@ -1178,6 +1178,7 @@ void dlm_midcomms_exit(void)
> >  }
> >  }
> >  srcu_read_unlock(&nodes_srcu, idx);
> >  + srcu_barrier(&nodes_srcu);
> > 
> >  dlm_lowcomms_exit();
> >  }
> >  --
> >  2.17.1
> >
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-02 14:15   ` Zqiang
@ 2026-09-02 14:29     ` Zqiang
  2026-09-02 14:36       ` Alexander Aring
  2026-09-02 14:30     ` Alexander Aring
  1 sibling, 1 reply; 13+ messages in thread
From: Zqiang @ 2026-09-02 14:29 UTC (permalink / raw)
  To: Alexander Aring; +Cc: teigland, gfs2, linux-kernel, rcu

> 
> > 
> > Hi,
> >  
> >  On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
> >  
> >  
> >  The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> >  srcu protected connection and node hash tables and hand each
> >  element to call_srcu() for deferred freeing (connection_release()
> >  and midcomms_node_release()). call_srcu() is asynchronous: the
> >  callbacks are invoked only after an SRCU grace period, which may
> >  happen after the exit function has already returned.
> >  
> >  These exit functions are reached from exit_dlm() on module unload.
> >  Once they return, module teardown continues and the module text
> >  may be unloaded while call_srcu() callbacks are still pending. When
> >  such a callback finally runs, it executes freed module code and
> >  touches the static SRCU domains that are being torn down, resulting
> >  in a use-after-free.

     I should rephrase it:

     Due to missed call to the srcu_barrier(), when invoke cleanup_srcu_struct()
     and find there are still pending srcu callback, the WARN_ON() will trigger.

     from kernel/rcu/srcutree.c in cleanup_srcu_struct():


                // Call srcu_barrier() before this cleanup_srcu_struct()
                // to avoid triggering this WARN_ON().
                if (WARN_ON(timer_delete_sync(&sdp->delay_work) &&
                            rcu_segcblist_n_cbs(&sdp->srcu_cblist)) &&
                    rcu_cpu_beenfullyonline(sdp->cpu))
                        queue_work_on(sdp->cpu, rcu_gp_wq, &sdp->work);
                flush_work(&sdp->work);

Thanks
Zqiang

> >  
> >  I thought again about this and in my opinion this is not possible as
> >  it is already being handled by DEFINE_STATIC_SRCU() with a cleanup
> >  handling when the module is unloaded.
> > 
> When the moudle unload, the srcu_module_going() will call cleanup_srcu_struct()
> and free_percpu(ssp->sda) to release resource. but we not call srcu_barrier(),
> the srcu_barrier() should be called before cleanup_srcu_struct().
> 
> > 
> > I know that srcu subsystem does a lot of magic with modules init/exit
> >  functionality to call init_srcu_struct() and cleanup_srcu_struct().
> >  See
> >  
> >  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v7.3-rc1#n2711
> >  
> >  so this patch should be reverted. If they don't use a barrier there,
> >  the fix should be in the SRCU subsystem, but I believe the current
> >  SRCU implementation already handles this.
> > 
> The srcu_barrier() need to be called, there are some description
> from Documentation/RCU/rcubarrier.rst:
> 
> rcu_barrier()
> -------------
> 
> This situation can be handled by the rcu_barrier() primitive. Rather
> than waiting for a grace period to elapse, rcu_barrier() waits for all
> outstanding RCU callbacks to complete. Please note that rcu_barrier()
> does **not** imply synchronize_rcu(), in particular, if there are no RCU
> callbacks queued anywhere, rcu_barrier() is within its rights to return
> immediately, without waiting for anything, let alone a grace period.
> 
> Pseudo-code using rcu_barrier() is as follows:
> 
>  1. Prevent any new RCU callbacks from being posted.
>  2. Execute rcu_barrier().
>  3. Allow the module to be unloaded.
> 
> There is also an srcu_barrier() function for SRCU, and you of course
> must match the flavor of srcu_barrier() with that of call_srcu().
> If your module uses multiple srcu_struct structures, then it must also
> use multiple invocations of srcu_barrier() when unloading that module.
> For example, if it uses call_rcu(), call_srcu() on srcu_struct_1, and
> call_srcu() on srcu_struct_2, then the following three lines of code
> will be required when unloading::
> 
>  1 rcu_barrier();
>  2 srcu_barrier(&srcu_struct_1);
>  3 srcu_barrier(&srcu_struct_2);
> ....
> 
> Thanks
> Zqiang
> 
> > 
> > - Alex
> >  
> >  
> >  Add an srcu_barrier() after the call_srcu() loop in each exit function
> >  to wait for all outstanding callbacks of the respective SRCU domain to
> >  complete before returning. In dlm_midcomms_exit() the barrier is issued
> >  before dlm_lowcomms_exit() so that node callbacks are drained prior to
> >  tearing down the lower layer.
> >  
> >  Signed-off-by: Zqiang <qiang.zhang@linux.dev>
> >  ---
> >  fs/dlm/lowcomms.c | 1 +
> >  fs/dlm/midcomms.c | 1 +
> >  2 files changed, 2 insertions(+)
> >  
> >  diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> >  index 2aff1c7c17de..ea8353c4638d 100644
> >  --- a/fs/dlm/lowcomms.c
> >  +++ b/fs/dlm/lowcomms.c
> >  @@ -1984,4 +1984,5 @@ void dlm_lowcomms_exit(void)
> >  }
> >  }
> >  srcu_read_unlock(&connections_srcu, idx);
> >  + srcu_barrier(&connections_srcu);
> >  }
> >  diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
> >  index 8964164600d2..045431524494 100644
> >  --- a/fs/dlm/midcomms.c
> >  +++ b/fs/dlm/midcomms.c
> >  @@ -1178,6 +1178,7 @@ void dlm_midcomms_exit(void)
> >  }
> >  }
> >  srcu_read_unlock(&nodes_srcu, idx);
> >  + srcu_barrier(&nodes_srcu);
> >  
> >  dlm_lowcomms_exit();
> >  }
> >  --
> >  2.17.1
> >
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-02 14:15   ` Zqiang
  2026-09-02 14:29     ` Zqiang
@ 2026-09-02 14:30     ` Alexander Aring
  2026-09-02 14:48       ` Alexander Aring
  2026-09-02 16:44       ` Paul E. McKenney
  1 sibling, 2 replies; 13+ messages in thread
From: Alexander Aring @ 2026-09-02 14:30 UTC (permalink / raw)
  To: Zqiang; +Cc: teigland, gfs2, linux-kernel, rcu

Hi,

On Wed, Sep 2, 2026 at 10:16 AM Zqiang <qiang.zhang@linux.dev> wrote:
>
> >
> > Hi,
> >
> > On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
> >
> > >
> > > The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> > >  srcu protected connection and node hash tables and hand each
> > >  element to call_srcu() for deferred freeing (connection_release()
> > >  and midcomms_node_release()). call_srcu() is asynchronous: the
> > >  callbacks are invoked only after an SRCU grace period, which may
> > >  happen after the exit function has already returned.
> > >
> > >  These exit functions are reached from exit_dlm() on module unload.
> > >  Once they return, module teardown continues and the module text
> > >  may be unloaded while call_srcu() callbacks are still pending. When
> > >  such a callback finally runs, it executes freed module code and
> > >  touches the static SRCU domains that are being torn down, resulting
> > >  in a use-after-free.
> > >
> > I thought again about this and in my opinion this is not possible as
> > it is already being handled by DEFINE_STATIC_SRCU() with a cleanup
> > handling when the module is unloaded.
>
> When the moudle unload, the srcu_module_going() will call cleanup_srcu_struct()
> and free_percpu(ssp->sda) to release resource. but we not call srcu_barrier(),
> the srcu_barrier() should be called before cleanup_srcu_struct().
>
>
> > I know that srcu subsystem does a lot of magic with modules init/exit
> > functionality to call init_srcu_struct() and cleanup_srcu_struct().
> > See
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v7.3-rc1#n2711
> >
> > so this patch should be reverted. If they don't use a barrier there,
> > the fix should be in the SRCU subsystem, but I believe the current
> > SRCU implementation already handles this.
>
> The srcu_barrier() need to be called, there are some description
> from Documentation/RCU/rcubarrier.rst:
>
> rcu_barrier()
> -------------
>
> This situation can be handled by the rcu_barrier() primitive.  Rather
> than waiting for a grace period to elapse, rcu_barrier() waits for all
> outstanding RCU callbacks to complete.  Please note that rcu_barrier()
> does **not** imply synchronize_rcu(), in particular, if there are no RCU
> callbacks queued anywhere, rcu_barrier() is within its rights to return
> immediately, without waiting for anything, let alone a grace period.
>
> Pseudo-code using rcu_barrier() is as follows:
>
>    1. Prevent any new RCU callbacks from being posted.
>    2. Execute rcu_barrier().
>    3. Allow the module to be unloaded.
>
> There is also an srcu_barrier() function for SRCU, and you of course
> must match the flavor of srcu_barrier() with that of call_srcu().
> If your module uses multiple srcu_struct structures, then it must also
> use multiple invocations of srcu_barrier() when unloading that module.
> For example, if it uses call_rcu(), call_srcu() on srcu_struct_1, and
> call_srcu() on srcu_struct_2, then the following three lines of code
> will be required when unloading::
>
>   1  rcu_barrier();
>   2  srcu_barrier(&srcu_struct_1);
>   3  srcu_barrier(&srcu_struct_2);
> ....

yes, I can see this makes problems when there is kmem_cache involved
and you need to be sure it's being done before you destroy the
kmem_cache.

This is not the case here.

I looked more into srcu functionality "cleanup_srcu_struct()"
(srcutiny) and it does "flush_work(&ssp->srcu_work)", workfn is
"srcu_drive_gp()" and the comment states "Workqueue handler to drive
one grace period and invoke any callbacks that become ready as a
result."

In my opinion "cleanup_srcu_struct()" should be sure there are no
pending operations.

- Alex


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-02 14:29     ` Zqiang
@ 2026-09-02 14:36       ` Alexander Aring
  2026-09-02 14:49         ` Zqiang
  0 siblings, 1 reply; 13+ messages in thread
From: Alexander Aring @ 2026-09-02 14:36 UTC (permalink / raw)
  To: Zqiang; +Cc: teigland, gfs2, linux-kernel, rcu

Hi,

On Wed, Sep 2, 2026 at 10:30 AM Zqiang <qiang.zhang@linux.dev> wrote:
>
> >
> > >
> > > Hi,
> > >
> > >  On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
> > >
> > >
> > >  The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> > >  srcu protected connection and node hash tables and hand each
> > >  element to call_srcu() for deferred freeing (connection_release()
> > >  and midcomms_node_release()). call_srcu() is asynchronous: the
> > >  callbacks are invoked only after an SRCU grace period, which may
> > >  happen after the exit function has already returned.
> > >
> > >  These exit functions are reached from exit_dlm() on module unload.
> > >  Once they return, module teardown continues and the module text
> > >  may be unloaded while call_srcu() callbacks are still pending. When
> > >  such a callback finally runs, it executes freed module code and
> > >  touches the static SRCU domains that are being torn down, resulting
> > >  in a use-after-free.
>
>      I should rephrase it:
>
>      Due to missed call to the srcu_barrier(), when invoke cleanup_srcu_struct()

Then there need to be a MUST call of srcu_barrier() before
cleanup_srcu_struct() every time.

I am confused here.

- Alex


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-02 14:30     ` Alexander Aring
@ 2026-09-02 14:48       ` Alexander Aring
  2026-09-02 16:44       ` Paul E. McKenney
  1 sibling, 0 replies; 13+ messages in thread
From: Alexander Aring @ 2026-09-02 14:48 UTC (permalink / raw)
  To: Zqiang; +Cc: teigland, gfs2, linux-kernel, rcu

Hi,

On Wed, Sep 2, 2026 at 10:30 AM Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Wed, Sep 2, 2026 at 10:16 AM Zqiang <qiang.zhang@linux.dev> wrote:
> >
> > >
> > > Hi,
> > >
> > > On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
> > >
> > > >
> > > > The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> > > >  srcu protected connection and node hash tables and hand each
> > > >  element to call_srcu() for deferred freeing (connection_release()
> > > >  and midcomms_node_release()). call_srcu() is asynchronous: the
> > > >  callbacks are invoked only after an SRCU grace period, which may
> > > >  happen after the exit function has already returned.
> > > >
> > > >  These exit functions are reached from exit_dlm() on module unload.
> > > >  Once they return, module teardown continues and the module text
> > > >  may be unloaded while call_srcu() callbacks are still pending. When
> > > >  such a callback finally runs, it executes freed module code and
> > > >  touches the static SRCU domains that are being torn down, resulting
> > > >  in a use-after-free.
> > > >
> > > I thought again about this and in my opinion this is not possible as
> > > it is already being handled by DEFINE_STATIC_SRCU() with a cleanup
> > > handling when the module is unloaded.
> >
> > When the moudle unload, the srcu_module_going() will call cleanup_srcu_struct()
> > and free_percpu(ssp->sda) to release resource. but we not call srcu_barrier(),
> > the srcu_barrier() should be called before cleanup_srcu_struct().
> >
> >
> > > I know that srcu subsystem does a lot of magic with modules init/exit
> > > functionality to call init_srcu_struct() and cleanup_srcu_struct().
> > > See
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v7.3-rc1#n2711
> > >
> > > so this patch should be reverted. If they don't use a barrier there,
> > > the fix should be in the SRCU subsystem, but I believe the current
> > > SRCU implementation already handles this.
> >
> > The srcu_barrier() need to be called, there are some description
> > from Documentation/RCU/rcubarrier.rst:
> >
> > rcu_barrier()
> > -------------
> >
> > This situation can be handled by the rcu_barrier() primitive.  Rather
> > than waiting for a grace period to elapse, rcu_barrier() waits for all
> > outstanding RCU callbacks to complete.  Please note that rcu_barrier()
> > does **not** imply synchronize_rcu(), in particular, if there are no RCU
> > callbacks queued anywhere, rcu_barrier() is within its rights to return
> > immediately, without waiting for anything, let alone a grace period.
> >
> > Pseudo-code using rcu_barrier() is as follows:
> >
> >    1. Prevent any new RCU callbacks from being posted.
> >    2. Execute rcu_barrier().
> >    3. Allow the module to be unloaded.
> >
> > There is also an srcu_barrier() function for SRCU, and you of course
> > must match the flavor of srcu_barrier() with that of call_srcu().
> > If your module uses multiple srcu_struct structures, then it must also
> > use multiple invocations of srcu_barrier() when unloading that module.
> > For example, if it uses call_rcu(), call_srcu() on srcu_struct_1, and
> > call_srcu() on srcu_struct_2, then the following three lines of code
> > will be required when unloading::
> >
> >   1  rcu_barrier();
> >   2  srcu_barrier(&srcu_struct_1);
> >   3  srcu_barrier(&srcu_struct_2);
> > ....
>
> yes, I can see this makes problems when there is kmem_cache involved
> and you need to be sure it's being done before you destroy the
> kmem_cache.
>

okay, it can be the case for "nodes_srcu" but with the argumentation
it can release memory being used by kmem_cache that is being destroyed
later.

- Alex


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-02 14:36       ` Alexander Aring
@ 2026-09-02 14:49         ` Zqiang
  2026-09-02 15:02           ` Alexander Aring
  0 siblings, 1 reply; 13+ messages in thread
From: Zqiang @ 2026-09-02 14:49 UTC (permalink / raw)
  To: Alexander Aring; +Cc: teigland, gfs2, linux-kernel, rcu

> 
> Hi,
> 
> On Wed, Sep 2, 2026 at 10:30 AM Zqiang <qiang.zhang@linux.dev> wrote:
> 
> > 
> > >
> >  > Hi,
> >  >
> >  > On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
> >  >
> >  >
> >  > The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> >  > srcu protected connection and node hash tables and hand each
> >  > element to call_srcu() for deferred freeing (connection_release()
> >  > and midcomms_node_release()). call_srcu() is asynchronous: the
> >  > callbacks are invoked only after an SRCU grace period, which may
> >  > happen after the exit function has already returned.
> >  >
> >  > These exit functions are reached from exit_dlm() on module unload.
> >  > Once they return, module teardown continues and the module text
> >  > may be unloaded while call_srcu() callbacks are still pending. When
> >  > such a callback finally runs, it executes freed module code and
> >  > touches the static SRCU domains that are being torn down, resulting
> >  > in a use-after-free.
> > 
> >  I should rephrase it:
> > 
> >  Due to missed call to the srcu_barrier(), when invoke cleanup_srcu_struct()
> > 
> Then there need to be a MUST call of srcu_barrier() before
> cleanup_srcu_struct() every time.

If there are some call_srcu() call, the srcu_barrier() should be called before cleanup_srcu_struct().
usually, when the cleanup_srcu_struct() run, we should ensure that there are no pending SRCU callbacks
or ongoing SRCU grace periods at this time.

there are another reason:

The midcomms_node_release() srcu callback can call call_rcu(), we must wait the all
call_rcu() to complete, because rcu_barrier() which in dlm_memory_exit() is required
to successfully intercept these RCU callbacks.

Thanks
Zqiang

> 
> I am confused here.
> 
> - Alex
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-02 14:49         ` Zqiang
@ 2026-09-02 15:02           ` Alexander Aring
  0 siblings, 0 replies; 13+ messages in thread
From: Alexander Aring @ 2026-09-02 15:02 UTC (permalink / raw)
  To: Zqiang; +Cc: teigland, gfs2, linux-kernel, rcu

Hi,

On Wed, Sep 2, 2026 at 10:49 AM Zqiang <qiang.zhang@linux.dev> wrote:
>
> >
> > Hi,
> >
> > On Wed, Sep 2, 2026 at 10:30 AM Zqiang <qiang.zhang@linux.dev> wrote:
> >
> > >
> > > >
> > >  > Hi,
> > >  >
> > >  > On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
> > >  >
> > >  >
> > >  > The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> > >  > srcu protected connection and node hash tables and hand each
> > >  > element to call_srcu() for deferred freeing (connection_release()
> > >  > and midcomms_node_release()). call_srcu() is asynchronous: the
> > >  > callbacks are invoked only after an SRCU grace period, which may
> > >  > happen after the exit function has already returned.
> > >  >
> > >  > These exit functions are reached from exit_dlm() on module unload.
> > >  > Once they return, module teardown continues and the module text
> > >  > may be unloaded while call_srcu() callbacks are still pending. When
> > >  > such a callback finally runs, it executes freed module code and
> > >  > touches the static SRCU domains that are being torn down, resulting
> > >  > in a use-after-free.
> > >
> > >  I should rephrase it:
> > >
> > >  Due to missed call to the srcu_barrier(), when invoke cleanup_srcu_struct()
> > >
> > Then there need to be a MUST call of srcu_barrier() before
> > cleanup_srcu_struct() every time.
>
> If there are some call_srcu() call, the srcu_barrier() should be called before cleanup_srcu_struct().
> usually, when the cleanup_srcu_struct() run, we should ensure that there are no pending SRCU callbacks
> or ongoing SRCU grace periods at this time.
>

For me, this is a MUST because who is maintaining something to know if
it was ever called?

> there are another reason:
>
> The midcomms_node_release() srcu callback can call call_rcu(), we must wait the all
> call_rcu() to complete, because rcu_barrier() which in dlm_memory_exit() is required
> to successfully intercept these RCU callbacks.
>

Yes, I can see why "nodes_srcu" is necessary as this has something to
do with kmem_cache is being involed.

The "connections_srcu" is necessary for the above reason that the srcu
API requires you to call srcu_barrier() before cleanup_srcu_struct()
because for whatever reason we don't do that in cleanup_srcu_struct()
as I understand it now.

- Alex


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-02 14:30     ` Alexander Aring
  2026-09-02 14:48       ` Alexander Aring
@ 2026-09-02 16:44       ` Paul E. McKenney
  2026-09-02 17:13         ` Alexander Aring
  1 sibling, 1 reply; 13+ messages in thread
From: Paul E. McKenney @ 2026-09-02 16:44 UTC (permalink / raw)
  To: Alexander Aring; +Cc: Zqiang, teigland, gfs2, linux-kernel, rcu

On Wed, Sep 02, 2026 at 10:30:15AM -0400, Alexander Aring wrote:
> Hi,
> 
> On Wed, Sep 2, 2026 at 10:16 AM Zqiang <qiang.zhang@linux.dev> wrote:
> >
> > >
> > > Hi,
> > >
> > > On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
> > >
> > > >
> > > > The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> > > >  srcu protected connection and node hash tables and hand each
> > > >  element to call_srcu() for deferred freeing (connection_release()
> > > >  and midcomms_node_release()). call_srcu() is asynchronous: the
> > > >  callbacks are invoked only after an SRCU grace period, which may
> > > >  happen after the exit function has already returned.
> > > >
> > > >  These exit functions are reached from exit_dlm() on module unload.
> > > >  Once they return, module teardown continues and the module text
> > > >  may be unloaded while call_srcu() callbacks are still pending. When
> > > >  such a callback finally runs, it executes freed module code and
> > > >  touches the static SRCU domains that are being torn down, resulting
> > > >  in a use-after-free.
> > > >
> > > I thought again about this and in my opinion this is not possible as
> > > it is already being handled by DEFINE_STATIC_SRCU() with a cleanup
> > > handling when the module is unloaded.
> >
> > When the moudle unload, the srcu_module_going() will call cleanup_srcu_struct()
> > and free_percpu(ssp->sda) to release resource. but we not call srcu_barrier(),
> > the srcu_barrier() should be called before cleanup_srcu_struct().
> >
> >
> > > I know that srcu subsystem does a lot of magic with modules init/exit
> > > functionality to call init_srcu_struct() and cleanup_srcu_struct().
> > > See
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v7.3-rc1#n2711
> > >
> > > so this patch should be reverted. If they don't use a barrier there,
> > > the fix should be in the SRCU subsystem, but I believe the current
> > > SRCU implementation already handles this.
> >
> > The srcu_barrier() need to be called, there are some description
> > from Documentation/RCU/rcubarrier.rst:
> >
> > rcu_barrier()
> > -------------
> >
> > This situation can be handled by the rcu_barrier() primitive.  Rather
> > than waiting for a grace period to elapse, rcu_barrier() waits for all
> > outstanding RCU callbacks to complete.  Please note that rcu_barrier()
> > does **not** imply synchronize_rcu(), in particular, if there are no RCU
> > callbacks queued anywhere, rcu_barrier() is within its rights to return
> > immediately, without waiting for anything, let alone a grace period.
> >
> > Pseudo-code using rcu_barrier() is as follows:
> >
> >    1. Prevent any new RCU callbacks from being posted.
> >    2. Execute rcu_barrier().
> >    3. Allow the module to be unloaded.
> >
> > There is also an srcu_barrier() function for SRCU, and you of course
> > must match the flavor of srcu_barrier() with that of call_srcu().
> > If your module uses multiple srcu_struct structures, then it must also
> > use multiple invocations of srcu_barrier() when unloading that module.
> > For example, if it uses call_rcu(), call_srcu() on srcu_struct_1, and
> > call_srcu() on srcu_struct_2, then the following three lines of code
> > will be required when unloading::
> >
> >   1  rcu_barrier();
> >   2  srcu_barrier(&srcu_struct_1);
> >   3  srcu_barrier(&srcu_struct_2);
> > ....
> 
> yes, I can see this makes problems when there is kmem_cache involved
> and you need to be sure it's being done before you destroy the
> kmem_cache.
> 
> This is not the case here.
> 
> I looked more into srcu functionality "cleanup_srcu_struct()"
> (srcutiny) and it does "flush_work(&ssp->srcu_work)", workfn is
> "srcu_drive_gp()" and the comment states "Workqueue handler to drive
> one grace period and invoke any callbacks that become ready as a
> result."
> 
> In my opinion "cleanup_srcu_struct()" should be sure there are no
> pending operations.

If the user never passed this srcu_struct structure to call_srcu(),
then there would be nothing to clean up.  Now, maybe all SRCU use cases
are OK with the extra srcu_barrier() overhead at cleanup_srcu_struct()
time, but as far as I know, that is still a "maybe".

And of course, cleanup_srcu_struct() has no ability to prevent the
user from doing a (buggy!) concurrent call to synchronize_srcu() or
call_srcu(), which would defeat any attempt by cleanup_srcu_struct()
to do this pending-operation cleanup.

Or am I missing your point?

							Thanx, Paul

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-02 16:44       ` Paul E. McKenney
@ 2026-09-02 17:13         ` Alexander Aring
  2026-09-02 20:05           ` Paul E. McKenney
  0 siblings, 1 reply; 13+ messages in thread
From: Alexander Aring @ 2026-09-02 17:13 UTC (permalink / raw)
  To: paulmck; +Cc: Zqiang, teigland, gfs2, linux-kernel, rcu

Hi,

On Wed, Sep 2, 2026 at 12:50 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Wed, Sep 02, 2026 at 10:30:15AM -0400, Alexander Aring wrote:
> > Hi,
> >
> > On Wed, Sep 2, 2026 at 10:16 AM Zqiang <qiang.zhang@linux.dev> wrote:
> > >
> > > >
> > > > Hi,
> > > >
> > > > On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
> > > >
> > > > >
> > > > > The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> > > > >  srcu protected connection and node hash tables and hand each
> > > > >  element to call_srcu() for deferred freeing (connection_release()
> > > > >  and midcomms_node_release()). call_srcu() is asynchronous: the
> > > > >  callbacks are invoked only after an SRCU grace period, which may
> > > > >  happen after the exit function has already returned.
> > > > >
> > > > >  These exit functions are reached from exit_dlm() on module unload.
> > > > >  Once they return, module teardown continues and the module text
> > > > >  may be unloaded while call_srcu() callbacks are still pending. When
> > > > >  such a callback finally runs, it executes freed module code and
> > > > >  touches the static SRCU domains that are being torn down, resulting
> > > > >  in a use-after-free.
> > > > >
> > > > I thought again about this and in my opinion this is not possible as
> > > > it is already being handled by DEFINE_STATIC_SRCU() with a cleanup
> > > > handling when the module is unloaded.
> > >
> > > When the moudle unload, the srcu_module_going() will call cleanup_srcu_struct()
> > > and free_percpu(ssp->sda) to release resource. but we not call srcu_barrier(),
> > > the srcu_barrier() should be called before cleanup_srcu_struct().
> > >
> > >
> > > > I know that srcu subsystem does a lot of magic with modules init/exit
> > > > functionality to call init_srcu_struct() and cleanup_srcu_struct().
> > > > See
> > > >
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v7.3-rc1#n2711
> > > >
> > > > so this patch should be reverted. If they don't use a barrier there,
> > > > the fix should be in the SRCU subsystem, but I believe the current
> > > > SRCU implementation already handles this.
> > >
> > > The srcu_barrier() need to be called, there are some description
> > > from Documentation/RCU/rcubarrier.rst:
> > >
> > > rcu_barrier()
> > > -------------
> > >
> > > This situation can be handled by the rcu_barrier() primitive.  Rather
> > > than waiting for a grace period to elapse, rcu_barrier() waits for all
> > > outstanding RCU callbacks to complete.  Please note that rcu_barrier()
> > > does **not** imply synchronize_rcu(), in particular, if there are no RCU
> > > callbacks queued anywhere, rcu_barrier() is within its rights to return
> > > immediately, without waiting for anything, let alone a grace period.
> > >
> > > Pseudo-code using rcu_barrier() is as follows:
> > >
> > >    1. Prevent any new RCU callbacks from being posted.
> > >    2. Execute rcu_barrier().
> > >    3. Allow the module to be unloaded.
> > >
> > > There is also an srcu_barrier() function for SRCU, and you of course
> > > must match the flavor of srcu_barrier() with that of call_srcu().
> > > If your module uses multiple srcu_struct structures, then it must also
> > > use multiple invocations of srcu_barrier() when unloading that module.
> > > For example, if it uses call_rcu(), call_srcu() on srcu_struct_1, and
> > > call_srcu() on srcu_struct_2, then the following three lines of code
> > > will be required when unloading::
> > >
> > >   1  rcu_barrier();
> > >   2  srcu_barrier(&srcu_struct_1);
> > >   3  srcu_barrier(&srcu_struct_2);
> > > ....
> >
> > yes, I can see this makes problems when there is kmem_cache involved
> > and you need to be sure it's being done before you destroy the
> > kmem_cache.
> >
> > This is not the case here.
> >
> > I looked more into srcu functionality "cleanup_srcu_struct()"
> > (srcutiny) and it does "flush_work(&ssp->srcu_work)", workfn is
> > "srcu_drive_gp()" and the comment states "Workqueue handler to drive
> > one grace period and invoke any callbacks that become ready as a
> > result."
> >
> > In my opinion "cleanup_srcu_struct()" should be sure there are no
> > pending operations.
>
> If the user never passed this srcu_struct structure to call_srcu(),
> then there would be nothing to clean up.  Now, maybe all SRCU use cases
> are OK with the extra srcu_barrier() overhead at cleanup_srcu_struct()
> time, but as far as I know, that is still a "maybe".
>

Okay, I agree with the "maybe" argument, then maybe we add a comment
to "cleanup_srcu_struct()" that there must be a "srcu_barrier()"
called before. Except "call_srcu()" is never being used.

> And of course, cleanup_srcu_struct() has no ability to prevent the
> user from doing a (buggy!) concurrent call to synchronize_srcu() or
> call_srcu(), which would defeat any attempt by cleanup_srcu_struct()
> to do this pending-operation cleanup.
>
> Or am I missing your point?

I use "DEFINE_STATIC_SRCU()" that does the srcu cleanup for me and I
was confused I need to care about "srcu_barrier()" before module exit.
Then "DEFINE_STATIC_SRCU()" needs a comment as well to call
"srcu_barrier()" before module exit functionality as my initial
thought would be it would do that for me as I am even not a caller of
"cleanup_srcu_struct()".

All of this, of course, should happen in a safe context. Wheras for
"DEFINE_STATIC_SRCU()" I would assume after users module exit function
there is nothing going on anymore.

- Alex


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths
  2026-09-02 17:13         ` Alexander Aring
@ 2026-09-02 20:05           ` Paul E. McKenney
  0 siblings, 0 replies; 13+ messages in thread
From: Paul E. McKenney @ 2026-09-02 20:05 UTC (permalink / raw)
  To: Alexander Aring; +Cc: Zqiang, teigland, gfs2, linux-kernel, rcu

On Wed, Sep 02, 2026 at 01:13:50PM -0400, Alexander Aring wrote:
> Hi,
> 
> On Wed, Sep 2, 2026 at 12:50 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Wed, Sep 02, 2026 at 10:30:15AM -0400, Alexander Aring wrote:
> > > Hi,
> > >
> > > On Wed, Sep 2, 2026 at 10:16 AM Zqiang <qiang.zhang@linux.dev> wrote:
> > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> wrote:
> > > > >
> > > > > >
> > > > > > The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the
> > > > > >  srcu protected connection and node hash tables and hand each
> > > > > >  element to call_srcu() for deferred freeing (connection_release()
> > > > > >  and midcomms_node_release()). call_srcu() is asynchronous: the
> > > > > >  callbacks are invoked only after an SRCU grace period, which may
> > > > > >  happen after the exit function has already returned.
> > > > > >
> > > > > >  These exit functions are reached from exit_dlm() on module unload.
> > > > > >  Once they return, module teardown continues and the module text
> > > > > >  may be unloaded while call_srcu() callbacks are still pending. When
> > > > > >  such a callback finally runs, it executes freed module code and
> > > > > >  touches the static SRCU domains that are being torn down, resulting
> > > > > >  in a use-after-free.
> > > > > >
> > > > > I thought again about this and in my opinion this is not possible as
> > > > > it is already being handled by DEFINE_STATIC_SRCU() with a cleanup
> > > > > handling when the module is unloaded.
> > > >
> > > > When the moudle unload, the srcu_module_going() will call cleanup_srcu_struct()
> > > > and free_percpu(ssp->sda) to release resource. but we not call srcu_barrier(),
> > > > the srcu_barrier() should be called before cleanup_srcu_struct().
> > > >
> > > >
> > > > > I know that srcu subsystem does a lot of magic with modules init/exit
> > > > > functionality to call init_srcu_struct() and cleanup_srcu_struct().
> > > > > See
> > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v7.3-rc1#n2711
> > > > >
> > > > > so this patch should be reverted. If they don't use a barrier there,
> > > > > the fix should be in the SRCU subsystem, but I believe the current
> > > > > SRCU implementation already handles this.
> > > >
> > > > The srcu_barrier() need to be called, there are some description
> > > > from Documentation/RCU/rcubarrier.rst:
> > > >
> > > > rcu_barrier()
> > > > -------------
> > > >
> > > > This situation can be handled by the rcu_barrier() primitive.  Rather
> > > > than waiting for a grace period to elapse, rcu_barrier() waits for all
> > > > outstanding RCU callbacks to complete.  Please note that rcu_barrier()
> > > > does **not** imply synchronize_rcu(), in particular, if there are no RCU
> > > > callbacks queued anywhere, rcu_barrier() is within its rights to return
> > > > immediately, without waiting for anything, let alone a grace period.
> > > >
> > > > Pseudo-code using rcu_barrier() is as follows:
> > > >
> > > >    1. Prevent any new RCU callbacks from being posted.
> > > >    2. Execute rcu_barrier().
> > > >    3. Allow the module to be unloaded.
> > > >
> > > > There is also an srcu_barrier() function for SRCU, and you of course
> > > > must match the flavor of srcu_barrier() with that of call_srcu().
> > > > If your module uses multiple srcu_struct structures, then it must also
> > > > use multiple invocations of srcu_barrier() when unloading that module.
> > > > For example, if it uses call_rcu(), call_srcu() on srcu_struct_1, and
> > > > call_srcu() on srcu_struct_2, then the following three lines of code
> > > > will be required when unloading::
> > > >
> > > >   1  rcu_barrier();
> > > >   2  srcu_barrier(&srcu_struct_1);
> > > >   3  srcu_barrier(&srcu_struct_2);
> > > > ....
> > >
> > > yes, I can see this makes problems when there is kmem_cache involved
> > > and you need to be sure it's being done before you destroy the
> > > kmem_cache.
> > >
> > > This is not the case here.
> > >
> > > I looked more into srcu functionality "cleanup_srcu_struct()"
> > > (srcutiny) and it does "flush_work(&ssp->srcu_work)", workfn is
> > > "srcu_drive_gp()" and the comment states "Workqueue handler to drive
> > > one grace period and invoke any callbacks that become ready as a
> > > result."
> > >
> > > In my opinion "cleanup_srcu_struct()" should be sure there are no
> > > pending operations.
> >
> > If the user never passed this srcu_struct structure to call_srcu(),
> > then there would be nothing to clean up.  Now, maybe all SRCU use cases
> > are OK with the extra srcu_barrier() overhead at cleanup_srcu_struct()
> > time, but as far as I know, that is still a "maybe".
> >
> 
> Okay, I agree with the "maybe" argument, then maybe we add a comment
> to "cleanup_srcu_struct()" that there must be a "srcu_barrier()"
> called before. Except "call_srcu()" is never being used.
> 
> > And of course, cleanup_srcu_struct() has no ability to prevent the
> > user from doing a (buggy!) concurrent call to synchronize_srcu() or
> > call_srcu(), which would defeat any attempt by cleanup_srcu_struct()
> > to do this pending-operation cleanup.
> >
> > Or am I missing your point?
> 
> I use "DEFINE_STATIC_SRCU()" that does the srcu cleanup for me and I
> was confused I need to care about "srcu_barrier()" before module exit.
> Then "DEFINE_STATIC_SRCU()" needs a comment as well to call
> "srcu_barrier()" before module exit functionality as my initial
> thought would be it would do that for me as I am even not a caller of
> "cleanup_srcu_struct()".
> 
> All of this, of course, should happen in a safe context. Wheras for
> "DEFINE_STATIC_SRCU()" I would assume after users module exit function
> there is nothing going on anymore.

Fair point!  How about this?

Now I am wondering if the various DEFINE_SRCU() macros need kernel-doc
headers...

							Thanx, Paul

------------------------------------------------------------------------

commit 2a20fcf8bd8ffffd0e7cdd0adf80e41d1efdfc2b
Author: Paul E. McKenney <paulmck@kernel.org>
Date:   Wed Sep 2 12:58:24 2026 -0700

    srcutree: Explicitly note DEFINE_SRCU() needs for srcu_barrier()
    
    In the core kernel, an srcu_struct structure created by DEFINE_SRCU()
    or friends lives as long as the kernel does, so there are no particular
    requirements surrounding the end of that structure's life.  In contrast,
    when DEFINE_SRCU() and friends are used within a module, the corresponding
    srcu_struct structures' lifetimes end when that module exits.  This in
    turn means that if such a structure was passed to call_srcu(), then
    srcu_barrier() must be invoked after the last call_srcu() invocation
    but before the module exits.
    
    This commit therefore adds a comment stating this.
    
    Reported-by: Alexander Aring <aahringo@redhat.com>
    Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
index ad9d9658b0a2f5..93b76e54389432 100644
--- a/include/linux/srcutree.h
+++ b/include/linux/srcutree.h
@@ -214,6 +214,12 @@ struct srcu_struct {
  * instead of smp_mb(), and given that the first (for example)
  * srcu_read_lock_fast() might race with the first synchronize_srcu(),
  * this different must be specified at initialization time.
+ *
+ * If you use any of the DEFINE_SRCU() functions within a module, the
+ * module-entry code will invoke init_srcu_struct() and the module-exit
+ * code will invoke cleanup_srcu_struct().  This means that if your module
+ * passes the resulting srcu_struct structure to call_srcu(), you will
+ * need to also pass this structure to srcu_barrier() prior to module exit.
  */
 #ifdef MODULE
 # define __DEFINE_SRCU(name, fast, is_static)							\

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-09-02 20:05 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-01  9:18 [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths Zqiang
2026-09-01 17:12 ` Alexander Aring
2026-09-02 13:21 ` Alexander Aring
2026-09-02 14:15   ` Zqiang
2026-09-02 14:29     ` Zqiang
2026-09-02 14:36       ` Alexander Aring
2026-09-02 14:49         ` Zqiang
2026-09-02 15:02           ` Alexander Aring
2026-09-02 14:30     ` Alexander Aring
2026-09-02 14:48       ` Alexander Aring
2026-09-02 16:44       ` Paul E. McKenney
2026-09-02 17:13         ` Alexander Aring
2026-09-02 20:05           ` Paul E. McKenney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®