From: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com> To: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@linux.ie, daniel@ffwll.ch, peterz@infradead.org, mingo@redhat.com, will@kernel.org, longman@redhat.com, boqun.feng@gmail.com Cc: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [PATCH v2 2/2] drm: add lockdep assert to drm_is_current_master_locked Date: Sat, 31 Jul 2021 16:24:58 +0800 [thread overview] Message-ID: <20210731082458.1962043-3-desmondcheongzx@gmail.com> (raw) In-Reply-To: <20210731082458.1962043-1-desmondcheongzx@gmail.com> In drm_is_current_master_locked, accessing drm_file.master should be protected by either drm_file.master_lookup_lock or drm_device.master_mutex. This was previously awkward to assert with lockdep. Following patch ("locking/lockdep: Provide lockdep_assert{,_once}() helpers"), this assertion is now convenient. So we add in the assertion and explain this lock design in the kerneldoc. Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com> Acked-by: Boqun Feng <boqun.feng@gmail.com> Acked-by: Waiman Long <longman@redhat.com> --- drivers/gpu/drm/drm_auth.c | 6 +++--- include/drm/drm_file.h | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 9c24b8cc8e36..6f4d7ff23c80 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -63,9 +63,9 @@ static bool drm_is_current_master_locked(struct drm_file *fpriv) { - /* Either drm_device.master_mutex or drm_file.master_lookup_lock - * should be held here. - */ + lockdep_assert_once(lockdep_is_held(&fpriv->master_lookup_lock) || + lockdep_is_held(&fpriv->minor->dev->master_mutex)); + return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master; } diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 726cfe0ff5f5..a3acb7ac3550 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -233,6 +233,10 @@ struct drm_file { * this only matches &drm_device.master if the master is the currently * active one. * + * To update @master, both &drm_device.master_mutex and + * @master_lookup_lock need to be held, therefore holding either of + * them is safe and enough for the read side. + * * When dereferencing this pointer, either hold struct * &drm_device.master_mutex for the duration of the pointer's use, or * use drm_file_get_master() if struct &drm_device.master_mutex is not -- 2.25.1
WARNING: multiple messages have this Message-ID (diff)
From: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com> To: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@linux.ie, daniel@ffwll.ch, peterz@infradead.org, mingo@redhat.com, will@kernel.org, longman@redhat.com, boqun.feng@gmail.com Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>, linux-kernel-mentees@lists.linuxfoundation.org Subject: [PATCH v2 2/2] drm: add lockdep assert to drm_is_current_master_locked Date: Sat, 31 Jul 2021 16:24:58 +0800 [thread overview] Message-ID: <20210731082458.1962043-3-desmondcheongzx@gmail.com> (raw) In-Reply-To: <20210731082458.1962043-1-desmondcheongzx@gmail.com> In drm_is_current_master_locked, accessing drm_file.master should be protected by either drm_file.master_lookup_lock or drm_device.master_mutex. This was previously awkward to assert with lockdep. Following patch ("locking/lockdep: Provide lockdep_assert{,_once}() helpers"), this assertion is now convenient. So we add in the assertion and explain this lock design in the kerneldoc. Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com> Acked-by: Boqun Feng <boqun.feng@gmail.com> Acked-by: Waiman Long <longman@redhat.com> --- drivers/gpu/drm/drm_auth.c | 6 +++--- include/drm/drm_file.h | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 9c24b8cc8e36..6f4d7ff23c80 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -63,9 +63,9 @@ static bool drm_is_current_master_locked(struct drm_file *fpriv) { - /* Either drm_device.master_mutex or drm_file.master_lookup_lock - * should be held here. - */ + lockdep_assert_once(lockdep_is_held(&fpriv->master_lookup_lock) || + lockdep_is_held(&fpriv->minor->dev->master_mutex)); + return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master; } diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 726cfe0ff5f5..a3acb7ac3550 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -233,6 +233,10 @@ struct drm_file { * this only matches &drm_device.master if the master is the currently * active one. * + * To update @master, both &drm_device.master_mutex and + * @master_lookup_lock need to be held, therefore holding either of + * them is safe and enough for the read side. + * * When dereferencing this pointer, either hold struct * &drm_device.master_mutex for the duration of the pointer's use, or * use drm_file_get_master() if struct &drm_device.master_mutex is not -- 2.25.1 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
next prev parent reply other threads:[~2021-07-31 8:27 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-31 8:24 [PATCH v2 0/2] locking/lockdep, drm: apply new lockdep assert in drm_auth.c Desmond Cheong Zhi Xi 2021-07-31 8:24 ` Desmond Cheong Zhi Xi 2021-07-31 8:24 ` [PATCH v2 1/2] locking/lockdep: Provide lockdep_assert{,_once}() helpers Desmond Cheong Zhi Xi 2021-07-31 8:24 ` [PATCH v2 1/2] locking/lockdep: Provide lockdep_assert{, _once}() helpers Desmond Cheong Zhi Xi 2021-07-31 8:24 ` Desmond Cheong Zhi Xi 2021-08-02 9:49 ` [PATCH v2 1/2] locking/lockdep: Provide lockdep_assert{,_once}() helpers Maarten Lankhorst 2021-08-02 9:49 ` [PATCH v2 1/2] locking/lockdep: Provide lockdep_assert{, _once}() helpers Maarten Lankhorst 2021-08-02 9:49 ` Maarten Lankhorst 2021-07-31 8:24 ` Desmond Cheong Zhi Xi [this message] 2021-07-31 8:24 ` [PATCH v2 2/2] drm: add lockdep assert to drm_is_current_master_locked Desmond Cheong Zhi Xi 2021-08-02 8:26 ` [PATCH v2 0/2] locking/lockdep, drm: apply new lockdep assert in drm_auth.c Daniel Vetter 2021-08-02 8:26 ` Daniel Vetter 2021-08-02 9:42 ` Peter Zijlstra 2021-08-02 9:42 ` Peter Zijlstra 2021-08-02 10:44 ` Desmond Cheong Zhi Xi 2021-08-02 10:44 ` Desmond Cheong Zhi Xi
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=20210731082458.1962043-3-desmondcheongzx@gmail.com \ --to=desmondcheongzx@gmail.com \ --cc=airlied@linux.ie \ --cc=boqun.feng@gmail.com \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=gregkh@linuxfoundation.org \ --cc=linux-kernel-mentees@lists.linuxfoundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=longman@redhat.com \ --cc=maarten.lankhorst@linux.intel.com \ --cc=mingo@redhat.com \ --cc=mripard@kernel.org \ --cc=peterz@infradead.org \ --cc=skhan@linuxfoundation.org \ --cc=tzimmermann@suse.de \ --cc=will@kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.