All of lore.kernel.org
 help / color / mirror / Atom feed
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,
	sumit.semwal@linaro.org, christian.koenig@amd.com
Cc: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	intel-gfx@lists.freedesktop.org, linux-media@vger.kernel.org,
	linaro-mm-sig@lists.linaro.org, skhan@linuxfoundation.org,
	gregkh@linuxfoundation.org,
	linux-kernel-mentees@lists.linuxfoundation.org
Subject: [PATCH v4 0/5] drm: update locking for modesetting
Date: Fri, 20 Aug 2021 18:02:46 +0800	[thread overview]
Message-ID: <20210820100251.448346-1-desmondcheongzx@gmail.com> (raw)

Hi,

Thanks for all the helpful feedback on the previous version.

Taking all the suggestions together, this series now converts
drm_device.master_mutex into master_rwsem, and also attempts to remove
drm_file.master_lookup_lock. There might still be lock inversions
lurking, so the output from Intel-gfx CI should be interesting to see.

Overall, this series makes the following changes:

- Patch 1: Fix a potential null ptr dereference in drm_master_release

- Patch 2: Convert master_mutex into rwsem (avoids creating a new lock)

- Patch 3: Update global mutex locking in the ioctl handler (avoids
deadlock when grabbing read lock on master_rwsem in drm_ioctl_kernel)

- Patch 4: Plug races with drm modesetting rights

- Patch 5: Replace master_lookup_lock with master_rwsem by untangling
remaining lock hierarchy inversions

v3 -> v4 (suggested by Daniel Vetter):
- Drop a patch that added an unnecessary master_lookup_lock in
drm_master_release (previously patch 2)
- Drop a patch that addressed a non-existent race in
drm_is_current_master_locked (previously patch 3)
- Remove fixes for non-existent null ptr dereferences (previous patch 4)
- Protect drm_master.magic_map,unique{_len} with master_rwsem instead of
master_lookup_lock (dropped previous patch 5)
- Drop the patch that moved master_lookup_lock into struct drm_device
(previously patch 1)
- Drop a patch to export task_work_add (previously patch 8)
- Revert the check for the global mutex in the ioctl handler to use
drm_core_check_feature instead of drm_dev_needs_global_mutex
- Push down master_rwsem locking for selected ioctls to avoid lock
hierarchy inversions, and to allow us to hold write locks on
master_rwsem instead of flushing readers
- Remove master_lookup_lock by replacing it with master_rwsem

v2 -> v3:
- Unexport drm_master_flush, as suggested by Daniel Vetter.
- Merge master_mutex and master_rwsem, as suggested by Daniel Vetter.
- Export task_work_add, reported by kernel test robot.
- Make master_flush static, reported by kernel test robot.
- Move master_lookup_lock into struct drm_device.
- Add a missing lock on master_lookup_lock in drm_master_release.
- Fix a potential race in drm_is_current_master_locked.
- Fix potential null ptr dereferences in drm_{auth, ioctl}.
- Protect magic_map,unique{_len} with  master_lookup_lock.
- Convert master_mutex into a rwsem.
- Update global mutex locking in the ioctl handler.

v1 -> v2 (suggested by Daniel Vetter):
- Address an additional race when drm_open runs.
- Switch from SRCU to rwsem to synchronise readers and writers.
- Implement drm_master_flush with task_work so that flushes can be
queued to run before returning to userspace without creating a new
DRM_MASTER_FLUSH ioctl flag.

Best wishes,
Desmond

