Re: [PATCH] crypto: x86: Do not acquire fpu context for too long

[Taehee Yoo <ap420073@gmail.com>]

Hi Elliott, Robert

2022. 10. 10. 오전 4:58에 Elliott, Robert (Servers) 이(가) 쓴 글:
 >
 >
 >> -----Original Message-----
 >> From: Herbert Xu <herbert@gondor.apana.org.au>
 >> Sent: Sunday, October 9, 2022 1:20 AM
 >> To: Elliott, Robert (Servers) <elliott@hpe.com>
 >> Cc: Ard Biesheuvel <ardb@kernel.org>; Taehee Yoo 
<ap420073@gmail.com>; linux-
 >> crypto@vger.kernel.org; davem@davemloft.net; tglx@linutronix.de;
 >> mingo@redhat.com; bp@alien8.de; dave.hansen@linux.intel.com; 
x86@kernel.org;
 >> hpa@zytor.com; ebiggers@google.com
 >> Subject: Re: [PATCH] crypto: x86: Do not acquire fpu context for too 
long
 >>
 >> On Sat, Oct 08, 2022 at 07:48:07PM +0000, Elliott, Robert (Servers) 
wrote:
 >>>
 >>> Perhaps the cycles mode needs to call cond_resched() too?
 >>
 >> Yes, just make the cond_resched unconditional.  Having a few too many
 >> rescheds shouldn't be an issue.
 >
 > This looks promising. I was able to trigger a lot of rcu stalls by 
setting:
 >    echo 2 > /sys/module/rcupdate/parameters/rcu_cpu_stall_timeout
 >    echo 200 > /sys/module/rcupdate/parameters/rcu_exp_cpu_stall_timeout
 >
 > and running these concurrently:
 >    watch -n 0 modprobe tcrypt=200
 >    watch -n 0 module tcrypt=0 through 999
 >
 > After changing tcrypt to call cond_resched in both cases, I don't see any
 > more rcu stalls.
 >
 > I am getting miscompares from the extended self-test for crc32 and
 > crct10dif, and will investigate those further.
 >
 > BTW, the way tcrypt always refuses to load leads to an ever-growing 
list in
 > the Call Traces:
 >
 > kernel: Unloaded tainted modules: tcrypt():1 tcrypt():1 tcrypt():1 
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 
tcrypt():1 tcrypt():1 tcrypt():1 t
 > crypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
 > tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1
 >   tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():
 > 1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 
tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt():1 tcrypt()
 > :1 tcrypt():1 tcrypt():1 tcrypt():1
 >
 >
 >

I tested mb_aead as well.

I can find rcu stalls easily while testing gcm(aes-generic) with the 
below commands.
#shell1
while :
do
     modprobe tcrypt mode=215 num_mb=1024
done
#shell2
while :
do
     modprobe tcrypt mode=0
done

Then, I added cond_resched() as you mentioned.

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index a82679b576bb..eeb3abb4eece 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -415,6 +415,7 @@ static void test_mb_aead_speed(const char *algo, int 
enc, int secs,
                         } else {
                                 ret = test_mb_aead_cycles(data, enc, bs,
                                                           num_mb);
+                               cond_resched();
                         }

I can't see rcu stalls anymore, I think it works well.