posix_lock_file and blocking locks

posix_lock_file and blocking locks

[Steve French]

Can the posix_lock* calls work with the following case:

1) lock range1
2) lock succeeds
3) blocking lock on range1 - blocks waiting to get lock
4) unlock range1
5) unlock succeeds
   *** kernel generates panic
	Attempting to free lock with active block list
6) blocking lock succeeds

For network filesystems (e.g. cifs, nfs), should we be calling calling
something other than posix_lock_file perhaps calling
posix_lock_file_wait (what is this call for?).

Although before sending a byte range lock request to the server it would
be helpful to have local vfs helper calls to see if the lock would:
1) suceed (if we know enough from the local system's perspective to know
that the lock would fail - we might as well fail the request
immediately)
2) change the state - A second lock sometimes has no effect since it is
common practice in Unix (although apparently not required by POSIX) to
"merge" overlapping locks - if a second lock would have no effect on the
server (because it would be merged into an existing lock(s) which
completely overlaps it) - it would be nice to be able to thow those lock
requests away before sending them to the server

Any idea if this is possible with the current fs/locks.c exports?

Since the server already is keeping track of the locks for this inode
the only reason I can see for calling posix_lock_file on the client (as
was added in 2.6.9) would be to have the local client keep a list of the
current lock state so it can replay them if the session server crashes
(so the locks can be replayed when the server comes back up). 

Until I figure out a better way to store the local state of the locks -
I don't see a way out of removing the call to posix_lock_file that was
added to fs/cifs/file.c back in 2.6.9 (it causes the kernel panic in the
case described above).

Re: posix_lock_file and blocking locks

[Trond Myklebust]

[-- Attachment #1: Type: text/plain, Size: 957 bytes --]

on den 15.12.2004 Klokka 09:39 (-0600) skreiv Steve French:

> For network filesystems (e.g. cifs, nfs), should we be calling calling
> something other than posix_lock_file perhaps calling
> posix_lock_file_wait (what is this call for?).
> 

They may wall posix_lock_file() for F_UNLCK type calls (since those
never block), but for blocking locks, you probably should call
posix_lock_file_wait().

My guess is that what is happening here is something I've observed
already on NFS systems: 
	process 1 locks the file
		process 2 tries to lock, but blocks on process 1.
	process 1 calls the server that it should unlocks the file
		the server notifies process 2 that it now has the lock before process
1 receives its reply
		process 2 calls posix_lock_file(), which puts the lock on the blocking
list, then returns. Panic, when VFS tries to free that lock.

So does the following patch help?

Cheers,
  Trond

-- 
Trond Myklebust <trond.myklebust@fys.uio.no>

[-- Attachment #2: linux-2.6.10-fix_cifslock.dif --]
[-- Type: text/plain, Size: 682 bytes --]

 file.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.10-rc3/fs/cifs/file.c
===================================================================
--- linux-2.6.10-rc3.orig/fs/cifs/file.c	2004-12-13 17:45:34.000000000 -0500
+++ linux-2.6.10-rc3/fs/cifs/file.c	2004-12-15 16:18:21.514077956 -0500
@@ -597,9 +597,9 @@ cifs_lock(struct file *file, int cmd, st
 			 netfid, length,
 			 pfLock->fl_start, numUnlock, numLock, lockType,
 			 wait_flag);
-	if (rc == 0 && (pfLock->fl_flags & FL_POSIX))
-		posix_lock_file(file, pfLock);
 	FreeXid(xid);
+	if (rc == 0 && (pfLock->fl_flags & FL_POSIX))
+		posix_lock_file_wait(file, pfLock);
 	return rc;
 }
 

Re: posix_lock_file and blocking locks

[Steve French]

Trond Myklebust wrote:

>on den 15.12.2004 Klokka 09:39 (-0600) skreiv Steve French:
>
>  
>
>>For network filesystems (e.g. cifs, nfs), should we be calling calling
>>something other than posix_lock_file perhaps calling
>>posix_lock_file_wait (what is this call for?).
>>
>>    
>>
>
>They may wall posix_lock_file() for F_UNLCK type calls (since those
>never block), but for blocking locks, you probably should call
>posix_lock_file_wait().
>
>My guess is that what is happening here is something I've observed
>already on NFS systems: 
>	process 1 locks the file
>		process 2 tries to lock, but blocks on process 1.
>	process 1 calls the server that it should unlocks the file
>		the server notifies process 2 that it now has the lock before process
>1 receives its reply
>		process 2 calls posix_lock_file(), which puts the lock on the blocking
>list, then returns. Panic, when VFS tries to free that lock.
>
>So does the following patch help?
>
>Cheers,
>  Trond
>
>  
>
>------------------------------------------------------------------------
>
> file.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>Index: linux-2.6.10-rc3/fs/cifs/file.c
>===================================================================
>--- linux-2.6.10-rc3.orig/fs/cifs/file.c	2004-12-13 17:45:34.000000000 -0500
>+++ linux-2.6.10-rc3/fs/cifs/file.c	2004-12-15 16:18:21.514077956 -0500
>@@ -597,9 +597,9 @@ cifs_lock(struct file *file, int cmd, st
> 			 netfid, length,
> 			 pfLock->fl_start, numUnlock, numLock, lockType,
> 			 wait_flag);
>-	if (rc == 0 && (pfLock->fl_flags & FL_POSIX))
>-		posix_lock_file(file, pfLock);
> 	FreeXid(xid);
>+	if (rc == 0 && (pfLock->fl_flags & FL_POSIX))
>+		posix_lock_file_wait(file, pfLock);
> 	return rc;
> }
> 
>  
>
Yes - your suggested patch bypassed the kernel panic in fs/locks.c and 
the scenario you described as originally causing the nfs recreate is 
similar to what I see happening in cifs with "connectathon nfs" locktest 7.

I will merge this into the cifs bk tree.  This is probably worth pushing 
into mainline before 2.6.10 (if it is not already too late) as the 
kernel panic in fs/locks.c will affect some users.   I will do a bit 
more testing and send it.