Desmond Cheong Zhi Xi (5):
  drm: fix null ptr dereference in drm_master_release
  drm: convert drm_device.master_mutex into a rwsem
  drm: lock drm_global_mutex earlier in the ioctl handler
  drm: avoid races with modesetting rights
  drm: remove drm_file.master_lookup_lock

 drivers/gpu/drm/drm_auth.c        | 54 ++++++++++++------------
 drivers/gpu/drm/drm_debugfs.c     |  4 +-
 drivers/gpu/drm/drm_drv.c         |  3 +-
 drivers/gpu/drm/drm_file.c        |  7 ++--
 drivers/gpu/drm/drm_internal.h    |  1 +
 drivers/gpu/drm/drm_ioctl.c       | 48 ++++++++++++---------
 drivers/gpu/drm/drm_lease.c       | 69 ++++++++++++++-----------------
 drivers/gpu/drm/drm_mode_object.c | 14 +++++--
 include/drm/drm_auth.h            |  6 +--
 include/drm/drm_device.h          | 15 +++++--
 include/drm/drm_file.h            | 17 +++-----
 include/drm/drm_lease.h           |  2 +-
 12 files changed, 125 insertions(+), 115 deletions(-)

-- 
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,
	sumit.semwal@linaro.org, christian.koenig@amd.com
Cc: intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org,
	Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>,
	linux-kernel-mentees@lists.linuxfoundation.org,
	linux-media@vger.kernel.org
Subject: [PATCH v4 0/5] drm: update locking for modesetting
Date: Fri, 20 Aug 2021 18:02:46 +0800	[thread overview]
Message-ID: <20210820100251.448346-1-desmondcheongzx@gmail.com> (raw)

Hi,

Thanks for all the helpful feedback on the previous version.

Taking all the suggestions together, this series now converts
drm_device.master_mutex into master_rwsem, and also attempts to remove
drm_file.master_lookup_lock. There might still be lock inversions
lurking, so the output from Intel-gfx CI should be interesting to see.

Overall, this series makes the following changes:

- Patch 1: Fix a potential null ptr dereference in drm_master_release

- Patch 2: Convert master_mutex into rwsem (avoids creating a new lock)

- Patch 3: Update global mutex locking in the ioctl handler (avoids
deadlock when grabbing read lock on master_rwsem in drm_ioctl_kernel)

- Patch 4: Plug races with drm modesetting rights

- Patch 5: Replace master_lookup_lock with master_rwsem by untangling
remaining lock hierarchy inversions

v3 -> v4 (suggested by Daniel Vetter):
- Drop a patch that added an unnecessary master_lookup_lock in
drm_master_release (previously patch 2)
- Drop a patch that addressed a non-existent race in
drm_is_current_master_locked (previously patch 3)
- Remove fixes for non-existent null ptr dereferences (previous patch 4)
- Protect drm_master.magic_map,unique{_len} with master_rwsem instead of
master_lookup_lock (dropped previous patch 5)
- Drop the patch that moved master_lookup_lock into struct drm_device
(previously patch 1)
- Drop a patch to export task_work_add (previously patch 8)
- Revert the check for the global mutex in the ioctl handler to use
drm_core_check_feature instead of drm_dev_needs_global_mutex
- Push down master_rwsem locking for selected ioctls to avoid lock
hierarchy inversions, and to allow us to hold write locks on
master_rwsem instead of flushing readers
- Remove master_lookup_lock by replacing it with master_rwsem

v2 -> v3:
- Unexport drm_master_flush, as suggested by Daniel Vetter.
- Merge master_mutex and master_rwsem, as suggested by Daniel Vetter.
- Export task_work_add, reported by kernel test robot.
- Make master_flush static, reported by kernel test robot.
- Move master_lookup_lock into struct drm_device.
- Add a missing lock on master_lookup_lock in drm_master_release.
- Fix a potential race in drm_is_current_master_locked.
- Fix potential null ptr dereferences in drm_{auth, ioctl}.
- Protect magic_map,unique{_len} with  master_lookup_lock.
- Convert master_mutex into a rwsem.
- Update global mutex locking in the ioctl handler.

v1 -> v2 (suggested by Daniel Vetter):
- Address an additional race when drm_open runs.
- Switch from SRCU to rwsem to synchronise readers and writers.
- Implement drm_master_flush with task_work so that flushes can be
queued to run before returning to userspace without creating a new
DRM_MASTER_FLUSH ioctl flag.

Best wishes,
Desmond

