RE: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-06 Thread Chanho Min
> > > > I am sorry for the reverting this patch. It's also my fault that > > I didn't check lockdep. But, We decided to keep this patch in our product. > > Freeze fail is a real problem we've had for the last two years, > > whereas lockdep's notice is not a real problem. > > We hope this issue

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-06 Thread Pavel Machek
Hi! > > On 12/04, Ingo Molnar wrote: > > > > > > * Oleg Nesterov wrote: > > > > > > > we really need to narrow the (huge) scope of ->cred_guard_mutex in exec > > paths. > > > > > > > > my attempt to fix this was nacked, and nobody suggested a better > > > > solution > > so far. > > > > > > Any

RE: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-06 Thread Chanho Min
Revert "exec: make de_thread() freezable (was: Re: Linux > 4.20-rc4) > > Ingo, et al, > > Sorry, I am sick and can't participate this discussion right now, > > On 12/04, Ingo Molnar wrote: > > > > * Oleg Nesterov wrote: > > > > > we reall

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-05 Thread Oleg Nesterov
Ingo, et al, Sorry, I am sick and can't participate this discussion right now, On 12/04, Ingo Molnar wrote: > > * Oleg Nesterov wrote: > > > we really need to narrow the (huge) scope of ->cred_guard_mutex in exec > > paths. > > > > my attempt to fix this was nacked, and nobody suggested a

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Linus Torvalds
On Tue, Dec 4, 2018 at 11:49 AM Linus Torvalds wrote: > > because honestly, the *only* reason we hold on to that lock is for the > insane and not really interesting case of "somebody tried to use > ptrace to change the creds in-flight during the exec". No, sorry, me confused. Not somebody trying

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Pavel Machek
On Tue 2018-12-04 09:31:11, Linus Torvalds wrote: > On Tue, Dec 4, 2018 at 1:58 AM Michal Hocko wrote: > > > > AFAIU both suspend and hibernation require the system to enter quiescent > > state with no task potentially interfering with suspended devices. And > > in this particular case those

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Linus Torvalds
On Tue, Dec 4, 2018 at 11:33 AM Linus Torvalds wrote: > > Looking at this, I'm agreeing that ot would be better to just try to > narrow down the cred_guard_mutex use a lot. Ho humm. This is a crazy idea, but I don't see why it wouldn't work. How about we: - stop holding on to cred_guard_mutex

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Pavel Machek
On Tue 2018-12-04 10:33:10, Ingo Molnar wrote: > > * Michal Hocko wrote: > > > I dunno. I do not use hibernation. I am a heavy user of the suspend > > though. I s2ram all the time. And I have certainly experienced cases > > where suspend has failed and I onlyi found out later when I've picked

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Linus Torvalds
On Tue, Dec 4, 2018 at 10:17 AM Michal Hocko wrote: > > > How about something like we set PF_NOFREEZE when we set PF_EXITING? At > > that point we've pretty much turned into a kernel thread, no? > > Hmm, that doesn't sound like a bad idea but I am not sure it will > help because those threads we

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Pavel Machek
Hi! > * Michal Hocko wrote: > > > > Do we actually have reports of this happening for people outside > > > Android? > > > > Not that I am aware of. > > I'd say outside of Android 99% of the use of hibernation is the fail-safe > that distributions offer on laptops with very low battery

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Michal Hocko
On Tue 04-12-18 09:31:11, Linus Torvalds wrote: > On Tue, Dec 4, 2018 at 1:58 AM Michal Hocko wrote: > > > > AFAIU both suspend and hibernation require the system to enter quiescent > > state with no task potentially interfering with suspended devices. And > > in this particular case those

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Linus Torvalds
On Tue, Dec 4, 2018 at 1:58 AM Michal Hocko wrote: > > AFAIU both suspend and hibernation require the system to enter quiescent > state with no task potentially interfering with suspended devices. And > in this particular case those de-thread-ed threads will certainly not > interfere so silencing

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Michal Hocko
On Tue 04-12-18 10:33:10, Ingo Molnar wrote: > > * Michal Hocko wrote: > > > I dunno. I do not use hibernation. I am a heavy user of the suspend > > though. I s2ram all the time. And I have certainly experienced cases > > where suspend has failed and I onlyi found out later when I've picked

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Ingo Molnar
* Michal Hocko wrote: > I dunno. I do not use hibernation. I am a heavy user of the suspend > though. I s2ram all the time. And I have certainly experienced cases > where suspend has failed and I onlyi found out later when I've picked > up my laptop from my heat up bag. Nothing fatal has

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Ingo Molnar
* Oleg Nesterov wrote: > > I reviewed the ->cred_guard_mutex code, and the mutex is held across all > > of exec() - and we always did this. > > Yes, and this was always wrong. For example, this test-case hangs: > > #include > #include > #include > #include > >

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Michal Hocko
On Tue 04-12-18 10:02:28, Ingo Molnar wrote: > > * Michal Hocko wrote: > > > > Do we actually have reports of this happening for people outside > > > Android? > > > > Not that I am aware of. > > I'd say outside of Android 99% of the use of hibernation is the fail-safe > that distributions

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Ingo Molnar
* Michal Hocko wrote: > > Do we actually have reports of this happening for people outside > > Android? > > Not that I am aware of. I'd say outside of Android 99% of the use of hibernation is the fail-safe that distributions offer on laptops with very low battery levels: the emergency

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-04 Thread Ingo Molnar
* Rafael J. Wysocki wrote: > > Note that I haven't tested the revert yet, but the code and the breakage > > looks pretty obvious. (I'll boot the revert, will follow up if that > > didn't solve the problem.) > > I can queue up a revert unless anyone beats me to that. Thanks! I have 1+ days

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Pavel Machek
On Mon 2018-12-03 09:06:18, Linus Torvalds wrote: > On Mon, Dec 3, 2018 at 6:17 AM Michal Hocko wrote: > > > > This argument just doesn't make any sense. Rare bugs are maybe even more > > annoying because you do not expect them to happen. > > Absolutely. > > And valid lockdep complaints are a

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Michal Hocko
On Mon 03-12-18 09:06:18, Linus Torvalds wrote: > On Mon, Dec 3, 2018 at 6:17 AM Michal Hocko wrote: > > > > This argument just doesn't make any sense. Rare bugs are maybe even more > > annoying because you do not expect them to happen. > > Absolutely. > > And valid lockdep complaints are a

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Linus Torvalds
On Mon, Dec 3, 2018 at 6:17 AM Michal Hocko wrote: > > This argument just doesn't make any sense. Rare bugs are maybe even more > annoying because you do not expect them to happen. Absolutely. And valid lockdep complaints are a real issue too. So I don't think there's any question that this

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Pavel Machek
On Mon 2018-12-03 15:17:37, Michal Hocko wrote: > On Mon 03-12-18 15:14:59, Pavel Machek wrote: > > On Mon 2018-12-03 14:53:51, Michal Hocko wrote: > > > On Mon 03-12-18 14:10:06, Pavel Machek wrote: > > > > On Mon 2018-12-03 13:38:57, Michal Hocko wrote: > > > > > On Mon 03-12-18 13:31:49, Oleg

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Michal Hocko
On Mon 03-12-18 15:14:59, Pavel Machek wrote: > On Mon 2018-12-03 14:53:51, Michal Hocko wrote: > > On Mon 03-12-18 14:10:06, Pavel Machek wrote: > > > On Mon 2018-12-03 13:38:57, Michal Hocko wrote: > > > > On Mon 03-12-18 13:31:49, Oleg Nesterov wrote: > > > > > On 12/03, Michal Hocko wrote: > >

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Pavel Machek
On Mon 2018-12-03 14:53:51, Michal Hocko wrote: > On Mon 03-12-18 14:10:06, Pavel Machek wrote: > > On Mon 2018-12-03 13:38:57, Michal Hocko wrote: > > > On Mon 03-12-18 13:31:49, Oleg Nesterov wrote: > > > > On 12/03, Michal Hocko wrote: > > > > > > > > > > Now, I wouldn't mind to revert this

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Michal Hocko
On Mon 03-12-18 14:10:06, Pavel Machek wrote: > On Mon 2018-12-03 13:38:57, Michal Hocko wrote: > > On Mon 03-12-18 13:31:49, Oleg Nesterov wrote: > > > On 12/03, Michal Hocko wrote: > > > > > > > > Now, I wouldn't mind to revert this because the code is really old and > > > > we haven't seen many

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Oleg Nesterov
On 12/03, Michal Hocko wrote: > > On Mon 03-12-18 13:31:49, Oleg Nesterov wrote: > > On 12/03, Michal Hocko wrote: > > > > > > Now, I wouldn't mind to revert this because the code is really old and > > > we haven't seen many bug reports about failing suspend yet. But what is > > > the actual plan

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Pavel Machek
On Mon 2018-12-03 13:38:57, Michal Hocko wrote: > On Mon 03-12-18 13:31:49, Oleg Nesterov wrote: > > On 12/03, Michal Hocko wrote: > > > > > > Now, I wouldn't mind to revert this because the code is really old and > > > we haven't seen many bug reports about failing suspend yet. But what is > > >

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Michal Hocko
On Mon 03-12-18 13:31:49, Oleg Nesterov wrote: > On 12/03, Michal Hocko wrote: > > > > Now, I wouldn't mind to revert this because the code is really old and > > we haven't seen many bug reports about failing suspend yet. But what is > > the actual plan to make this work properly? > > I don't see

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Oleg Nesterov
On 12/03, Michal Hocko wrote: > > Now, I wouldn't mind to revert this because the code is really old and > we haven't seen many bug reports about failing suspend yet. But what is > the actual plan to make this work properly? I don't see a simple solution... But we need to fix exec/de_thread

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Rafael J. Wysocki
On Monday, December 3, 2018 9:39:42 AM CET Michal Hocko wrote: > On Mon 03-12-18 08:47:00, Ingo Molnar wrote: > [...] > > I reviewed the ->cred_guard_mutex code, and the mutex is held across all > > of exec() - and we always did this. > > Yes, this is something that has been pointed out during

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Rafael J. Wysocki
On Monday, December 3, 2018 8:47:00 AM CET Ingo Molnar wrote: > > * Linus Torvalds wrote: > > > The patch stats this week look a little bit more normal than last tim, > > probably simply because it's also a normal-sized rc4 rather than the > > unusually small rc3. > > So there's a new

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Oleg Nesterov
On 12/03, Ingo Molnar wrote: > > So there's a new regression in v4.20-rc4, my desktop produces this > lockdep splat: > > [ 1772.588771] WARNING: pkexec/4633 still has locks held! > [ 1772.588773] 4.20.0-rc4-custom-00213-g93a49841322b #1 Not tainted > [ 1772.588775]

Re: [PATCH] Revert "exec: make de_thread() freezable (was: Re: Linux 4.20-rc4)

2018-12-03 Thread Michal Hocko
On Mon 03-12-18 08:47:00, Ingo Molnar wrote: [...] > I reviewed the ->cred_guard_mutex code, and the mutex is held across all > of exec() - and we always did this. Yes, this is something that has been pointed out during the review. Oleg has argued that making this path freezable is really hard