Re: Introducing localcount(9)

[Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>]

Hi,

On 2017/05/06 14:12, Paul Goyette wrote:
> If you want to look at the implementation of localcount(9), you can see
> the code in the CVS repository [5].  Files sys/kern/subr_localcount.c
> and sys/sys/localcount.h contain the localcount functionality;  files
> sys/kern/subr_autoconf.c and sys/kern/subr_devsw.c have been updated to
> use the localcount functionality;  and many device drivers throughout
> the system have been updated to provide improved reference counting.
> (Don't forget to look for any newly created files in the Attic!)
> 
> Unless there are substantial objections, I'd like to commit localcount
> in a couple of weeks.  Constructive comments and code review welcomed!

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.

    (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?


Thanks,

-- 
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>