LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Byungchul Park <byungchul.park@lge.com>
To: torvalds@linux-foundation.org, peterz@infradead.org,
	mingo@redhat.com, will@kernel.org, herbert@gondor.apana.org.au,
	davem@davemloft.net, linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, tglx@linutronix.de,
	rostedt@goodmis.org, joel@joelfernandes.org,
	alexander.levin@microsoft.com, daniel.vetter@ffwll.ch,
	chris@chris-wilson.co.uk, duyuyang@gmail.com,
	johannes.berg@intel.com, tj@kernel.org, tytso@mit.edu,
	willy@infradead.org, david@fromorbit.com, amir73il@gmail.com,
	bfields@fieldses.org, gregkh@linuxfoundation.org,
	kernel-team@lge.com
Subject: [REPORT] Request for reviewing crypto code wrt wait_for_completion()
Date: Fri, 6 Aug 2021 17:03:44 +0900	[thread overview]
Message-ID: <20210806080344.GA5788@X58A-UD3R> (raw)
In-Reply-To: <20210803021611.GA28236@X58A-UD3R>

Hello crypto folks,

I developed a tool for tracking waiters and reporting if any of the
events that the waiters are waiting for would never happen, say, a
deadlock. Yes, it would look like Lockdep but more inclusive.

While I ran the tool(Dept: Dependency Tracker) on v5.4.96, I got some
reports from the tool. One of them is related to crypto subsystem.
Because I'm not that familiar with the code, I'd like to ask you guys to
review the related code.

If I understand correctly, it doesn't actually cause deadlock but looks
like a problematic code. I know you are not used to the format of the
report from Dept so.. let me summerize the result.

The simplified call trace looks like when the problem araised :

THREAD A
--------
A1 crypto_alg_mod_lookup()
A2    crypto_probing_notify(CRYPTO_MSG_ALG_REQUEST)
A3       cryptomgr_schedule_probe()
A4          kthread_run(cyptomgr_probe) ---> Start THREAD B

A5    crypto_larval_wait()
A6       wait_for_completion_killable_timeout(c) /* waiting for B10 */

THREAD B
--------
B1 cryptomgr_probe()
B2    pkcslpad_create()
B3       crypto_wait_for_test()
B4          crypto_probing_notify(CRYPTO_MSG_ALG_REGISTER)
B5             cryptomgr_schedule_test()
B6                kthread_run(cyptomgr_test) ---> Start THREAD C

B7    tmpl->alloc()
B8    crupto_register_instance()
B9          wait_for_completion_killable(c) /* waiting for C3 */
B10   complete_all(c)

THREAD C
--------
C1 cryptomgr_test()
C2    crypto_alg_tested()
C3       complete_all(c)

---

For example, in this situation, I think C3 could wake up both A6 and B9
before THREAD B reaches B10 which is not desired by A6. Say, is it okay
to wake up A6 with B7 ~ B9 having yet to complete?

Sorry if I misunderstand the code. It looks so complicated to me. Could
you check if the code is good?

Just FYI, the below is the report from the tool, Dept, I developed.

Thanks,
Byungchul

---

