On 30/11/2021 19:51, Bart Van Assche wrote: > On 11/29/21 10:41 PM, Adrian Hunter wrote: >> On 29/11/2021 21:32, Bart Van Assche wrote: >>> * The code in blk_cleanup_queue() that waits for pending requests to finish >>> before resources associated with the request queue are freed. >>> ufshcd_remove() calls blk_cleanup_queue(hba->cmd_queue) and hence waits until >>> pending device management commands have finished. That would no longer be the >>> case if the block layer is bypassed to submit device management commands. >> >> cmd_queue is used only by the UFS driver, so if the driver is racing with >> itself at "remove", then that should be fixed. The risk is not that the UFS >> driver might use requests, but that it might still be operating when hba or >> other resources get freed. >> >> So the question remains, for device commands, we do not need the block >> layer, nor SCSI commands which still begs the question, why involve them >> at all? > > By using the block layer request allocation functions the block layer guarantees > that each tag is in use in only one context. When bypassing the block layer code > would have to be inserted in ufshcd_exec_dev_cmd() and ufshcd_issue_devman_upiu_cmd() > to serialize these functions. They already are serialized, but you are essentially saying the functionality being duplicated is just a lock. What you are proposing seems awfully complicated just to get the functionality of a lock. > In other words, we would be duplicating existing > functionality if we bypass the block layer. The recommended approach in the Linux > kernel is not to duplicate existing functionality. More accurately, the functionality would not be being used at all, so not really any duplication.