David
On 5/20/2020 12:46 AM, David Miller wrote:
From: Vinay Kumar Yadav <vinay.ya...@chelsio.com>
Date: Tue, 19 May 2020 13:13:27 +0530
+ spin_lock_bh(&ctx->encrypt_compl_lock);
+ pending = atomic_read(&ctx->encrypt_pending);
+ spin_unlock_bh(&ctx->encrypt_compl_lock);
The sequence:
lock();
x = p->y;
unlock();
Does not fix anything, and is superfluous locking.
The value of p->y can change right after the unlock() call, so you
aren't protecting the atomic'ness of the read and test sequence
because the test is outside of the lock.
Here, by using lock I want to achieve atomicity of following statements.
pending = atomic_dec_return(&ctx->decrypt_pending);
if (!pending && READ_ONCE(ctx->async_notify))
complete(&ctx->async_wait.completion);
means, don't want to read (atomic_read(&ctx->decrypt_pending))
in middle of two statements
atomic_dec_return(&ctx->decrypt_pending);
and
complete(&ctx->async_wait.completion);
Why am I protecting only read, not test ?
complete() is called only if pending == 0
if we read atomic_read(&ctx->decrypt_pending) = 0
that means complete() is already called and its okay to
initialize completion (reinit_completion(&ctx->async_wait.completion))
if we read atomic_read(&ctx->decrypt_pending) as non zero that means:
1- complete() is going to be called or
2- complete() already called (if we read atomic_read(&ctx->decrypt_pending) ==
1, then complete() is called just after unlock())
for both scenario its okay to go into wait (crypto_wait_req(-EINPROGRESS,
&ctx->async_wait))
Thanks,
Vinay
