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 3/3] kernel/workqueue: Suppress a false positive lockdep complaint Date: Thu, 25 Oct 2018 21:59:38 +0200 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> (sfid-20181025_175553_256485_FAC088A9) On Thu, 2018-10-25 at 08:55 -0700, Bart Van Assche wrote: > Please have a look at the call trace in the description of this patch and also > at the direct I/O code. The lockdep complaint in the description of this patch > really is a false positive. I agree. I just don't agree that it's a false positive because of the lockdep tracking of workqueues (which you're essentially saying since you want to change it), I think it's a false positive because the user isn't telling lockdep properly what it's doing. > What I think needs further discussion is on how to > address this false positive - track whether or not a work queue has been used > or follow Tejun's proposal that I became aware of after I posted this patch, > namely introduce a new function for destroying a work queue that skips draining, > e.g. destroy_workqueue_skip_drain() (https://lkml.org/lkml/2018/10/24/2). Agree. Ted's suggestion is basically what I meant in my other email when I said: > So, thinking about this more, can you guarantee (somehow) that the > workqueue is empty at this point? (I hadn't looked at the code then - obviously that's guaranteed) > Perhaps then we should encode that > guarantee into the API, and actually make it WARN_ON() if something is > scheduled on it at that point? And then skip lockdep in this case. So that API I'm talking about is what Ted suggests with destroy_never_used_workqueue() or Tejun's suggestion of destroy_workqueue_skip_drain(). Either is fine with me, though I'd probably think it would make sense to actually check that it really was never used, or at least is actually empty right now and thus doesn't need the train. However, I think more generally useful would be the destroy_workqueue_nested() API I've proposed in my other reply just moments ago, since that could be used in other code with non-empty workqueues, as long as you can ensure that there's a guaranteed layering of them. Then you could even have two objects A and B of the same class, but separate instances, and flush B's workqueue from a work executing on A's workqueue, which may come in handy in other places to address false positives similar to this one. johannes
next prev parent reply other threads:[~2018-10-25 20:00 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 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 [this message] 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 3/3] kernel/workqueue: Suppress a false positive lockdep complaint' \ /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).