lkml.org 
[lkml]   [2017]   [Jun]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 2/2] fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks
Date

On 19 Jun 2017, at 13:32, Jeff Layton wrote:

> On Mon, 2017-06-19 at 09:24 -0400, Benjamin Coddington wrote:
>> @@ -2041,16 +2034,46 @@ SYSCALL_DEFINE2(flock, unsigned int, fd,
>> unsigned int, cmd)
>> */
>> int vfs_test_lock(struct file *filp, struct file_lock *fl)
>> {
>> - if (filp->f_op->lock && is_remote_lock(filp))
>> + if (filp->f_op->lock && is_remote_lock(filp)) {
>> + fl->fl_flags |= FL_PID_PRIV;
>> return filp->f_op->lock(filp, F_GETLK, fl);
>> + }
>> posix_test_lock(filp, fl);
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(vfs_test_lock);
>>
>
> I think this looks wrong for NFS.

Oh yes, this is completely wrong.. It should be looking for fl_ops,
which
would set the flag for lock managers.

> There are really two cases we're concerned with here:
>
> 1) the lock is held by a task on the client itself, in which case we
> probably want to report the pid as we would on a local fs.
>
> ...or...
>
> 2) the lock is held by another host entirely in which case the pid
> doesn't have any meaning. We probably ought to return something like
> '-
> 1' as the pid (like we would for OFD locks).

I don't think we have f_op->lock() users that only set remote locks.
For
NFS, the remote lock is always matched by a local lock.

> The problem for NFS is that you're setting the flag unconditionally
> there. It may very well be the case that we _want_ to translate the
> fl_pid according to the local namespace (i.e. if the lock is held by a
> task on the same host).
>
> I think what you want to do here is have the fs ->lock operation set
> that flag if the fl_pid should be used "as-is" instead of being
> translated.
>
> Most of the current lock operations can just set it early (to preserve
> the existing behavior), but NFS could be set up to set that flag if
> the
> lock request goes to the server.

I think this is just a mistake.. I think we want to always translate all
local locks, unless the lock is placed by a lock manager.

I'll send a corrected version.

Ben

\
 
 \ /
  Last update: 2017-06-20 16:04    [W:1.317 / U:0.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site