All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Eric Dumazet" <eric.dumazet@gmail.com>,
	"Oleg Nesterov" <oleg@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Maciej Żenczykowski" <maze@google.com>
Subject: Re: [PATCH] task_work: remove fifo ordering guarantee
Date: Sat, 5 Sep 2015 06:12:34 +0100	[thread overview]
Message-ID: <20150905051232.GB22011@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFwKAnKb4oTRRSes7ssXZ_cFJb6SsOVHhaYYSpFxd04yEg@mail.gmail.com>

On Sat, Aug 29, 2015 at 10:08:30AM -0700, Linus Torvalds wrote:
> Hmm.
> 
> I'm wondering if we should just make close_files() (or maybe even
> filp_close()) use a synchronous fput().
> 
> Iirc, the reason we delay fput() is that we had some nasty issues for
> the generic fput case. It was called from interrupt context by the aio
> code, and just in general there's a lot of nasty cases that can cause
> the final fput to happen (so there are lockdep issues with the mmap
> locks because the last fput being from munmap etc).
> 
> Maybe I forget some detail - it's been several years by now - but I
> think we could make the regular "close()" and "exit()" cases just use
> the synchronous fput (it's called "__fput_sync()" and currently
> explicitly limited to just kernel threads).
> 
> Al?

First of all, we'd better not count on e.g. delayed fput() *NOT* doing
task_work_add() - we still need to check if any new work had been added.
After all, final close() might very well have done a final mntput()
on a lazy-unmounted filesystem, possibly leaving us with fs shutdown via
task_work_add().  And if that sucker e.g. closes a socket, well, we are
back to closing an opened struct file, with task_work_add() etc.

I'm a bit nervious about filp_close() (that sucker is exported and widely
abused), but close_files()... sure, shouldn't be a problem.  And yes,
we can teach __close_fd() to do the same.  I really don't understand what's
the benefit, though - it's about the case when we are closing the last
descriptor for given opened file, so I would be rather surprised if slower
path taken on the way out to userland was not lost in noise...

  parent reply	other threads:[~2015-09-05  5:12 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-29  2:42 [PATCH] task_work: remove fifo ordering guarantee Eric Dumazet
2015-08-29  3:19 ` Linus Torvalds
2015-08-29  9:22   ` Ingo Molnar
2015-08-29 12:54     ` Oleg Nesterov
2015-08-31  6:02       ` Ingo Molnar
2015-08-31 12:51         ` Oleg Nesterov
2015-08-29 12:49 ` Oleg Nesterov
2015-08-29 13:57   ` Eric Dumazet
2015-08-29 14:11     ` Eric Dumazet
2015-08-29 17:08       ` Linus Torvalds
2015-08-31  5:22         ` yalin wang
2015-09-05  5:19           ` Al Viro
2015-08-31 12:44         ` Oleg Nesterov
2015-09-05  5:12         ` Al Viro [this message]
2015-09-05  5:42           ` Al Viro
2015-09-05 20:46             ` Linus Torvalds
2015-08-31 12:05     ` change filp_close() to use __fput_sync() ? (Was: [PATCH] task_work: remove fifo ordering guarantee) Oleg Nesterov
2015-09-05  5:35   ` [PATCH] task_work: remove fifo ordering guarantee Al Viro
2015-09-07 12:27     ` [PATCH?] fput: don't abuse task_work_add() too much Oleg Nesterov
2015-09-07 13:49       ` [PATCH? v2] " Oleg Nesterov
  -- strict thread matches above, loose matches on Subject: below --
2015-08-29 21:08 [PATCH] task_work: remove fifo ordering guarantee George Spelvin
2015-08-31 13:22 ` Oleg Nesterov
2015-08-31 15:21   ` George Spelvin

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=20150905051232.GB22011@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=eric.dumazet@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maze@google.com \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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.