Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d12093d7-d01c-081f-fa04-c608f8021a60@gmail.com>
Date:   Wed, 12 Oct 2022 15:08:24 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.13.1
Subject: Re: [PATCH] crypto: x86: Do not acquire fpu context for too long
To:     "Elliott, Robert (Servers)" <elliott@hpe.com>,
        Herbert Xu <herbert@gondor.apana.org.au>
Cc:     Ard Biesheuvel <ardb@kernel.org>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "bp@alien8.de" <bp@alien8.de>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "x86@kernel.org" <x86@kernel.org>, "hpa@zytor.com" <hpa@zytor.com>,
        "ebiggers@google.com" <ebiggers@google.com>
References: <20221004044912.24770-1-ap420073@gmail.com>
 <Yzu8Kd2botr3eegj@gondor.apana.org.au>
 <f7c52ed1-8061-8147-f676-86190118cc56@gmail.com>
 <MW5PR84MB18420D6E1A31D9C765EF6ED4AB5A9@MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM>
 <CAMj1kXGmKunh-OCvGFf8T6KJJXSHRYzacjSojBD3__u0o-3D1w@mail.gmail.com>
 <MW5PR84MB1842762F8B2ABC27A1A13614AB5E9@MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM>
 <Y0JoDWx0Q5BmO/wR@gondor.apana.org.au>
 <MW5PR84MB1842FD77B90B1553367235CEAB219@MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM>
From:   Taehee Yoo <ap420073@gmail.com>
In-Reply-To: <MW5PR84MB1842FD77B90B1553367235CEAB219@MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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.