Re: Introducing localcount(9)

[Paul Goyette <paul%whooppee.com@localhost>]

On Thu, 11 May 2017, Kengo NAKAHARA wrote:

> Excellent! I expect the code is committed.
> I think opencrypto(9) would use localcount(9) to prevent detaching
> encrypt drivers while they are still used.
>
> I have two questions about localcount_release() in localcount.c.
>
>    (1) Why splsoftserial() is required instead of kpreempt_disable()?
>        localcount_drain() uses xc_broadcast(0, ...), that is, it uses
>        low priority xcall. Low priority xcall would be done by kthread
>        context, so I think kpreempt_disable() would be sufficient to
>        prevent localcount_drain() xcall running.

I think you are correct.  Taylor, do you agree?


>    (2) Why both "localcount_adjust(lc, -1)" and "*lc->lc_totalp -= 1" are
>        necessary in "lc->lc_totalp != NULL" case?
>        I am afraid of below situation
>            [1] CPU#A: mutex_enter(interlock)
>            [2] CPU#A: call localcount_drain()
>            [3] CPU#A: mutex_exit(interlock) before xc_broadcast
>            [4] CPU#B: call localcount_release()
>            [5] CPU#B: mutex_enter(interlock)
>            [6] CPU#B: localcount_adjust(lc, -1)
>            [7] CPU#B: *lc->lc_totalp -= 1
>            [8] CPU#B: done localcount_release()
>            [9] CPU#A: call xc_broadcast()
>            [10]CPU#B: begin localcount_xc() and done
>            [11]CPU#A: resume localcount_drain()
>            [12]CPU#A: mutex_enter(interlock)
>        Can "lc->lc_totalp" be -1 at this point?

localcount_adjust() updates the local CPU's reference counter, while 
*lc->lc_totalp is the global sum that we are accumulating.  We want to 
keep them synchronized.

Note that an individual CPU's local counter can certainly be negative; 
this is a natural result of the ability of localcount references being 
able to migrate across CPUs.  Depending on the order in which CPUs 
execute the xcall()s, the running total could also be negative at some 
intermediate point.  But after all of the xcall()s have returned, the 
sum must be non-negative.

We only care about the final total, after all xcall()s have finished, 
and we KASSERTMSG() to make certain that the value is non-negative.

I had the same question when I was integrating Taylor's initial draft, 
and it took a lot of hard thinking before I was able to convice myself 
that it really works correctly!

Thanks for the review - it is very much appreciated.



+------------------+--------------------------+----------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:          |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+------------------+--------------------------+----------------------------+