From: Johannes Berg <email@example.com> To: Bart Van Assche <firstname.lastname@example.org>, Tejun Heo <email@example.com> Cc: "firstname.lastname@example.org" <email@example.com>, Christoph Hellwig <firstname.lastname@example.org>, Sagi Grimberg <email@example.com>, "firstname.lastname@example.org" <email@example.com> Subject: Re: [PATCH 2/3] kernel/workqueue: Surround work execution with shared lock annotations Date: Thu, 25 Oct 2018 18:53:44 +0200 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> On Thu, 2018-10-25 at 15:05 +0000, Bart Van Assche wrote: > Surround execution of work with a shared lockdep annotation because multiple > work items associated with a work queue may execute concurrently. Hmm. So, I'm not really entirely sure of the semantics here, but I fail to see how "may execute concurrently" means "can be taken recursively"? After all, if they execute concurrently, that's in a different thread, right? So each thread is really just doing something with this work. It may not match mutex semantics in how mutexes would lock each other out and prevent concurrency, but I don't think that matters to lockdep at all. In fact, I'm not sure this actually changes anything, since you can't really execute a work struct while executing one already? What's this intended to change? I currently don't see how lockdep's behaviour would differ with read==1, unless you actually tried to do recursive locking, which isn't really possible? Or perhaps this is actually the right change for the issue described in patch 1, where a work struct flushes another work on the same wq, and that causes recursion of sorts? But that recursion should only happen if the workqueues is actually marked as ordered, in which case it *is* in fact wrong? johannes
next prev parent reply other threads:[~2018-10-25 16:54 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-25 15:05 [PATCH 0/3] Suppress false positives triggered by workqueue lockdep annotations Bart Van Assche 2018-10-25 15:05 ` [PATCH 1/3] kernel/workqueue: Remove lockdep annotation from __flush_work() Bart Van Assche 2018-10-25 15:31 ` Johannes Berg 2018-10-25 15:57 ` Johannes Berg 2018-10-25 16:01 ` Bart Van Assche 2018-10-25 15:05 ` [PATCH 2/3] kernel/workqueue: Surround work execution with shared lock annotations Bart Van Assche 2018-10-25 16:53 ` Johannes Berg [this message] 2018-10-25 17:22 ` Bart Van Assche 2018-10-25 19:17 ` Johannes Berg 2018-10-25 15:05 ` [PATCH 3/3] kernel/workqueue: Suppress a false positive lockdep complaint Bart Van Assche 2018-10-25 15:34 ` Johannes Berg 2018-10-25 15:55 ` Bart Van Assche 2018-10-25 19:59 ` Johannes Berg 2018-10-25 20:21 ` Theodore Y. Ts'o 2018-10-25 20:26 ` Johannes Berg 2018-10-25 15:36 ` Tejun Heo 2018-10-25 15:37 ` Tejun Heo 2018-10-25 20:13 ` Johannes Berg 2018-10-25 15:40 ` Theodore Y. Ts'o 2018-10-25 17:02 ` Johannes Berg 2018-10-25 17:11 ` Bart Van Assche 2018-10-25 19:51 ` Johannes Berg 2018-10-25 20:39 ` Bart Van Assche 2018-10-25 20:47 ` Johannes Berg 2018-10-25 15:27 ` [PATCH 0/3] Suppress false positives triggered by workqueue lockdep annotations Johannes Berg 2018-10-25 15:47 ` Bart Van Assche
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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH 2/3] kernel/workqueue: Surround work execution with shared lock annotations' \ /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).