Re: i386 hangs during halt "vnodes remaining... 0 time out"

On Sun, 22 Apr 2018 15:05:21 +0300 Konstantin Belousov <kostikbel_at_gmail.com> wrote:
> On Sat, Apr 21, 2018 at 11:49:34PM +0200, Tijl Coosemans wrote:
>> On Sat, 21 Apr 2018 21:09:09 +0000 Rick Macklem <rmacklem_at_uoguelph.ca> wrote:  
>>> With a recent head/current kernel (doesn't happen when running a Dec.
>>> 2017 one), when I do a halt, it gets as far as:
>>> 
>>> vnodes remaining... 0 time out
>>> 
>>> and that's it (the time out appears several seconds after the first "0").
>>> With a Dec. 2017 kernel there would be several "0"s printed.
>>> It appears that it is stuck in the first iteration of the sched_sync()
>>> loop after it is no longer in SYNCER_RUNNING state.
>>> 
>>> Any ideas? rick  
>> 
>> See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=227404
>> I have a patch (attached) but haven't been able to test it yet.  
>> 
>> Index: sys/kern/vfs_bio.c
>> ===================================================================
>> --- sys/kern/vfs_bio.c	(revision 332165)
>> +++ sys/kern/vfs_bio.c	(working copy)
>> _at__at_ -791,9 +791,12 _at__at_ bufspace_daemon(void *arg)
>>  {
>>  	struct bufdomain *bd;
>>  
>> +	EVENTHANDLER_REGISTER(shutdown_pre_sync, kthread_shutdown, curthread,
>> +	    SHUTDOWN_PRI_LAST);
>> +
>>  	bd = arg;
>>  	for (;;) {
>> -		kproc_suspend_check(curproc);
>> +		kthread_suspend_check();
>>  
>>  		/*
>>  		 * Free buffers from the clean queue until we meet our
>> _at__at_ -3357,7 +3360,7 _at__at_ buf_daemon()
>>  	/*
>>  	 * This process needs to be suspended prior to shutdown sync.
>>  	 */
>> -	EVENTHANDLER_REGISTER(shutdown_pre_sync, kproc_shutdown, bufdaemonproc,
>> +	EVENTHANDLER_REGISTER(shutdown_pre_sync, kthread_shutdown, curthread,
>>  	    SHUTDOWN_PRI_LAST);
>>  
>>  	/*
>> _at__at_ -3381,7 +3384,7 _at__at_ buf_daemon()
>>  		bd_request = 0;
>>  		mtx_unlock(&bdlock);
>>  
>> -		kproc_suspend_check(bufdaemonproc);
>> +		kthread_suspend_check();
>>  
>>  		/*
>>  		 * Save speedupreq for this pass and reset to capture new  
> This looks fine.

Thanks for the review.  There's just one concern I have.  With this patch
the bufspace_daemon threads appear to shutdown after the buf_daemon and
after the syncer because the event handlers are registered later.  Are
there any dependencies between these processes that require the bufspace
threads to be stopped earlier?