linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Ivan Zuboff <anotherdiskmag@gmail.com>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: Fwd: Bug: lockf returns false-positive EDEADLK in multiprocess multithreaded environment
Date: Mon, 31 Jan 2022 06:21:35 -0500	[thread overview]
Message-ID: <ad60a99826063822d4a9fbe12ebb20f285a20410.camel@kernel.org> (raw)
In-Reply-To: <CAL-cVeiHF3+1bq9+RLsdZU-kzfMNYxD0CJBGVeKOrrEpBAyt4Q@mail.gmail.com>

On Mon, 2022-01-31 at 12:37 +0300, Ivan Zuboff wrote:
> Hello, Jeff!
> 
> Several weeks ago I mailed linux-fsdevel about some weird behavior
> I've found. To me, it looks like a bug. Unfortunately, I've got no
> response, so I decided to forward this message to you directly.
> 
> Sorry for the interruption and for my bad English -- it's not my
> native language.
> 
> Hope to hear your opinion on this!
> 
> Best regards,
> Ivan
> 

Sorry I missed your message. Re-cc'ing linux-fsdevel, so others can join
in on the discussion:

> ---------- Forwarded message ---------
> From: Ivan Zuboff <anotherdiskmag@gmail.com>
> Date: Mon, Jan 10, 2022 at 1:46 PM
> Subject: Bug: lockf returns false-positive EDEADLK in multiprocess
> multithreaded environment
> To: <linux-fsdevel@vger.kernel.org>
> 
> 
> As an application-level developer, I found a counter-intuitive
> behavior in lockf function provided by glibc and Linux kernel that is
> likely a bug.
> 
> In glibc, lockf function is implemented on top of fcntl system call:
> https://github.com/lattera/glibc/blob/master/io/lockf.c
> man page says that lockf can sometimes detect deadlock:
> http://manpages.ubuntu.com/manpages/xenial/man3/lockf.3.html
> Same with fcntl(F_SETLKW), on top of which lockf is implemented:
> http://manpages.ubuntu.com/manpages/hirsute/en/man3/fcntl.3posix.html
> 
> Deadlock detection algorithm in the Linux kernel
> (https://github.com/torvalds/linux/blob/master/fs/locks.c) seems buggy
> because it can easily give false positives. Suppose we have two
> processes A and B, process A has threads 1 and 2, process B has
> threads 3 and 4. When this processes execute concurrently, following
> sequence of actions is possible:
> 1. processA thread1 gets lockI
> 2. processB thread2 gets lockII
> 3. processA thread3 tries to get lockII, starts to wait
> 4. processB thread4 tries to get lockI, kernel detects deadlock,
> EDEADLK is returned from lockf function
> 
> Steps to reproduce this scenario (see attached file):
> 1. gcc -o edeadlk ./edeadlk.c -lpthread
> 2. Launch "./edeadlk a b" in the first terminal window.
> 3. Launch "./edeadlk a b" in the second terminal window.
> 
> What I expected to happen: two instances of the program are steadily working.
> 
> What happened instead:
> Assertion failed: (lockf(fd, 1, 1)) != -1 file: ./edeadlk.c, line:25,
> errno:35 . Error:: Resource deadlock avoided
> Aborted (core dumped)
> 
> Surely, this behavior is kind of "right". lockf file locks belongs to
> process, so on the process level it seems that deadlock is just about
> to happen: process A holds lockI and waits for lockII, process B holds
> lockII and is going to wait for lockI. However, the algorithm in the
> kernel doesn't take threads into account. In fact, a deadlock is not
> going to happen here if the thread scheduler will give control to some
> thread holding a lock.
> 
> I think there's a problem with the deadlock detection algorithm
> because it's overly pessimistic, which in turn creates problems --
> lockf errors in applications. I had to patch my application to use
> flock instead because flock doesn't have this overly-pessimistic
> behavior.
> 
> 

The POSIX locking API predates the concept of threading, and so it was
written with some unfortunate concepts around processes. Because you're
doing all of your lock acquisition from different threads, obviously
nothing should deadlock, but all of the locks are owned by the process
so the deadlock detection algorithm can't tell that.

If you have need to do something like this, then you may want to
consider using OFD locks, which were designed to allow proper file
locking in threaded programs. Here's an older article that predates the
name, but it gives a good overview:

    https://lwn.net/Articles/586904/

-- 
Jeff Layton <jlayton@kernel.org>

  parent reply	other threads:[~2022-01-31 11:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-10 10:46 Bug: lockf returns false-positive EDEADLK in multiprocess multithreaded environment Ivan Zuboff
     [not found] ` <CAL-cVeiHF3+1bq9+RLsdZU-kzfMNYxD0CJBGVeKOrrEpBAyt4Q@mail.gmail.com>
2022-01-31 11:21   ` Jeff Layton [this message]
2022-01-31 12:06     ` Fwd: " Ivan Zuboff
2022-01-31 12:45       ` Jeff Layton
2022-01-31 13:10         ` Ivan Zuboff

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=ad60a99826063822d4a9fbe12ebb20f285a20410.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=anotherdiskmag@gmail.com \
    --cc=linux-fsdevel@vger.kernel.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).