Pull request: TASK_KILLABLE

Pull request: TASK_KILLABLE

[Matthew Wilcox]

Hi Linus,

I'd like you to pull the task_killable branch of
git://git.kernel.org/pub/scm/linux/kernel/git/willy/misc.git

All the patches have been posted to linux-kernel before, and various
comments (and acks) have been taken into account.

Andrew's had the first fifteen patches (up to, and including 'Use
lock_page_killable') in his tree for a few months.  There's a lot more
work pending, but it's all incremental improvements.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: Pull request: TASK_KILLABLE

[Linus Torvalds]

On Mon, 28 Jan 2008, Matthew Wilcox wrote:
> 
> I'd like you to pull the task_killable branch of 
> git://git.kernel.org/pub/scm/linux/kernel/git/willy/misc.git

Well, Andrew already pointed out some of this, but I do want more 
information. There's a "git request-pull" script that even comes with git 
and that generates a shortlog, a proper url+branch description, and a 
diffstat of the changes.

So that is already much more descriptive, but in addition to that, if this 
isn't one of the "regular" pulls (which it obviously isn't), I really want 
a human description too so that I know what I'm pulling and what it's all 
about.

I do *not* want to be in the position of having to fetch first, just to be 
able to look at what I'm fetching. By the time I pull a branch, I want to 
basically know (a) what I'm pulling and (b) why I _want_ to pull it.

		Linus

Re: Pull request: TASK_KILLABLE

[Matthew Wilcox]

On Tue, Jan 29, 2008 at 10:41:24PM +1100, Linus Torvalds wrote:
> On Mon, 28 Jan 2008, Matthew Wilcox wrote:
> > 
> > I'd like you to pull the task_killable branch of 
> > git://git.kernel.org/pub/scm/linux/kernel/git/willy/misc.git
> 
> Well, Andrew already pointed out some of this, but I do want more 
> information. There's a "git request-pull" script that even comes with git 
> and that generates a shortlog, a proper url+branch description, and a 
> diffstat of the changes.
> 
> So that is already much more descriptive, but in addition to that, if this 
> isn't one of the "regular" pulls (which it obviously isn't), I really want 
> a human description too so that I know what I'm pulling and what it's all 
> about.
> 
> I do *not* want to be in the position of having to fetch first, just to be 
> able to look at what I'm fetching. By the time I pull a branch, I want to 
> basically know (a) what I'm pulling and (b) why I _want_ to pull it.

OK.  Hopefully this satisfies you:


Introduce TASK_KILLABLE.

The basic idea was Linus', back in 2002.  We can't normally return a
short read because it breaks applications.  But if a task has received
a fatal signal, it doesn't have a chance to notice that it didn't get
all the data it asked for.

To allow tasks to be interrupted by fatal signals, we introduce a new
TASK_* bit; TASK_WAKEKILL.  We also add a predicate fatal_signal_pending;
the counterpart of signal_pending().  Then we add killable versions
of lock_page(), mutex_lock(), schedule_timeout(), wait_event(), and
wait_for_completion().  Finally, we can make the NFS 'intr' mount option
a no-op.

At the end of this patch set, you can now kill a task in readdir(), read()
and write(), at least most of the time.  There is still much more work
that can (and should) be done, such as implementing down_killable(),
lock_kernel_killable() and even spin_lock_killable().

While my main motivation has been to fix problems I have with flaky
wireless networks and/or NFS servers, people like Ingo and Nick Piggin
want to see this infrastructure for helping them improve the OOM killer.

The following changes since commit 8af03e782cae1e0a0f530ddd22301cdd12cf9dc0:
  Linus Torvalds (1):
        Merge branch 'for-2.6.25' of git://git.kernel.org/.../paulus/powerpc

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/willy/misc.git task_killable

Liam R. Howlett (2):
      Add mutex_lock_killable
      Use mutex_lock_killable in vfs_readdir

