All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chanho Min" <chanho.min@lge.com>
To: 'Michal Hocko' <mhocko@kernel.org>, 'Oleg Nesterov' <oleg@redhat.com>
Cc: "'Rafael J. Wysocki'" <rjw@rjwysocki.net>,
	'Pavel Machek' <pavel@ucw.cz>, 'Len Brown' <len.brown@intel.com>,
	'Andrew Morton' <akpm@linux-foundation.org>,
	"'Eric W. Biederman'" <ebiederm@xmission.com>,
	'Christian Brauner' <christian@brauner.io>,
	'Anna-Maria Gleixner' <anna-maria@linutronix.de>,
	'Alexander Viro' <viro@zeniv.linux.org.uk>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	'Seungho Park' <seungho1.park@lge.com>,
	'Inkyu Hwang' <inkyu.hwang@lge.com>,
	'Donghwan Jung' <donghwan.jung@lge.com>,
	'Jongsung Kim' <neidhard.kim@lge.com>
Subject: RE: [PATCH v2] exec: make de_thread() freezable
Date: Wed, 14 Nov 2018 19:18:42 +0900	[thread overview]
Message-ID: <014a01d47c03$6b64eef0$422eccd0$@lge.com> (raw)
In-Reply-To: <20181113180058.GT15120@dhcp22.suse.cz>

> > > It's been some time since I have looked into this code so bear with
me.
> > > One thing is not really clear to me. Why does it help to exclude this
> > > particular task from the freezer
> >
> > we don't exclude it,
> >
> > > when it is not sleeping in the freezer.
> >
> > Yes, it is not sleeping in __refrigerator(), but it does
> >
> > 	schedule();
> > 	freezer_count();
> >
> > so it will enter __refrigerator() right after wakeup. If it won't be
> woken
> > up we do not care, we can consider it "frozen".
> 
> Right, but this is just silencing the freezing code to exclude this
> task, right?
> 
> > > I can see how other threads need to be zapped and TASK_WAKEKILL
> doesn't
> > > do that but shouldn't we fix that instead?
> >
> > Not sure I understand, but unlikely we can (or want) to make
> __refrigerator()
> > killable.
> 
> Why would that be a problem. If the kill is fatal then why to keep the
> killed task in the fridge?
> 

Is it  different between 'the killed task is frozen' and '__refrigerator()
is killable'?
>From a general '__refrigerator()' implementation point of view I know that
it should not be killable.

> > Otherwise, how can we fix that?
> 
> We can mark all threads PF_NOFREEZE and wake them up. This would require
> some more changes of course but wouldn't that be a more appropriate
> solution? Do we want to block exec for ever just because some threads
> are in the fridge?
> 

IMHO, It seems to be difficult and buggy to control with PF_NOFREEZE.
Because,
The sub-thread can freeze and receive SIG_KILL before the marking of
PF_NOFREEZE
and it should be freezable in other cases. I don't understand why it isn't
appropriate
for exec to block. The exec can freeze. When tasks are thawed, the killed
sub-thread
will die and wake de_thread(). The exec will continue to work from resume.

Chanho

WARNING: multiple messages have this Message-ID (diff)
From: "Chanho Min" <chanho.min@lge.com>
To: "'Michal Hocko'" <mhocko@kernel.org>,
	"'Oleg Nesterov'" <oleg@redhat.com>
Cc: "'Rafael J. Wysocki'" <rjw@rjwysocki.net>,
	"'Pavel Machek'" <pavel@ucw.cz>,
	"'Len Brown'" <len.brown@intel.com>,
	"'Andrew Morton'" <akpm@linux-foundation.org>,
	"'Eric W. Biederman'" <ebiederm@xmission.com>,
	"'Christian Brauner'" <christian@brauner.io>,
	"'Anna-Maria Gleixner'" <anna-maria@linutronix.de>,
	"'Alexander Viro'" <viro@zeniv.linux.org.uk>,
	<linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-fsdevel@vger.kernel.org>,
	"'Seungho Park'" <seungho1.park@lge.com>,
	"'Inkyu Hwang'" <inkyu.hwang@lge.com>,
	"'Donghwan Jung'" <donghwan.jung@lge.com>,
	"'Jongsung Kim'" <neidhard.kim@lge.com>
Subject: RE: [PATCH v2] exec: make de_thread() freezable
Date: Wed, 14 Nov 2018 19:18:42 +0900	[thread overview]
Message-ID: <014a01d47c03$6b64eef0$422eccd0$@lge.com> (raw)
In-Reply-To: <20181113180058.GT15120@dhcp22.suse.cz>

> > > It's been some time since I have looked into this code so bear with
me.
> > > One thing is not really clear to me. Why does it help to exclude this
> > > particular task from the freezer
> >
> > we don't exclude it,
> >
> > > when it is not sleeping in the freezer.
> >
> > Yes, it is not sleeping in __refrigerator(), but it does
> >
> > 	schedule();
> > 	freezer_count();
> >
> > so it will enter __refrigerator() right after wakeup. If it won't be
> woken
> > up we do not care, we can consider it "frozen".
> 
> Right, but this is just silencing the freezing code to exclude this
> task, right?
> 
> > > I can see how other threads need to be zapped and TASK_WAKEKILL
> doesn't
> > > do that but shouldn't we fix that instead?
> >
> > Not sure I understand, but unlikely we can (or want) to make
> __refrigerator()
> > killable.
> 
> Why would that be a problem. If the kill is fatal then why to keep the
> killed task in the fridge?
> 

