On Mon, Oct 19, 2020 at 07:17:52AM -0700, Bart Van Assche wrote: > On 10/9/20 8:25 PM, Ming Lei wrote: > > Current scsi host scan mechanism supposes to fallback into sync host > > scan if async scan is in-progress. However, this rule isn't strictly > > respected, because scsi_prep_async_scan() doesn't hold scan_mutex when > > checking shost->async_scan. When scsi_scan_host() is called > > concurrently, two async scan on same host may be started, and hang in > > do_scan_async() is observed. > > > > Fixes this issue by checking & setting shost->async_scan atomically > > with shost->scan_mutex. > > Did you perhaps mean "by serializing shost->async_scan accesses with > shost->scan_mutex"? Specifically, the following checking & setting has to be done atomically, so shost->scan_mutex is required, just as what scsi_finish_async_scan() does for clearing shost->async_scan: scsi_prep_async_scan(): if (!shost->async_scan) return NULL; ... shost->async_scan = 1 > > It is not clear to me why the shost->async_scan assignment is protected > with the host lock. Can spin_lock_irqsave(shost->host_lock, flags) and > spin_unlock_irqrestore(shost->host_lock, flags) be left out from this > function? I think it is doable to remove the ->host_lock from both scsi_prep_async_scan() and scsi_finish_async_scan(), which can be done as one follow-up cleanup. With this patch, all reading/writing shost->async_scan are protected by shost->scan_mutex. > > Anyway: > > Reviewed-by: Bart Van Assche <bvanassche@xxxxxxx> Thanks! -- Ming