Matthew Wilcox (20):
      Use wake_up_locked() in eventpoll
      Add macros to replace direct uses of TASK_ flags
      perfmon: Use task_is_*
      proc/array.c: Use TASK_REPORT
      proc/base.c: Use task_is_*
      wait: Use TASK_NORMAL
      power: Use task_is_*
      ptrace: Use task_is_*
      sched: Use task_contributes_to_load, TASK_ALL and TASK_NORMAL
      signal: Use task_is_*
      exit: Use task_is_*
      Add TASK_WAKEKILL
      Add fatal_signal_pending
      Add lock_page_killable
      Use lock_page_killable
      Add schedule_timeout_killable
      Add wait_event_killable
      Add wait_for_completion_killable
      NFS: Switch from intr mount option to TASK_KILLABLE
      Remove commented-out code copied from NFS

 arch/ia64/kernel/perfmon.c   |    4 +-
 fs/eventpoll.c               |   11 ++---
 fs/nfs/client.c              |    6 +--
 fs/nfs/direct.c              |   10 +----
 fs/nfs/inode.c               |    6 +--
 fs/nfs/mount_clnt.c          |    2 +-
 fs/nfs/nfs3proc.c            |    7 +--
 fs/nfs/nfs4proc.c            |   27 +++---------
 fs/nfs/nfsroot.c             |    3 -
 fs/nfs/pagelist.c            |   18 ++------
 fs/nfs/read.c                |    5 --
 fs/nfs/super.c               |    4 --
 fs/nfs/write.c               |    7 +---
 fs/proc/array.c              |    7 +---
 fs/proc/base.c               |    2 +-
 fs/readdir.c                 |    5 ++-
 fs/smbfs/request.c           |    2 +-
 include/linux/completion.h   |    1 +
 include/linux/mutex.h        |    5 ++
 include/linux/nfs_fs.h       |    9 +----
 include/linux/nfs_mount.h    |    2 +-
 include/linux/pagemap.h      |   14 +++++++
 include/linux/sched.h        |   36 ++++++++++++++++-
 include/linux/sunrpc/clnt.h  |    2 -
 include/linux/sunrpc/sched.h |    2 -
 include/linux/wait.h         |   52 ++++++++++++++++++++++--
 kernel/exit.c                |   88 ++++++++++++++++++-----------------------
 kernel/mutex.c               |   36 ++++++++++++++++-
 kernel/power/process.c       |    6 +-
 kernel/ptrace.c              |    8 ++--
 kernel/sched.c               |   28 +++++++++-----
 kernel/signal.c              |   19 ++++++---
 kernel/timer.c               |    7 +++
 kernel/wait.c                |    2 +-
 mm/filemap.c                 |   25 ++++++++++--
 net/sunrpc/clnt.c            |   47 +---------------------
 net/sunrpc/rpcb_clnt.c       |    3 +-
 net/sunrpc/sched.c           |   15 +++----
 net/sunrpc/sunrpc_syms.c     |    2 -
 39 files changed, 281 insertions(+), 254 deletions(-)

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: Pull request: TASK_KILLABLE

[Linus Torvalds]

On Thu, 31 Jan 2008, Matthew Wilcox wrote:
> 
> To allow tasks to be interrupted by fatal signals, we introduce a new
> TASK_* bit; TASK_WAKEKILL.  We also add a predicate fatal_signal_pending;
> the counterpart of signal_pending().  Then we add killable versions
> of lock_page(), mutex_lock(), schedule_timeout(), wait_event(), and
> wait_for_completion().  Finally, we can make the NFS 'intr' mount option
> a no-op.

Ok, I've pulled this. It conflicted with some NFS changes, but I did what 
appeared to be the obvious conflict resolution, and it all at least 
compiles and it looked sane to me. But I don't have NFS enabled normally, 
and I certainly don't have a good way to test with my laptop while in 
Melbourne, so Trond/Matthew, can you please double- and triple-check my 
merge?

			Linus

Re: Pull request: TASK_KILLABLE

[Trond Myklebust]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]


On Fri, 2008-02-01 at 11:48 +1100, Linus Torvalds wrote:
> 
> On Thu, 31 Jan 2008, Matthew Wilcox wrote:
> > 
> > To allow tasks to be interrupted by fatal signals, we introduce a new
> > TASK_* bit; TASK_WAKEKILL.  We also add a predicate fatal_signal_pending;
> > the counterpart of signal_pending().  Then we add killable versions
> > of lock_page(), mutex_lock(), schedule_timeout(), wait_event(), and
> > wait_for_completion().  Finally, we can make the NFS 'intr' mount option
> > a no-op.
> 
> Ok, I've pulled this. It conflicted with some NFS changes, but I did what 
> appeared to be the obvious conflict resolution, and it all at least 
> compiles and it looked sane to me. But I don't have NFS enabled normally, 
> and I certainly don't have a good way to test with my laptop while in 
> Melbourne, so Trond/Matthew, can you please double- and triple-check my 
> merge?

Hmm... The current code won't compile as a module. We're at least going
to require something like the attached patch.

Cheers
  Trond

