diff mbox

multipathd: fix SIGUSR2 handling

Message ID 1482998174-11884-1-git-send-email-tang.junhui@zte.com.cn (mailing list archive)
State Not Applicable, archived
Delegated to: christophe varoqui
Headers show

Commit Message

tang.junhui@zte.com.cn Dec. 29, 2016, 7:56 a.m. UTC
From: "tang.junhui" <tang.junhui@zte.com.cn>

SIGUSR2 is not blocked by default, it would cause bellow conflict
in removing of the last path:
-------------------------------------------------------------
uevent processing thread                 | waiter thread
-----------------------------------------|-------------------
uev_remove_path()                        |   waiteventloop()
  lock(&vecs->lock)                      |
  ev_remove_path()                       |     
    dm_queue_if_no_path()----------------|----> dm_task_run()	
    flush_map()                          |      |
      remove_map_and_stop_waiter()       |      -> pthread_cleanup_push(cleanup_lock, &waiter->vecs->lock);
        _remove_map()                    |         lock(&waiter->vecs->lock)//wait for the locker	
          stop_waiter_thread()           |         
            pthread_cancel(thread)       |          
            pthread_kill(thread,SIGUSR2)-|------> sigusr2 (int sig)
                                         |          condlog()
                                         |          fprintf() //it has test cancel point
                                         |          cleanup_lock() //thread cancel, and pop, which unlock the 
                                         |                           locker of uevent processing thread 
--------------------------------------------------------------
Since SIGUSR2 is only needed when waiter thread running in dm_task_run(),
and it has already been unblocked before dm_task_run(), so this patch block 
SIGUSR2 for other times.

Change-Id: I8c46292dc954415764ebbe054498b4f7ea53c1c6
Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
---
 multipathd/main.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Benjamin Marzinski Jan. 3, 2017, 6:59 p.m. UTC | #1
On Thu, Dec 29, 2016 at 03:56:14PM +0800, tang.junhui@zte.com.cn wrote:
> From: "tang.junhui" <tang.junhui@zte.com.cn>
> 
> SIGUSR2 is not blocked by default, it would cause bellow conflict
> in removing of the last path:

ACK

-Ben

> -------------------------------------------------------------
> uevent processing thread                 | waiter thread
> -----------------------------------------|-------------------
> uev_remove_path()                        |   waiteventloop()
>   lock(&vecs->lock)                      |
>   ev_remove_path()                       |     
>     dm_queue_if_no_path()----------------|----> dm_task_run()	
>     flush_map()                          |      |
>       remove_map_and_stop_waiter()       |      -> pthread_cleanup_push(cleanup_lock, &waiter->vecs->lock);
>         _remove_map()                    |         lock(&waiter->vecs->lock)//wait for the locker	
>           stop_waiter_thread()           |         
>             pthread_cancel(thread)       |          
>             pthread_kill(thread,SIGUSR2)-|------> sigusr2 (int sig)
>                                          |          condlog()
>                                          |          fprintf() //it has test cancel point
>                                          |          cleanup_lock() //thread cancel, and pop, which unlock the 
>                                          |                           locker of uevent processing thread 
> --------------------------------------------------------------
> Since SIGUSR2 is only needed when waiter thread running in dm_task_run(),
> and it has already been unblocked before dm_task_run(), so this patch block 
> SIGUSR2 for other times.
> 
> Change-Id: I8c46292dc954415764ebbe054498b4f7ea53c1c6
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  multipathd/main.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index adc3258..fe69abd 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2181,6 +2181,12 @@ sigusr2 (int sig)
>  static void
>  signal_init(void)
>  {
> +	sigset_t set;
> +
> +	sigemptyset(&set);
> +	sigaddset(&set, SIGUSR2);
> +	pthread_sigmask(SIG_SETMASK, &set, NULL);
> +
>  	signal_set(SIGHUP, sighup);
>  	signal_set(SIGUSR1, sigusr1);
>  	signal_set(SIGUSR2, sigusr2);
> -- 
> 2.8.1.windows.1
> 

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Bart Van Assche March 14, 2017, 11:29 p.m. UTC | #2
On Tue, 2017-01-03 at 12:59 -0600, Benjamin Marzinski wrote:
> On Thu, Dec 29, 2016 at 03:56:14PM +0800, tang.junhui@zte.com.cn wrote:
> > From: "tang.junhui" <tang.junhui@zte.com.cn>
> > 
> > SIGUSR2 is not blocked by default, it would cause bellow conflict
> > in removing of the last path:
> 
> ACK

Hello Christophe,

This patch was posted more than two months ago. Do you think it should
be reposted?

Thanks,

Bart.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Christophe Varoqui March 16, 2017, 6:43 a.m. UTC | #3
Applied.
Thanks.

On Wed, Mar 15, 2017 at 12:29 AM, Bart Van Assche <
Bart.VanAssche@sandisk.com> wrote:

> On Tue, 2017-01-03 at 12:59 -0600, Benjamin Marzinski wrote:
> > On Thu, Dec 29, 2016 at 03:56:14PM +0800, tang.junhui@zte.com.cn wrote:
> > > From: "tang.junhui" <tang.junhui@zte.com.cn>
> > >
> > > SIGUSR2 is not blocked by default, it would cause bellow conflict
> > > in removing of the last path:
> >
> > ACK
>
> Hello Christophe,
>
> This patch was posted more than two months ago. Do you think it should
> be reposted?
>
> Thanks,
>
> Bart.
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
diff mbox

Patch

diff --git a/multipathd/main.c b/multipathd/main.c
index adc3258..fe69abd 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2181,6 +2181,12 @@  sigusr2 (int sig)
 static void
 signal_init(void)
 {
+	sigset_t set;
+
+	sigemptyset(&set);
+	sigaddset(&set, SIGUSR2);
+	pthread_sigmask(SIG_SETMASK, &set, NULL);
+
 	signal_set(SIGHUP, sighup);
 	signal_set(SIGUSR1, sigusr1);
 	signal_set(SIGUSR2, sigusr2);