Re: [PATCH] Wake up mandatory locks waiter on chmod (v2)

[Pavel Emelyanov <xemul@openvz.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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/