[-- Attachment #2: linux-2.6.24-000-export_fatal_signal_pending.dif --]
[-- Type: message/rfc822, Size: 803 bytes --]

From: Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: Ensure that we export __fatal_signal_pending()
Date: Thu, 31 Jan 2008 20:40:29 -0500
Message-ID: <1201830304.3087.3.camel@heimdal.trondhjem.org>

It may be used by the modules nfs.ko and sunrpc.ko

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 kernel/signal.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 8054dd4..1f2d4e1 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -998,6 +998,7 @@ int fastcall __fatal_signal_pending(struct task_struct *tsk)
 {
 	return sigismember(&tsk->pending.signal, SIGKILL);
 }
+EXPORT_SYMBOL_GPL(__fatal_signal_pending);
 
 /*
  * Must be called under rcu_read_lock() or with tasklist_lock read-held.

Re: Pull request: TASK_KILLABLE

[Linus Torvalds]

On Thu, 31 Jan 2008, Trond Myklebust wrote:
> 
> Hmm... The current code won't compile as a module. We're at least going
> to require something like the attached patch.

Ahh, ok, I obviously only tested compiling it in.

No need to make that one-liner function be a GPL-only export, it's not 
like any of the other regular sigpending functions are we export are 
GPL-only either (dequeue_signal() is, but that is because we *really* 
don't want people using that, and it's really about internal signal 
handling behavior).

			Linus

Re: Pull request: TASK_KILLABLE

[Andrew Morton]

On Thu, 31 Jan 2008 20:45:04 -0500 Trond Myklebust <Trond.Myklebust@netapp.com> wrote:

> 
> On Fri, 2008-02-01 at 11:48 +1100, Linus Torvalds wrote:
> > 
> > On Thu, 31 Jan 2008, Matthew Wilcox wrote:
> > > 
> > > To allow tasks to be interrupted by fatal signals, we introduce a new
> > > TASK_* bit; TASK_WAKEKILL.  We also add a predicate fatal_signal_pending;
> > > the counterpart of signal_pending().  Then we add killable versions
> > > of lock_page(), mutex_lock(), schedule_timeout(), wait_event(), and
> > > wait_for_completion().  Finally, we can make the NFS 'intr' mount option
> > > a no-op.
> > 
> > Ok, I've pulled this. It conflicted with some NFS changes, but I did what 
> > appeared to be the obvious conflict resolution, and it all at least 
> > compiles and it looked sane to me. But I don't have NFS enabled normally, 
> > and I certainly don't have a good way to test with my laptop while in 
> > Melbourne, so Trond/Matthew, can you please double- and triple-check my 
> > merge?
> 
> Hmm... The current code won't compile as a module. We're at least going
> to require something like the attached patch.
> 
> ...
> +EXPORT_SYMBOL_GPL(__fatal_signal_pending);
>

No such export was needed in the patches which I added to -mm.  So
something changed between then and now.

And going back through the mailing list all I can find is a series of five
patches in October - it's unclear where and when the other 17 were
reviewed, if they were.

Sorry, but I don't think this work has been suitably reviewed and tested.

Re: Pull request: TASK_KILLABLE

[Trond Myklebust]

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]


On Fri, 2008-02-01 at 12:54 +1100, Linus Torvalds wrote:
> No need to make that one-liner function be a GPL-only export, it's not 
> like any of the other regular sigpending functions are we export are 
> GPL-only either (dequeue_signal() is, but that is because we *really* 
> don't want people using that, and it's really about internal signal 
> handling behavior).

No. It was more in the spirit of "let's not export a function that
others may implement trivially using established interfaces". However if
you don't care, then here's a replacement.

Cheers
  Trond

[-- Attachment #2: Type: message/rfc822, Size: 800 bytes --]

From: Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: Ensure that we export __fatal_signal_pending()
Date: Thu, 31 Jan 2008 20:40:29 -0500
Message-ID: <1201831343.3087.12.camel@heimdal.trondhjem.org>

It may be used by the modules nfs.ko and sunrpc.ko

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 kernel/signal.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 8054dd4..4333b6d 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -998,6 +998,7 @@ int fastcall __fatal_signal_pending(struct task_struct *tsk)
 {
 	return sigismember(&tsk->pending.signal, SIGKILL);
 }
+EXPORT_SYMBOL(__fatal_signal_pending);
 
 /*
  * Must be called under rcu_read_lock() or with tasklist_lock read-held.

Re: Pull request: TASK_KILLABLE

[Matthew Wilcox]

On Thu, Jan 31, 2008 at 06:02:05PM -0800, Andrew Morton wrote:
> No such export was needed in the patches which I added to -mm.  So
> something changed between then and now.

Not sure abouut that problem -- still on holiday, so just checking ym
mail quickly.

> And going back through the mailing list all I can find is a series of five
> patches in October - it's unclear where and when the other 17 were
> reviewed, if they were.

A large number of these patches are just a resplit of the patches sent
back in October -- you complained they weren't split up enough.  So I
resplit them.  And sent them to you.  Asking if this was how you
preferred it.  Which you didn't reply to.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: Pull request: TASK_KILLABLE

[Andrew Morton]

On Tue, 5 Feb 2008 19:19:42 -0700 Matthew Wilcox <matthew@wil.cx> wrote:

> > And going back through the mailing list all I can find is a series of five
> > patches in October - it's unclear where and when the other 17 were
> > reviewed, if they were.
> 
> A large number of these patches are just a resplit of the patches sent
> back in October -- you complained they weren't split up enough.  So I
> resplit them.  And sent them to you.  Asking if this was how you
> preferred it.  Which you didn't reply to.

Well, I apologise if that's the case, but...  I can find no record of the
later patch series.  Maybe an MTA ate them?