Re: [PATCH] scsi: sd: Move sd_read_cpr() out of the q->limits_lock region

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/1/24 2:42 PM, Shin'ichiro Kawasaki wrote:
> 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>

Given that sd_read_cpr() does not change any limit, looks good to me.

Reviewed-by: Damien Le Moal <dlemoal@xxxxxxxxxx>

-- 
Damien Le Moal
Western Digital Research





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux