Hi Bean, On Fri, 2021-01-08 at 12:29 +0100, Bean Huo wrote: > On Wed, 2021-01-06 at 09:20 +0800, Can Guo wrote: > > Hi Bean, > > > > On 2021-01-06 02:38, Bean Huo wrote: > > > On Tue, 2021-01-05 at 09:07 +0800, Can Guo wrote: > > > > On 2021-01-05 04:05, Bean Huo wrote: > > > > > On Sat, 2021-01-02 at 05:59 -0800, Can Guo wrote: > > > > > > + * @shutting_down: flag to check if shutdown has been > > > > > > invoked > > > > > > > > > > I am not much sure if this flag is need, since once PM going in > > > > > shutdown path, what will be returnded by pm_runtime_get_sync()? > > > > > > > > > > If pm_runtime_get_sync() will fail, just check its return. > > > > > > > > > > > > > That depends. During/after shutdown, for UFS's case only, > > > > pm_runtime_get_sync(hba->dev) will most likely return 0, > > > > because it is already RUNTIME_ACTIVE, pm_runtime_get_sync() > > > > will directly return 0... meaning you cannot count on it. > > > > > > > > Check Stanley's change - > > > > https://lore.kernel.org/patchwork/patch/1341389/ > > > > > > > > Can Guo. > > > > > > Can, > > > > > > Thanks for pointing out that. > > > > > > Based on my understanding, that patch is redundent. maybe I > > > misundestood Linux shutdown sequence. > > > > Sorry, do you mean Stanley's change is redundant? > > yes. > > > > > > > > > I checked the shutdown flow: > > > > > > 1. Set the "system_state" variable > > > 2. Disable usermod to ensure that no user from userspace can start > > > a > > > request > > > > I hope it is like what you interpreted, but step #2 only stops > > UMH(#265) > > but not all user space activities. Whereas, UMH is for kernel space > > calling > > user space. > > > Can, > > I did further study and homework on the Linux shutdown in the last few > days. Yes, you are right, usermodehelper_disable() is to prevent > executing the process from the kernel space. > > But I didn't reproduce this "maybe" race issue while shutdown. no > matter how I torment my system, once Linux shutdown/halt/reboot starts, > nobody can access the sysfs node. I create 10 processes in the user > space and constantly access UFS sysfs node, also, fio is running in the > background for the normal data read/write. there is a shutdown thread > that will randomly trigger shutdown/halt/reboot. but no race issue > appears. > > I don't know if this is a hypothetical issue(the race between shutdown > flow and sysfs node access), it may not really exist in the Linux > envriroment. everytime, the shutdonw flow will be: > > e10_sync_handler()->e10_svc()->do_e10_svc()->__do_sys_reboot()- > >kernel_poweroff/kernel_halt()->device_shutdown()->platform_shutdown()- > >ufshcd_platform_shutdown()->ufshcd_shutdown(). > > I think before going into the kernel shutdown, the userspace cannot > issue new requests anymore. otherwise, this would be a big issue. > > pm_runtime_get_sync() will return 0 or failure while shutdown? the > answer is not important now, maybe as you said, it is always 0. But in > my testing, it didn't get there the system has been shutdown. Which > means once shutdonw starts, sysfs node access path cannot reach > pm_runtime_get_sync(). (note, I don't know if sysfs node access thread > has been disabled or not) > > > Responsibly say, I didn't reproduce this issue on my system (ubuntu), > maybe you are using Android. I am not an expert on this topic, if you > have the best idea on how to reproduce this issue. please please let me > try. appreciate it!!!!! > One of the racing in my platforms happens due to I/O access triggered from kernel space, not user space. Thanks, Stanley Chu > > Thanks, > Bean > > > > > > 264 system_state = state; > > 265 usermodehelper_disable(); > > 266 device_shutdown(); > > > > Thanks, > > Can Guo. >