Desmond Cheong Zhi Xi (5):
  drm: fix null ptr dereference in drm_master_release
  drm: convert drm_device.master_mutex into a rwsem
  drm: lock drm_global_mutex earlier in the ioctl handler
  drm: avoid races with modesetting rights
  drm: remove drm_file.master_lookup_lock

 drivers/gpu/drm/drm_auth.c        | 54 ++++++++++++------------
 drivers/gpu/drm/drm_debugfs.c     |  4 +-
 drivers/gpu/drm/drm_drv.c         |  3 +-
 drivers/gpu/drm/drm_file.c        |  7 ++--
 drivers/gpu/drm/drm_internal.h    |  1 +
 drivers/gpu/drm/drm_ioctl.c       | 48 ++++++++++++---------
 drivers/gpu/drm/drm_lease.c       | 69 ++++++++++++++-----------------
 drivers/gpu/drm/drm_mode_object.c | 14 +++++--
 include/drm/drm_auth.h            |  6 +--
 include/drm/drm_device.h          | 15 +++++--
 include/drm/drm_file.h            | 17 +++-----
 include/drm/drm_lease.h           |  2 +-
 12 files changed, 125 insertions(+), 115 deletions(-)

-- 
2.25.1

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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,
	sumit.semwal@linaro.org, christian.koenig@amd.com
Cc: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	intel-gfx@lists.freedesktop.org, linux-media@vger.kernel.org,
	linaro-mm-sig@lists.linaro.org, skhan@linuxfoundation.org,
	gregkh@linuxfoundation.org,
	linux-kernel-mentees@lists.linuxfoundation.org
Subject: [Intel-gfx] [PATCH v4 0/5] drm: update locking for modesetting
Date: Fri, 20 Aug 2021 18:02:46 +0800	[thread overview]
Message-ID: <20210820100251.448346-1-desmondcheongzx@gmail.com> (raw)

Hi,

Thanks for all the helpful feedback on the previous version.

Taking all the suggestions together, this series now converts
drm_device.master_mutex into master_rwsem, and also attempts to remove
drm_file.master_lookup_lock. There might still be lock inversions
lurking, so the output from Intel-gfx CI should be interesting to see.

Overall, this series makes the following changes:

- Patch 1: Fix a potential null ptr dereference in drm_master_release

- Patch 2: Convert master_mutex into rwsem (avoids creating a new lock)

- Patch 3: Update global mutex locking in the ioctl handler (avoids
deadlock when grabbing read lock on master_rwsem in drm_ioctl_kernel)

- Patch 4: Plug races with drm modesetting rights

- Patch 5: Replace master_lookup_lock with master_rwsem by untangling
remaining lock hierarchy inversions

v3 -> v4 (suggested by Daniel Vetter):
- Drop a patch that added an unnecessary master_lookup_lock in
drm_master_release (previously patch 2)
- Drop a patch that addressed a non-existent race in
drm_is_current_master_locked (previously patch 3)
- Remove fixes for non-existent null ptr dereferences (previous patch 4)
- Protect drm_master.magic_map,unique{_len} with master_rwsem instead of
master_lookup_lock (dropped previous patch 5)
- Drop the patch that moved master_lookup_lock into struct drm_device
(previously patch 1)
- Drop a patch to export task_work_add (previously patch 8)
- Revert the check for the global mutex in the ioctl handler to use
drm_core_check_feature instead of drm_dev_needs_global_mutex
- Push down master_rwsem locking for selected ioctls to avoid lock
hierarchy inversions, and to allow us to hold write locks on
master_rwsem instead of flushing readers
- Remove master_lookup_lock by replacing it with master_rwsem

v2 -> v3:
- Unexport drm_master_flush, as suggested by Daniel Vetter.
- Merge master_mutex and master_rwsem, as suggested by Daniel Vetter.
- Export task_work_add, reported by kernel test robot.
- Make master_flush static, reported by kernel test robot.
- Move master_lookup_lock into struct drm_device.
- Add a missing lock on master_lookup_lock in drm_master_release.
- Fix a potential race in drm_is_current_master_locked.
- Fix potential null ptr dereferences in drm_{auth, ioctl}.
- Protect magic_map,unique{_len} with  master_lookup_lock.
- Convert master_mutex into a rwsem.
- Update global mutex locking in the ioctl handler.

