RE: [RFC PATCH v2 0/2] Fix deadlock in ufs

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

 



> 
> On Thu, Feb 04 2021 at 11:48 -0800, Alan Stern wrote:
> >On Wed, Feb 03, 2021 at 04:13:54PM -0800, Asutosh Das wrote:
> >> Thanks Alan.
> >> I understand the issues with the current ufs design.
> >>
> >> ufs has a wlun (well-known lun) that handles power management
> commands,
> >> such as SSUs. Now this wlun (device wlun) is registered as a scsi_device.
> >> It's queue is also set up for runtime-pm. Likewise there're 2
> >> more wluns, BOOT and RPMB.
> >>
> >> Currently, the scsi devices independently runtime suspend/resume - request
> driven.
> >> So to send SSU while suspending wlun (scsi_device) this scsi device should
> >> be the last to be runtime suspended amongst all other ufs luns (scsi devices).
> The
> >> reason is syncronize_cache() is sent to luns during their suspend and if SSU
> has
> >> been sent already, it mostly would fail.
> >
> >The SCSI subsystem assumes that different LUNs operate independently.
> >Evidently that isn't true here.
> >
> >> Perhaps that's the reason to send SSU during platform-device suspend. I'm
> not
> >> sure if that's the right thing to do, but that's what it is now and is causing
> >> this deadlock.
> >> Now this wlun is also registered to bsg and some applications interact with
> rpmb
> >> wlun and the device-wlun using that interface. Registering the
> corresponding
> >> queues to runtime-pm ensures that the whole path is resumed before the
> request
> >> is issued.
> >> Because, we see this deadlock, in the RFC patch, I skipped registering the
> >> queues representing the wluns to runtime-pm, thus removing the
> restrictions to
> >> issue the request until queue is resumed.
> >> But when the requests come-in via bsg, the device has to be resumed. Hence
> the
> >> get_sync()/put_sync() in bsg driver.
> >
> >Does the bsg interface send its I/O requests to the LUNs through the
> >block request queue?
> >
> >
> >> The reason for initiating get_sync()/put_sync() on the parent device was
> because
> >> the corresponding queue of this wlun was not setup for runtime-pm
> anymore.
> >> And for ufs resuming the scsi device essentially means sending a SSU to wlun
> >> which the ufs platform device does in its runtime resume now. I'm not sure
> if
> >> that was a good idea though, hence the RFC on the patches.
> >>
> >> And now it looks to me that adding a cb to sd_suspend_runtime may not
> work.
> >> Because the scsi devices suspend asynchronously and the wlun suspends
> earlier than the others.
> >>
> >> [    7.846165]scsi 0:0:0:49488: scsi_runtime_idle
> >> [    7.851547]scsi 0:0:0:49488: device wlun
> >> [    7.851809]sd 0:0:0:49488: scsi_runtime_idle
> >> [    7.861536]sd 0:0:0:49488: scsi_runtime_suspend < suspends prior to other
> luns
> >> [...]
> >> [   12.861984]sd 0:0:0:1: [sdb] Synchronizing SCSI cache
> >> [   12.868894]sd 0:0:0:2: [sdc] Synchronizing SCSI cache
> >> [   13.124331]sd 0:0:0:0: [sda] Synchronizing SCSI cache
> >> [   13.143961]sd 0:0:0:3: [sdd] Synchronizing SCSI cache
> >> [   13.163876]sd 0:0:0:6: [sdg] Synchronizing SCSI cache
> >> [   13.164024]sd 0:0:0:4: [sde] Synchronizing SCSI cache
> >> [   13.167066]sd 0:0:0:5: [sdf] Synchronizing SCSI cache
> >> [   17.101285]sd 0:0:0:7: [sdh] Synchronizing SCSI cache
> >> [   73.889551]sd 0:0:0:4: [sde] Synchronizing SCSI cache
> >>
> >> I'm not sure if there's a way to force the wlun to suspend only after all other
> luns are suspended.
> >> Is there? I hope Bart/others help provide some inputs on this.
> >
> >I don't know what would work best for you; it depends on how the LUNs
> >are used.  But one possibility is to make sure that whenever the boot
> >and rpmb wluns are resumed, the device wlun is also resumed.  So for
> >example, the runtime-resume callback routines for the rpmb and boot
> >wluns could call pm_runtime_get_sync() for the device wlun, and their
> >runtime-suspend callback routines could call pm_runtime_put() for the
> >device wlun.  And of course there would have to be appropriate
> >operations when those LUNs are bound to and unbound from their drivers.
> >
> >Alan Stern
> >
> Thanks Alan.
> CanG & I had some discussions on it as well the other day.
> I'm now looking into creating a device link between the siblings.
> e.g. make the device wlun as a supplier for all the other luns & wluns.
> So device wlun (supplier) wouldn't suspend (runtime/system) until all of the
> other
> consumers are suspended. After this linking, I can move all the
> pm commands that are being sent by host to the dedicated suspend routine of
> the device
> wlun and the host needn't send any cmds during its suspend and layering
> violation wouldn't take place.
Regardless of your above proposal, as for the issues you were witnessing with rpmb,
That started this RFC in the first place, and the whole clearing uac series for that matter:
 "In order to conduct FFU or RPMB operations, UFS needs to clear UNIT ATTENTION condition...."

Functionally, This was already done for the device wlun, and only added the rpmb wlun.

Now you are trying to solve stuff because the rpmb is not provisioned.
a) There should be no relation between response to request-sense command,
 and if the key is programmed or not. And 
b) rpmb is accessed from user-space.  If it is not provisioned, it should processed the error (-7)
    and realize that by itself.  And also, It only makes sense that if needed,
    the access sequence will include  the request-sense command.

Therefore, IMHO, just reverting Randall commit (1918651f2d7e) and fixing the user-space code
Should suffice.

Thanks,
Avri





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux