Hi Bart,
On 2021-06-24 05:51, Bart Van Assche wrote:
On 6/23/21 12:35 AM, Can Guo wrote:
+int ufshcd_get_user_access(struct ufs_hba *hba)
+__acquires(&hba->host_sem)
+{
+ down(&hba->host_sem);
+ if (!ufshcd_is_user_access_allowed(hba)) {
+ up(&hba->host_sem);
+ return -EBUSY;
+ }
+ if (ufshcd_rpm_get_sync(hba)) {
+ ufshcd_rpm_put_sync(hba);
+ up(&hba->host_sem);
+ return -EBUSY;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ufshcd_get_user_access);
+
+void ufshcd_put_user_access(struct ufs_hba *hba)
+__releases(&hba->host_sem)
+{
+ ufshcd_rpm_put_sync(hba);
+ up(&hba->host_sem);
+}
+EXPORT_SYMBOL_GPL(ufshcd_put_user_access);
Please indent __acquires() and __releases() annotations by one tab as
is
done elsewhere in the kernel.
OK.
static inline bool ufshcd_is_user_access_allowed(struct ufs_hba *hba)
{
- return !hba->shutting_down;
+ return !hba->shutting_down && !hba->is_sys_suspended &&
+ !hba->is_wlu_sys_suspended &&
+ hba->ufshcd_state == UFSHCD_STATE_OPERATIONAL;
}
Is my understanding of the following correct?
- ufshcd_is_user_access_allowed() is not in the hot path and hence
should not be inline.
OK.
- The hba->shutting_down member variable is set from inside a shutdown
callback. Hence, the hba->shutting_down test can be left out since
no UFS sysfs attributes are accessed after shutdown has started.
We see that user can still access UFS sysfs during shutdown if shutdown
is invoked by: reboot -f, hence the check.
- During system suspend, user space software is paused before the
device
driver freeze callbacks are invoked. Hence, the hba->is_sys_suspended
check can be left out.
is_sys_suspended indicates that system resume failed (power/clk is OFF).
- If a LUN is runtime suspended, it should be resumed if accessed from
user space instead of failing user space accesses. In other words,
the
hba->is_wlu_sys_suspended check seems inappropriate to me.
hba->is_wlu_sys_suspended indicates that wl system resume failed, device
is not operational.
- If the HBA is not in an operational state, user space accesses
should be blocked until error handling has finished. After error
handling has finished, the user space access should fail if and only
if error handling failed.
Yes, which is why ufshcd_get_user_access() acquires host_sem first and
checks the OPERATOINAL flag here. host_sem shall make sure that user
space accesses should be blocked until error handling has finished.
Thanks,
Can Guo.
Thanks,
Bart.
