All of lore.kernel.org
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Trond Myklebust <trond.myklebust@fys.uio.no>
Cc: Pavel Emelyanov <xemul@openvz.org>, Andrew Morton <akpm@osdl.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	devel@openvz.org
Subject: Re: [PATCH] Wake up mandatory locks waiter on chmod (v2)
Date: Tue, 18 Sep 2007 12:52:20 -0400	[thread overview]
Message-ID: <20070918165220.GE18476@fieldses.org> (raw)
In-Reply-To: <1190132095.6656.12.camel@heimdal.trondhjem.org>

On Tue, Sep 18, 2007 at 12:14:55PM -0400, Trond Myklebust wrote:
> Note also that strictly speaking, we're not even compliant with the
> System V behaviour on read() and write(). See:
> 
>   http://www.unix.org.ua/orelly/networking_2ndEd/nfs/ch11_01.htm
> and
>   http://docs.sun.com/app/docs/doc/801-6736/6i13fom0a?l=en&a=view&q=mandatory+lock
> 
> According to these docs, we should be wrapping each and every read() and
> write() syscall with a mandatory lock. The fact that we're not, and yet
> still not seeing any complaints just goes to show how few people are
> actually using and relying on this...

So currently there's nothing to prevent this:

				- write passes locks_mandatory_area() checks
	- get mandatory lock
	- read old data
				- write updates file data
	- read new data

You can see the data change even while you hold a mandatory lock that
should exclude writes.

Similarly you might think that an application could prevent anyone from
seeing the intermediate state of a file while it performs a series of
writes under an exclusive mandatory lock, but actually there's nothing
to stop a read in progress from racing with acquisition of the lock.

Unless I'm missing something, that makes our mandatory lock
implementation pretty pointless.  I wish we could either fix it or just
ditch it, but I suppose either option would be unpopular.

--b.

  reply	other threads:[~2007-09-18 16:52 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-17  8:13 [PATCH] Wake up mandatory locks waiter on chmod (v2) Pavel Emelyanov
2007-09-17 13:55 ` Trond Myklebust
2007-09-17 14:16   ` Pavel Emelyanov
2007-09-17 16:00     ` Trond Myklebust
2007-09-18  6:33       ` Pavel Emelyanov
2007-09-18 15:19         ` J. Bruce Fields
2007-09-18 16:14           ` Trond Myklebust
2007-09-18 16:52             ` J. Bruce Fields [this message]
2007-09-18 16:54               ` Trond Myklebust
2007-09-18 17:40                 ` J. Bruce Fields
2007-09-18 18:38                   ` Hugh Dickins
2007-09-25 16:55                   ` [PATCH 1/2] Documentation: move mandatory locking documentation to filesystems/ J. Bruce Fields
2007-09-25 16:56                     ` [PATCH 2/2] locks: add warning about mandatory locking races J. Bruce Fields
2007-09-25 17:12                     ` [PATCH 1/2] Documentation: move mandatory locking documentation to filesystems/ Randy Dunlap
2007-09-25 17:24                       ` J. Bruce Fields

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=20070918165220.GE18476@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=akpm@osdl.org \
    --cc=devel@openvz.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=trond.myklebust@fys.uio.no \
    --cc=xemul@openvz.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.