On Sat, Jun 23, 2018 at 09:03:02PM +0000, Rick Macklem wrote:
> During testing of the pNFS server I have been frequently killing/restarting 
> the nfsd.
> Once in a while, the "slave" nfsd process doesn't terminate and a "ps axHl" 
> shows:
>   0 48889     1   0   20  0  5884  812 svcexit  D     -   0:00.01 nfsd: 
> server 
>   0 48889     1   0   40  0  5884  812 rpcsvc   I     -   0:00.00 nfsd: 
> server 
> ... more of the same
>   0 48889     1   0   40  0  5884  812 rpcsvc   I     -   0:00.00 nfsd: 
> server 
>   0 48889     1   0   -8  0  5884  812 rpcsvc   I     -   1:51.78 nfsd: 
> server 
>   0 48889     1   0   -8  0  5884  812 rpcsvc   I     -   2:27.75 nfsd: 
> server 
> 
> You can see that the top thread (the one that was created with the process) is
> stuck in "D"  on "svcexit".
> The rest of the threads are still servicing NFS RPCs. If you still have an 
> NFS mount on
> the server, the mount continues to work and the CPU time for the last two 
> threads
> slowly climbs, due to NFS RPC activity. A SIGKILL was posted for the process 
> and
> these threads (created by kthread_add) are here, but the
> cv_wait_sig/cv_timedwait_sig never seems to return EINTR for these other 
> threads.
> 
>                        if (ismaster || (!ismaster &&
> 1207                              grp->sg_threadcount > grp->sg_minthreads))
> 1208                                  error = cv_timedwait_sig(&st->st_cond,
> 1209                                      &grp->sg_lock, 5 * hz);
> 1210                          else
> 1211                                  error = cv_wait_sig(&st->st_cond,
> 1212                                      &grp->sg_lock);
> 
> The top thread (referred to in svc.c as "ismaster" did return from here with 
> EINTR
> and has now done an msleep() here, waiting for the other threads to terminate.
> 
>        /* Waiting for threads to stop. */
> 1387          for (g = 0; g < pool->sp_groupcount; g++) {
> 1388                  grp = &pool->sp_groups[g];
> 1389                  mtx_lock(&grp->sg_lock);
> 1390                  while (grp->sg_threadcount > 0)
> 1391                          msleep(grp, &grp->sg_lock, 0, "svcexit", 0);
> 1392                  mtx_unlock(&grp->sg_lock);
> 1393          }
> 
> Although I can't be sure if this patch has fixed the problem because it 
> happens
> intermittently, I have not seen the problem since applying this patch:
> --- rpc/svc.c.sav     2018-06-21 22:52:11.623955000 -0400
> +++ rpc/svc.c 2018-06-22 09:01:40.271803000 -0400
> @@ -1388,7 +1388,7 @@ svc_run(SVCPOOL *pool)
>               grp = &pool->sp_groups[g];
>               mtx_lock(&grp->sg_lock);
>               while (grp->sg_threadcount > 0)
> -                     msleep(grp, &grp->sg_lock, 0, "svcexit", 0);
> +                     msleep(grp, &grp->sg_lock, 0, "svcexit", 1);
>               mtx_unlock(&grp->sg_lock);
>       }
>  }
> 
> As you can see, all it does is add a timeout to the msleep(). 
> I am not familiar with the signal delivery code in sleepqeue, so it probably
> isn't correct, but my theory is alonge the lines of...
> 
> Since the msleep() doesn't have PCATCH, it does not set TDF_SINTR
> and if that happens before the other threads return EINTR from cv_wait_sig(),
> they no longer do so?
> And I thought that waking up from the msleep() via timeouts would maybe allow
> the other threads to return EINTR from cv_wait_sig()?
> 
> Does this make sense? rick
> ps: I'll post if I see the problem again with the patch applied.
> pss: This is a single core i386 system, just in case that might affect this.

No, the patch does not make sense. I think it was just coincidental that
with the patch you did not get the hang.

Signals are delivered to a thread, which should take the appropriate
actions. For the kernel process like rpc pool, the signals are never
delivered, they are queued in the randomly selected thread' signal queue
and sit there. The interruptible sleeps are aborted in the context
of that thread, but nothing else happens. So if you need to make svc
pools properly killable, all threads must check at least for EINTR and
instruct other threads to exit as well.

Your description at the start of the message of the behaviour after
SIGKILL, where other threads continued to serve RPCs, exactly matches
above explanation. You need to add some global 'stop' flag, if it is not
yet present, and recheck it after each RPC handled. Any thread which
notes EINTR or does a direct check for the pending signal, should set
the flag and wake up every other thread in the pool.
_______________________________________________
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to