v1 -> v2 (suggested by Daniel Vetter):
- Address an additional race when drm_open runs.
- Switch from SRCU to rwsem to synchronise readers and writers.
- Implement drm_master_flush with task_work so that flushes can be
queued to run before returning to userspace without creating a new
DRM_MASTER_FLUSH ioctl flag.

Best wishes,
Desmond

Desmond Cheong Zhi Xi (5):
  drm: fix null ptr dereference in drm_master_release
  drm: convert drm_device.master_mutex into a rwsem
  drm: lock drm_global_mutex earlier in the ioctl handler
  drm: avoid races with modesetting rights
  drm: remove drm_file.master_lookup_lock

 drivers/gpu/drm/drm_auth.c        | 54 ++++++++++++------------
 drivers/gpu/drm/drm_debugfs.c     |  4 +-
 drivers/gpu/drm/drm_drv.c         |  3 +-
 drivers/gpu/drm/drm_file.c        |  7 ++--
 drivers/gpu/drm/drm_internal.h    |  1 +
 drivers/gpu/drm/drm_ioctl.c       | 48 ++++++++++++---------
 drivers/gpu/drm/drm_lease.c       | 69 ++++++++++++++-----------------
 drivers/gpu/drm/drm_mode_object.c | 14 +++++--
 include/drm/drm_auth.h            |  6 +--
 include/drm/drm_device.h          | 15 +++++--
 include/drm/drm_file.h            | 17 +++-----
 include/drm/drm_lease.h           |  2 +-
 12 files changed, 125 insertions(+), 115 deletions(-)

-- 
2.25.1


             reply	other threads:[~2021-08-20 10:03 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-20 10:02 Desmond Cheong Zhi Xi [this message]
2021-08-20 10:02 ` [Intel-gfx] [PATCH v4 0/5] drm: update locking for modesetting Desmond Cheong Zhi Xi
2021-08-20 10:02 ` Desmond Cheong Zhi Xi
2021-08-20 10:02 ` [PATCH v4 1/5] drm: fix null ptr dereference in drm_master_release Desmond Cheong Zhi Xi
2021-08-20 10:02   ` [Intel-gfx] " Desmond Cheong Zhi Xi
2021-08-20 10:02   ` Desmond Cheong Zhi Xi
2021-08-20 10:02 ` [PATCH v4 2/5] drm: convert drm_device.master_mutex into a rwsem Desmond Cheong Zhi Xi
2021-08-20 10:02   ` [Intel-gfx] " Desmond Cheong Zhi Xi
2021-08-20 10:02   ` Desmond Cheong Zhi Xi
2021-08-20 10:02 ` [PATCH v4 3/5] drm: lock drm_global_mutex earlier in the ioctl handler Desmond Cheong Zhi Xi
2021-08-20 10:02   ` [Intel-gfx] " Desmond Cheong Zhi Xi
2021-08-20 10:02   ` Desmond Cheong Zhi Xi
2021-08-20 10:02 ` [PATCH v4 4/5] drm: avoid races with modesetting rights Desmond Cheong Zhi Xi
2021-08-20 10:02   ` [Intel-gfx] " Desmond Cheong Zhi Xi
2021-08-20 10:02   ` Desmond Cheong Zhi Xi
2021-08-20 10:02 ` [PATCH v4 5/5] drm: remove drm_file.master_lookup_lock Desmond Cheong Zhi Xi
2021-08-20 10:02   ` [Intel-gfx] " Desmond Cheong Zhi Xi
2021-08-20 10:02   ` Desmond Cheong Zhi Xi
2021-08-20 11:50 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm: update locking for modesetting Patchwork
2021-08-20 12:01 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

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=20210820100251.448346-1-desmondcheongzx@gmail.com \
    --to=desmondcheongzx@gmail.com \
    --cc=airlied@linux.ie \
    --cc=christian.koenig@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=sumit.semwal@linaro.org \
    --cc=tzimmermann@suse.de \
    /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
Be 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.