On Sat, Jan 19, 2019 at 11:29:12AM +0100, Hugo Lefeuvre wrote: > > > as far as I understand this code, freezable_schedule() avoids blocking the > > > freezer during the schedule() call, but in the end try_to_freeze() is still > > > called so the result is the same, right? > > > I wonder why wait_event_freezable is not calling freezable_schedule(). > > > > It could be something subtle in my view. freezable_schedule() actually makes > > the freezer code not send a wake up to the sleeping task if a freeze happens, > > because the PF_FREEZER_SKIP flag is set, as you pointed. > > > > Whereas wait_event_freezable() which uses try_to_freeze() does not seem to have > > this behavior and the task will enter the freezer. So I'm a bit skeptical if > > your API will behave as expected (or at least consistently with other wait > > APIs). > > oh right, now it is clear to me: > > - schedule(); try_to_freeze() > > schedule() is called and the task enters sleep. Since PF_FREEZER_SKIP is > not set, the task wakes up as soon as try_to_freeze_tasks() is called. > Right after waking up the task calls try_to_freeze() which freezes it. > > - freezable_schedule() > > schedule() is called and the task enters sleep. Since PF_FREEZER_SKIP is > set, the task does not wake up when try_to_freeze_tasks() is called. This > is not a problem, the task can't "do anything which isn't allowed for a > frozen task" while sleeping[0]. > > When the task wakes up (timeout, or whatever other reason) it calls > try_to_freeze() which freezes it if the freeze is still underway. > > So if a freeze is triggered while the task is sleeping, a task executing > freezable_schedule() might or might not notice the freeze depending on how > long it sleeps. A task executing schedule(); try_to_freeze() will always > notice it. > > I might be wrong on that, but freezable_schedule() just seems like a > performance improvement to me. > > Now I fully agree with you that there should be a uniform definition of > "freezable" between wait_event_freezable and wait_event_freezable_hrtimeout. > This leaves me to the question: should I modify my definition of > wait_event_freezable_hrtimeout, or prepare a patch for wait_event_freezable ? > > If I am right with the performance thing, the latter might be worth > considering? > > Either way, this will be fixed in the v2. I agree, it is probably better to use freezable_schedule() for all freeze related wait APIs, and keep it consistent. Your analysis is convincing. Peter, what do you think? > > > That being said, I am not sure that the try_to_freeze() call does anything > > > in the vsoc case because there is no call to set_freezable() so the thread > > > still has PF_NOFREEZE... > > > > I traced this, and PF_NOFREEZE is not set by default for tasks. > > Well, I did not check this in practice and might be confused somewhere but > the documentation[1] says "kernel threads are not freezable by default. > However, a kernel thread may clear PF_NOFREEZE for itself by calling > set_freezable()". > > Looking at the kthreadd() definition it seems like new tasks have > PF_NOFREEZE set by default[2]. > > I'll take some time to check this in practice in the next days. > > Anyways, thanks for your time ! Yeah, no problem. thanks, - Joel _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
