From: Pavel Emelyanov <xemul@openvz.org>
To: Trond Myklebust <trond.myklebust@fys.uio.no>
Cc: Andrew Morton <akpm@osdl.org>,
"J. Bruce Fields" <bfields@fieldses.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: Mon, 17 Sep 2007 18:16:50 +0400 [thread overview]
Message-ID: <46EE8C52.80503@openvz.org> (raw)
In-Reply-To: <1190037331.6700.14.camel@heimdal.trondhjem.org>
Trond Myklebust wrote:
> On Mon, 2007-09-17 at 12:13 +0400, Pavel Emelyanov wrote:
>> When the process is blocked on mandatory lock and someone changes
>> the inode's permissions, so that the lock is no longer mandatory,
>> nobody wakes up the blocked process, but probably should.
>
> Please explain in more detail why we need this patch.
>From "this fixes an OOPs/deadlock/leak" POV we do not. This is
just an attempt to make the locking code be more consistent and
clean.
On the other hand, as far as I see from the original comment
the code author expected the permissions to change and tried
to handle this case. In this particular code there's a BUG -
task will not be woken up. So this is a fix for this place. If
we don't want to see tasks blocked on no-longer-mandatory
locks, then we should remove the original check and comment.
> I don't see why changing a file from taking mandatory locks to advisory
> locks is really a useful operation that we need to support. For one
> thing, we don't support changing a file from using advisory locking to
> mandatory locking on-the-fly. Secondly, changing the locking type
Actually we have some code trying to do it - no mandatory locking
is allowed for already opened file.
> certainly isn't a documented operation and quite frankly, it doesn't
> even appear to make sense: if the file needs mandatory locking, then
> that means that you have a need to protect against some untrusted
> application that isn't following the locking rules. Then suddenly, you
> declare that you will trust that application after all???
Not trust, but keep things look as they are described in docs. I.e.
two conditions "inode is marked as mandlock" and "task cannot proceed
with opening mand.lock-ed file" must go in pair.
> Cheers,
> Trond
Thanks,
Pavel
next prev parent reply other threads:[~2007-09-17 14:19 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 [this message]
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
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=46EE8C52.80503@openvz.org \
--to=xemul@openvz.org \
--cc=akpm@osdl.org \
--cc=bfields@fieldses.org \
--cc=devel@openvz.org \
--cc=linux-kernel@vger.kernel.org \
--cc=trond.myklebust@fys.uio.no \
/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