linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Matthew Wilcox <matthew@wil.cx>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	"George G. Davis" <gdavis@mvista.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [RFC, PATCH] locks: remove posix deadlock detection
Date: Sun, 28 Oct 2007 21:13:06 -0400	[thread overview]
Message-ID: <20071029011306.GA10307@fieldses.org> (raw)
In-Reply-To: <20071028184052.49abd092@the-village.bc.nu>

On Sun, Oct 28, 2007 at 06:40:52PM +0000, Alan Cox wrote:
> On Sun, 28 Oct 2007 12:27:32 -0600
> Matthew Wilcox <matthew@wil.cx> wrote:
> 
> > On Sun, Oct 28, 2007 at 01:43:21PM -0400, J. Bruce Fields wrote:
> > > We currently attempt to return -EDEALK to blocking fcntl() file locking
> > > requests that would create a cycle in the graph of tasks waiting on
> > > locks.
> > > 
> > > This is inefficient: in the general case it requires us determining
> > > whether we're adding a cycle to an arbitrary directed acyclic graph.
> > > And this calculation has to be performed while holding a lock (currently
> > > the BKL) that prevents that graph from changing.
> > > 
> > > It has historically been a source of bugs; most recently it was noticed
> > > that it could loop indefinitely while holding the BKL.
> > 
> > It can also return -EDEADLK spuriously.  So yeah, just kill it.
> 
> NAK. This is an ABI change. It was also comprehensively rejected before
> because
> 
> - EDEADLK behaviour is ABI
> - EDEADLK behaviour is required by SuSv3
> - We have no idea what applications may rely on this behaviour.

On the second point I think you're mistaken; see

	http://www.opengroup.org/onlinepubs/009695399/functions/fcntl.html

	"Since implementation of full deadlock detection is not
	always feasible, the [EDEADLK] error was made optional."

The third objection is the one that concerns me most, and is the one I'd
like to understand better.  So I'd be interested in any evidence of
applications that do actually depend on the current behavior.  (Even
hypothetical use cases might be interesting.)  I'm also curious what
other OS's do.

> See the thread
> 	http://osdir.com/ml/file-systems/2004-06/msg00017.html
> 
> so we need to fix the bugs - the lock usage and the looping. At that
> point it merely becomes a performance concern to those who use it, which
> is the proper behaviour. If you want a faster non-checking one use
> flock(), or add another flag that is a Linux "don't check for deadlock"

That's an interesting idea.  The flag might not help as much, since
looking for a cycle in the graph of blocked requests may be more
difficult in the case where we don't know whether the graph is acyclic
to start out with.  (But I don't know--I haven't thought about it much.)

--b.

  parent reply	other threads:[~2007-10-29  1:13 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20071017185157.GC3785@mvista.com>
     [not found] ` <20071018185759.GU3785@mvista.com>
     [not found]   ` <20071026170750.GC13033@fieldses.org>
     [not found]     ` <20071026224707.GO13033@fieldses.org>
2007-10-28 17:31       ` [PATCH] locks: fix possible infinite loop in posix deadlock detection J. Bruce Fields
2007-10-28 17:43         ` [RFC, PATCH] locks: remove " J. Bruce Fields
2007-10-28 18:27           ` Matthew Wilcox
2007-10-28 18:40             ` Alan Cox
2007-10-28 20:11               ` Matthew Wilcox
2007-10-28 21:38                 ` Alan Cox
2007-10-28 21:45                   ` Jiri Kosina
2007-10-28 23:38                   ` Matthew Wilcox
2007-10-28 23:44                     ` Alan Cox
2007-10-28 21:50                 ` Trond Myklebust
2007-10-28 22:41                   ` Matthew Wilcox
2007-10-28 22:48                     ` Alan Cox
2007-10-28 22:55                       ` Matthew Wilcox
2007-10-28 23:38                         ` Alan Cox
2007-10-29  2:29                           ` J. Bruce Fields
2007-10-29  8:08                             ` Alan Cox
2007-10-29  9:15                             ` Jiri Kosina
2007-10-30 15:35                               ` J. Bruce Fields
2007-10-28 22:55                     ` Jiri Kosina
2007-10-28 23:31                       ` Matthew Wilcox
2007-10-29  9:11                         ` Jiri Kosina
2007-10-29  2:10                     ` J. Bruce Fields
2007-10-29  3:26                     ` Trond Myklebust
2007-10-29  1:13               ` J. Bruce Fields [this message]
2007-10-29  8:06           ` Alan Cox
2007-10-30 15:51             ` J. Bruce Fields
2007-10-30 15:20         ` [PATCH, RESEND] locks: fix possible infinite loop in " J. Bruce Fields
2007-10-30 15:35           ` Alan Cox

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=20071029011306.GA10307@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=gdavis@mvista.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew@wil.cx \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).