diff options
| author | Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> | 2024-08-01 07:42:34 +0200 |
|---|---|---|
| committer | Martin K. Petersen <martin.petersen@oracle.com> | 2024-08-02 04:02:34 +0200 |
| commit | ffed586b8c4f1fdb772ee350e229863f145defb5 (patch) | |
| tree | 38006e931b9c982d64337cb10287914a98bb0eed /drivers/scsi | |
| parent | scsi: ufs: core: Fix hba->last_dme_cmd_tstamp timestamp updating logic (diff) | |
| download | linux-ffed586b8c4f1fdb772ee350e229863f145defb5.tar.xz linux-ffed586b8c4f1fdb772ee350e229863f145defb5.zip | |
scsi: sd: Move sd_read_cpr() out of the q->limits_lock region
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@wdc.com>
Link: https://lore.kernel.org/r/20240801054234.540532-1-shinichiro.kawasaki@wdc.com
Tested-by: Luca Coelho <luciano.coelho@intel.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Diffstat (limited to 'drivers/scsi')
| -rw-r--r-- | drivers/scsi/sd.c | 9 |
1 files changed, 8 insertions, 1 deletions
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 8bb3a3611851..718eb91ba9a5 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); @@ -3809,6 +3808,14 @@ static int sd_revalidate_disk(struct gendisk *disk) 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 * capacity to 0. |