Is it  different between 'the killed task is frozen' and '__refrigerator()
is killable'?
>From a general '__refrigerator()' implementation point of view I know that
it should not be killable.

> > Otherwise, how can we fix that?
> 
> We can mark all threads PF_NOFREEZE and wake them up. This would require
> some more changes of course but wouldn't that be a more appropriate
> solution? Do we want to block exec for ever just because some threads
> are in the fridge?
> 

IMHO, It seems to be difficult and buggy to control with PF_NOFREEZE.
Because,
The sub-thread can freeze and receive SIG_KILL before the marking of
PF_NOFREEZE
and it should be freezable in other cases. I don't understand why it isn't
appropriate
for exec to block. The exec can freeze. When tasks are thawed, the killed
sub-thread
will die and wake de_thread(). The exec will continue to work from resume.

Chanho

WARNING: multiple messages have this Message-ID (diff)
From: "Chanho Min" <chanho.min@lge.com>
To: "'Michal Hocko'" <mhocko@kernel.org>,
	"'Oleg Nesterov'" <oleg@redhat.com>
Cc: "'Rafael J. Wysocki'" <rjw@rjwysocki.net>,
	"'Pavel Machek'" <pavel@ucw.cz>,
	"'Len Brown'" <len.brown@intel.com>,
	"'Andrew Morton'" <akpm@linux-foundation.org>,
	"'Eric W. Biederman'" <ebiederm@xmission.com>,
	"'Christian Brauner'" <christian@brauner.io>,
	"'Anna-Maria Gleixner'" <anna-maria@linutronix.de>,
	"'Alexander Viro'" <viro@zeniv.linux.org.uk>,
	<linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-fsdevel@vger.kernel.org>,
	"'Seungho Park'" <seungho1.park@lge.com>,
	"'Inkyu Hwang'" <inkyu.hwang@lge.com>,
	"'Donghwan Jung'" <donghwan.jung@lge.com>,
	"'Jongsung Kim'" <neidhard.kim@lge.com>
Subject: RE: [PATCH v2] exec: make de_thread() freezable
Date: Wed, 14 Nov 2018 19:18:42 +0900	[thread overview]
Message-ID: <014a01d47c03$6b64eef0$422eccd0$@lge.com> (raw)
In-Reply-To: <20181113180058.GT15120@dhcp22.suse.cz>

> > > It's been some time since I have looked into this code so bear with
me.
> > > One thing is not really clear to me. Why does it help to exclude this
> > > particular task from the freezer
> >
> > we don't exclude it,
> >
> > > when it is not sleeping in the freezer.
> >
> > Yes, it is not sleeping in __refrigerator(), but it does
> >
> > 	schedule();
> > 	freezer_count();
> >
> > so it will enter __refrigerator() right after wakeup. If it won't be
> woken
> > up we do not care, we can consider it "frozen".
> 
> Right, but this is just silencing the freezing code to exclude this
> task, right?
> 
> > > I can see how other threads need to be zapped and TASK_WAKEKILL
> doesn't
> > > do that but shouldn't we fix that instead?
> >
> > Not sure I understand, but unlikely we can (or want) to make
> __refrigerator()
> > killable.
> 
> Why would that be a problem. If the kill is fatal then why to keep the
> killed task in the fridge?
> 

Is it  different between 'the killed task is frozen' and '__refrigerator()
is killable'?
From a general '__refrigerator()' implementation point of view I know that
it should not be killable.

> > Otherwise, how can we fix that?
> 
> We can mark all threads PF_NOFREEZE and wake them up. This would require
> some more changes of course but wouldn't that be a more appropriate
> solution? Do we want to block exec for ever just because some threads
> are in the fridge?
> 

IMHO, It seems to be difficult and buggy to control with PF_NOFREEZE.
Because,
The sub-thread can freeze and receive SIG_KILL before the marking of
PF_NOFREEZE
and it should be freezable in other cases. I don't understand why it isn't
appropriate
for exec to block. The exec can freeze. When tasks are thawed, the killed
sub-thread
will die and wake de_thread(). The exec will continue to work from resume.

Chanho


  reply	other threads:[~2018-11-14 10:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-12  3:54 [PATCH v2] exec: make de_thread() freezable Chanho Min
2018-11-12  8:15 ` Oleg Nesterov
2018-11-21 23:35   ` Rafael J. Wysocki
2018-11-12  9:51 ` Pavel Machek
2018-11-13 14:53 ` Michal Hocko
2018-11-13 16:18   ` Oleg Nesterov
2018-11-13 18:00     ` Michal Hocko
2018-11-14 10:18       ` Chanho Min [this message]
2018-11-14 10:18         ` Chanho Min
2018-11-14 10:18         ` Chanho Min
2018-11-14 10:30         ` Michal Hocko
2018-11-14 14:37           ` Oleg Nesterov
2018-11-14 14:29       ` Oleg Nesterov
2018-11-14 11:31   ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='014a01d47c03$6b64eef0$422eccd0$@lge.com' \
    --to=chanho.min@lge.com \
    --cc=akpm@linux-foundation.org \
    --cc=anna-maria@linutronix.de \
    --cc=christian@brauner.io \
    --cc=donghwan.jung@lge.com \
    --cc=ebiederm@xmission.com \
    --cc=inkyu.hwang@lge.com \
    --cc=len.brown@intel.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mhocko@kernel.org \
    --cc=neidhard.kim@lge.com \
    --cc=oleg@redhat.com \
    --cc=pavel@ucw.cz \
    --cc=rjw@rjwysocki.net \
    --cc=seungho1.park@lge.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.