Re: RFC: localcount_hadref() or localcount_trydarin()

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

Hi,

On 2017/06/10 1:20, Taylor R Campbell wrote:
>> Date: Fri, 9 Jun 2017 16:50:18 +0900
>> From: Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>
>>
>> I tried using localcount(9) for opencyrpto(9) to avoid detach encryption
>> drivers. While I implement that, I want to something to use KASSERT which
>> ensure someone have reference or not.
>> In addition, localcount_drain() waits forever if someone has had reference.
>> So, I want to function which returns in not so long time regardless of
>> someone has reference or not. In other words, I want some function like
>> mutex_tryenter() for mutex_enter().
> 
> This sounds questionable to me.  If there is a reference remaining
> that does not drain in a bounded time, that sounds like a bug that
> should be addressed some other way.  Can you explain how you want to
> use this?  Can you share preliminary diffs to opencrypto code with
> localcount that motivate this?

At first, I mention conclusion shortly,
    - I want to avoid detaching encryption device when it is used by IPsec
    - I implement naive design of that with localcount(9)
      - The patches show in the last
    - After that, the detaching process(drvctl(8)) waits infinity

The details are below.

I want to avoid detaching the encryption device while it is used by IPsec.
That is, once someone creates Security Assocatation(SA) to call
crypto_newsession(), the encryption device related the SA must not be
detached until the SA is flushed(done crypto_freesession()) and the SA
is not used(done crypto_dispatch() and cryptointr()).

Thus, I can eliminate "migration job" in opencrypto. The migration job
runs when encryption device is detached while encryption job is
processing, that is, below two codes.
   [1] https://nxr.netbsd.org/xref/src/sys/opencrypto/crypto.c#1620
   [2] https://nxr.netbsd.org/xref/src/sys/opencrypto/crypto.c#1300
The [2] codes(crypto_invoke()) is fast path, so I want to avoid calling
crypto_newsession() which may sleep. Furthermore, the migration job can
disturb crp_q order.

I implement prototype which design is below.
    - acquire localcount(9) in crypto_newsession()
    - release localcount(9) in crypto_freesession()
    - acquire and release localcount(9) in crypto_dispatch() and cryptointr()
    - drain localcount(9) in crypto_unregister()
Hmm, that force drvctl(8) which detach the encryption device wait infinity
until the SA is flushed. And the drvctl(8) cannot be killed. It is not good.
I think localcount_trydrain() is required to avoid the drvctl(8) infinity
waiting simply.

Here is the working patch series. I'm sorry to say the patches is old,
so they are applied to crypto.c:r1.78 and cryptodev.h:r1.34. Furthermore,
there is still some bugs....
    - http://www.netbsd.org/~knakahara/opencrypto-localcount/0001-introduce-localcount.patch
    - http://www.netbsd.org/~knakahara/opencrypto-localcount/0002-remove-migrate-process.patch
    - http://www.netbsd.org/~knakahara/opencrypto-localcount/0003-avoid-localcount-acquire-after-drain.patch

Could you comment this design or patches? 
Hmm, I begin to feel this usage is not fit to localcount(9)....


Thanks,

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

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

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