svn commit: r272366 - head/sys/kern

[Bruce Evans]

On Wed, 1 Oct 2014, Mateusz Guzik wrote:

> On Wed, Oct 01, 2014 at 03:32:29PM +0000, Will Andrews wrote:
>> Author: will
>> Date: Wed Oct  1 15:32:28 2014
>> New Revision: 272366
>> URL: https://svnweb.freebsd.org/changeset/base/272366
>>
>> Log:
>>   In the syncer, drop the sync mutex while patting the watchdog.
>>
>>   Some watchdog drivers (like ipmi) need to sleep while patting the watchdog.
>>   See sys/dev/ipmi/ipmi.c:ipmi_wd_event(), which calls malloc(M_WAITOK).
>
> Is not such malloc inherently bad in case of watchdog?
>
> It looks like the code waits for request completion and then frees it
> unconditionally. As such, a local var on stack would do the trick.
>
> The code msleeps later, so this hack of dropping sync_mtx is still
> needed.

It's not a hack then.  Then it is at the end of the loop, so any state
changes are picked up by looping.

However, sync_vnode() has _always_ just released the lock and re-acquired
it, so there is no new problem, and probably no old one either.  It is
just hard to see that there is no problem in code that doesn't check
for state changes after sleeping.

% 		while (!LIST_EMPTY(slp)) {
% 			error = sync_vnode(slp, &bo, td);
% 			if (error == 1) {
% 				LIST_REMOVE(bo, bo_synclist);
% 				LIST_INSERT_HEAD(next, bo, bo_synclist);
% 				continue;
% 			}

Since we get here, sync_vnode() didn't return early; it unlocked, and the
last statement in it is to re-lock...

(All sync_vnode() did before unlocking was to read a vnode from the list
and lock and hold it.)

% 
% 			if (first_printf == 0)

... so we can sprinkle any number of unlock/lock pairs here without adding
any new problems.

% 				wdog_kern_pat(WD_LASTVAL);
% 
% 		}

Bruce