These is also this made by Luca https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11813 @Nikula, Jani , ok to merge. Already tested at trybot https://patchwork.freedesktop.org/series/136776/ > -----Original Message----- > From: Coelho, Luciano <luciano.coelho@xxxxxxxxx> > Sent: Thursday, 1 August 2024 11.23 > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Saarinen, Jani <jani.saarinen@xxxxxxxxx> > Subject: [core-for-CI PATCH] scsi: sd: Move sd_read_cpr() out of the q- > >limits_lock region > > From: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> > > Commit 804e498e0496 ("sd: convert to the atomic queue limits API") > introduced pairs of function calls to queue_limits_start_update() and > queue_limits_commit_update(). These two functions lock and unlock > q->limits_lock. In sd_revalidate_disk(), sd_read_cpr() is called after > queue_limits_start_update() call and before > queue_limits_commit_update() call. sd_read_cpr() locks q->sysfs_dir_lock > and &q->sysfs_lock. Then new lock dependencies were created between > q->limits_lock, q->sysfs_dir_lock and q->sysfs_lock, as follows: > > sd_revalidate_disk > queue_limits_start_update > mutex_lock(&q->limits_lock) > sd_read_cpr > disk_set_independent_access_ranges > mutex_lock(&q->sysfs_dir_lock) > mutex_lock(&q->sysfs_lock) > mutex_unlock(&q->sysfs_lock) > mutex_unlock(&q->sysfs_dir_lock) > queue_limits_commit_update > mutex_unlock(&q->limits_lock) > > However, the three locks already had reversed dependencies in other places. > Then the new dependencies triggered the lockdep WARN "possible circular > locking dependency detected" [1]. This WARN was observed by running the > blktests test case srp/002. > > To avoid the WARN, move the sd_read_cpr() call in sd_revalidate_disk() after > the queue_limits_commit_update() call. In other words, move the > sd_read_cpr() call out of the q->limits_lock region. > > [1] https://lore.kernel.org/linux- > scsi/vlmv53ni3ltwxplig5qnw4xsl2h6ccxijfbqzekx76vxoim5a5@dekv7q3es3tx/ > > Fixes: 804e498e0496 ("sd: convert to the atomic queue limits API") > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> > Signed-off-by: Luca Coelho <luciano.coelho@xxxxxxxxx> > --- > drivers/scsi/sd.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index > adeaa8ab9951..08cbe3815006 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -3753,7 +3753,6 @@ static int sd_revalidate_disk(struct gendisk *disk) > sd_read_block_limits_ext(sdkp); > sd_read_block_characteristics(sdkp, &lim); > sd_zbc_read_zones(sdkp, &lim, buffer); > - sd_read_cpr(sdkp); > } > > sd_print_capacity(sdkp, old_capacity); @@ -3808,6 +3807,14 > @@ static int sd_revalidate_disk(struct gendisk *disk) > if (err) > return err; > > + /* > + * Query concurrent positioning ranges after > + * queue_limits_commit_update() unlocked q->limits_lock to avoid > + * deadlock with q->sysfs_dir_lock and q->sysfs_lock. > + */ > + if (sdkp->media_present && scsi_device_supports_vpd(sdp)) > + sd_read_cpr(sdkp); > + > /* > * For a zoned drive, revalidating the zones can be done only once > * the gendisk capacity is set. So if this fails, set back the gendisk > -- > 2.39.2
