Move process_blocking_lock_queue out of timeout_processing?

[Jeremy Allison]

On Sun, Mar 18, 2007 at 12:48:03PM +0100, Volker Lendecke wrote:
> Hi, Jeremy!
> 
> In the move to simplify our central processing loop code
> attached find a change that I would like to have your
> approval on: It moves process_blocking_lock_queue out of the
> way into a timed event.
> 
> The idea is that we have blocking.c:brl_timeout as a timed
> event that is present whenever we do have a blocking lock
> pending. It fires brl_timeout_fn() which calls
> process_blocking_lock_queue(). Please note the
> change_to_root_user() in brl_timeout_fn().

Are the timeouts not normally run as root ? I thought
they were.

> Whenever we make changes to blocking_lock_queue, we trigger
> a recalc_brl_timeout() which sets a new brl_timout event if
> necessary. This makes the call to
> blocking_locks_timeout_ms() in setup_select_timeout()
> unnecessary, this is implicitly done in
> event_add_to_select_args() from the timed events.
> 
> Check in? It survives make test :-)

On quick scan it looks ok. I certainly think it's the
right way to move forward (event based code).

> BTW, while looking at the brl code it might become possible
> to add a scheme polling for a os-level fcntl lock on behalf
> of CIFS clients in case that is taken by a Unix process.
> Just add a timed event that looks every second. Ugly as
> hell, but as long as don't get async fcntl locks there's
> nothing we can do but use threads. Not sure which pill is
> more bitter. Polling or pthreads? :-)

Hmmm - the original code would set a select timeout
of 10 seconds for a blocking lock that had infinite
timeout - that was the "poll" mechanism for locks
held by non-smbd processes.

Looking in the code it seems this might have been
lost in some old, old, old rewrite :-).

I think this code in smbd/blocking.c :

614                 if (timeval_is_zero(&blr->expire_time)) {
615                         continue; /* Never timeout. */
616                 }
617 
618                 tv_dif_us = usec_time_diff(&blr->expire_time, &tv_curr);

Might have needed to be :

	...
	struct timeval tv_ex_time;
	....

	tmp_ex_time.tv_sec = blr->expire_time;
	if (timeval_is_zero(&tmp_ex_time)) {
		/* Set a tmp variable expire time to now + 10 secs */
		tmp_exp_time.tv_sec = tv_curr.tv_sec + 30;
		tmp_ext_time.tv_usec = tv_curr.tv_usec;
	}

	tv_dif_us = usec_time_diff(&tmp_ex_time, &tv_curr);

Any chance of adding that into the patch ?

Jeremy.