[   10.520128 ] ===================================================
[   10.526037 ] Dept: Circular dependency has been detected.
[   10.531337 ] 5.4.96-242 #1 Tainted: G        W   
[   10.536375 ] ---------------------------------------------------
[   10.542280 ] summary
[   10.544366 ] ---------------------------------------------------
[   10.550271 ] *** AA DEADLOCK ***
[   10.550271 ] 
[   10.554875 ] context A
[   10.557136 ]     [S] (unknown)(&larval->completion:0)
[   10.562087 ]     [W] wait_for_completion_killable(&larval->completion:0)
[   10.568688 ]     [E] complete_all(&larval->completion:0)
[   10.573898 ] 
[   10.575377 ] [S]: start of the event context
[   10.579546 ] [W]: the wait blocked
[   10.582848 ] [E]: the event not reachable
[   10.586757 ] ---------------------------------------------------
[   10.592662 ] context A's detail
[   10.595703 ] ---------------------------------------------------
[   10.601608 ] context A
[   10.603868 ]     [S] (unknown)(&larval->completion:0)
[   10.608819 ]     [W] wait_for_completion_killable(&larval->completion:0)
[   10.615419 ]     [E] complete_all(&larval->completion:0)
[   10.620630 ] 
[   10.622109 ] [S] (unknown)(&larval->completion:0):
[   10.626799 ] (N/A)
[   10.628712 ] 
[   10.630191 ] [W] wait_for_completion_killable(&larval->completion:0):
[   10.636537 ] [<ffffffc0104dfc20>] crypto_wait_for_test+0x40/0x80
[   10.642443 ] stacktrace:
[   10.644881 ]       wait_for_completion_killable+0x34/0x160
[   10.650267 ]       crypto_wait_for_test+0x40/0x80
[   10.654871 ]       crypto_register_instance+0xb0/0xe0
[   10.659824 ]       akcipher_register_instance+0x30/0x38
[   10.664950 ]       pkcs1pad_create+0x238/0x2b0
[   10.669295 ]       cryptomgr_probe+0x40/0xd0
[   10.673467 ]       kthread+0x150/0x188
[   10.677118 ]       ret_from_fork+0x10/0x18
[   10.681114 ] 
[   10.682592 ] [E] complete_all(&larval->completion:0):
[   10.687544 ] [<ffffffc0104e8d20>] cryptomgr_probe+0xb0/0xd0
[   10.693016 ] stacktrace:
[   10.695452 ]       complete_all+0x30/0x70
[   10.699362 ]       cryptomgr_probe+0xb0/0xd0
[   10.703532 ]       kthread+0x150/0x188
[   10.707181 ]       ret_from_fork+0x10/0x18
[   10.711177 ] ---------------------------------------------------
[   10.717083 ] information that might be helpful
[   10.721426 ] ---------------------------------------------------
[   10.727334 ] CPU: 0 PID: 1787 Comm: cryptomgr_probe Tainted: G        W
5.4.96-242 #1
[   10.735757 ] Hardware name: LG Electronics, DTV SoC LG1213 (AArch64) (DT)
[   10.742444 ] Call trace:
[   10.744879 ]  dump_backtrace+0x0/0x148
[   10.748529 ]  show_stack+0x14/0x20
[   10.751833 ]  dump_stack+0xd0/0x12c
[   10.755223 ]  print_circle+0x3b0/0x3f8
[   10.758873 ]  cb_check_dl+0x54/0x70
[   10.762262 ]  bfs+0x64/0x1a0
[   10.765043 ]  add_dep+0x90/0xb8
[   10.768086 ]  dept_event+0x4c8/0x560
[   10.771562 ]  complete_all+0x30/0x70
[   10.775038 ]  cryptomgr_probe+0xb0/0xd0
[   10.778774 ]  kthread+0x150/0x188
[   10.781989 ]  ret_from_fork+0x10/0x18
[   10.786091 ] cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
[   10.792783 ] platform regulatory.0: Direct firmware load for
regulatory.db failed with error -2
[   10.796148 ] ALSA device list:
[   10.801423 ] cfg80211: failed to load regulatory.db



  reply	other threads:[~2021-08-06  8:04 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-03  2:16 [RFC] Cross-release versus a new tool Byungchul Park
2021-08-06  8:03 ` Byungchul Park [this message]
2021-08-06 11:40   ` [REPORT] Request for reviewing crypto code wrt wait_for_completion() Herbert Xu
2021-08-07  3:46     ` Byungchul Park
2021-08-08  4:45       ` Herbert Xu
2021-08-25  1:27 ` [RFC] Cross-release versus a new tool Byungchul Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210806080344.GA5788@X58A-UD3R \
    --to=byungchul.park@lge.com \
    --cc=alexander.levin@microsoft.com \
    --cc=amir73il@gmail.com \
    --cc=bfields@fieldses.org \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel.vetter@ffwll.ch \
    --cc=davem@davemloft.net \
    --cc=david@fromorbit.com \
    --cc=duyuyang@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=joel@joelfernandes.org \
    --cc=johannes.berg@intel.com \
    --cc=kernel-team@lge.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=will@kernel.org \
    --cc=willy@infradead.org \
    --subject='Re: [REPORT] Request for reviewing crypto code wrt wait_for_completion()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).