* [PATCH fs/locks 3 of 3] Fix race conditions with file lock vs close
@ 2005-07-11 10:32 Olaf Kirch
2005-07-11 12:32 ` Peter Staubach
0 siblings, 1 reply; 6+ messages in thread
From: Olaf Kirch @ 2005-07-11 10:32 UTC (permalink / raw)
To: nfs; +Cc: akpm
# Subject: Fix race conditions with file lock vs close
#
# When a multithreaded application tries to obtain a lock on a remote
# file server (NFS or other), while another thread closes the file, we
# risk oopsing or running into a BUG(). This can happen if the close
# happens inbetween sys_fcntl() grabbing the file pointer, and before
# the call to __posix_lock_file.
#
# The fix is to change fcntl_setlk* and make them check if the file
# is still open after the lock has been obtained.
#
# Signed-off-by: Olaf Kirch <okir@suse.de>
Index: linux-2.6.12/fs/fcntl.c
===================================================================
--- linux-2.6.12.orig/fs/fcntl.c 2005-07-08 11:45:17.000000000 +0200
+++ linux-2.6.12/fs/fcntl.c 2005-07-11 12:17:31.000000000 +0200
@@ -288,7 +288,7 @@ static long do_fcntl(int fd, unsigned in
break;
case F_SETLK:
case F_SETLKW:
- err = fcntl_setlk(filp, cmd, (struct flock __user *) arg);
+ err = fcntl_setlk(filp, cmd, (struct flock __user *) arg, fd);
break;
case F_GETOWN:
/*
@@ -376,7 +376,7 @@ asmlinkage long sys_fcntl64(unsigned int
break;
case F_SETLK64:
case F_SETLKW64:
- err = fcntl_setlk64(filp, cmd, (struct flock64 __user *) arg);
+ err = fcntl_setlk64(filp, cmd, (struct flock64 __user *) arg, fd);
break;
default:
err = do_fcntl(fd, cmd, arg, filp);
Index: linux-2.6.12/fs/locks.c
===================================================================
--- linux-2.6.12.orig/fs/locks.c 2005-07-11 12:17:15.000000000 +0200
+++ linux-2.6.12/fs/locks.c 2005-07-11 12:17:31.000000000 +0200
@@ -1591,7 +1591,7 @@ out:
/* Apply the lock described by l to an open file descriptor.
* This implements both the F_SETLK and F_SETLKW commands of fcntl().
*/
-int fcntl_setlk(struct file *filp, unsigned int cmd, struct flock __user *l)
+int fcntl_setlk(struct file *filp, unsigned int cmd, struct flock __user *l, int fd)
{
struct file_lock *file_lock = locks_alloc_lock();
struct flock flock;
@@ -1666,6 +1666,18 @@ int fcntl_setlk(struct file *filp, unsig
break;
}
+ /* Make sure the file is still open, otherwise we need
+ * to release the lock we just claimed. */
+ spin_lock(¤t->files->file_lock);
+ if (current->files->fd[fd] != filp) {
+ /* Darn - someone closed the file in the meanwhile. */
+ spin_unlock(¤t->files->file_lock);
+ locks_remove_posix(filp, current->files);
+ error = -EBADF;
+ } else {
+ spin_unlock(¤t->files->file_lock);
+ }
+
out:
locks_free_lock(file_lock);
return error;
@@ -1722,7 +1734,7 @@ out:
/* Apply the lock described by l to an open file descriptor.
* This implements both the F_SETLK and F_SETLKW commands of fcntl().
*/
-int fcntl_setlk64(struct file *filp, unsigned int cmd, struct flock64 __user *l)
+int fcntl_setlk64(struct file *filp, unsigned int cmd, struct flock64 __user *l, int fd)
{
struct file_lock *file_lock = locks_alloc_lock();
struct flock64 flock;
@@ -1797,6 +1809,18 @@ int fcntl_setlk64(struct file *filp, uns
break;
}
+ /* Make sure the file is still open, otherwise we need
+ * to release the lock we just claimed. */
+ spin_lock(¤t->files->file_lock);
+ if (current->files->fd[fd] != filp) {
+ /* Darn - someone closed the file in the meanwhile. */
+ spin_unlock(¤t->files->file_lock);
+ locks_remove_posix(filp, current->files);
+ error = -EBADF;
+ } else {
+ spin_unlock(¤t->files->file_lock);
+ }
+
out:
locks_free_lock(file_lock);
return error;
Index: linux-2.6.12/include/linux/fs.h
===================================================================
--- linux-2.6.12.orig/include/linux/fs.h 2005-07-08 11:45:21.000000000 +0200
+++ linux-2.6.12/include/linux/fs.h 2005-07-11 12:17:31.000000000 +0200
@@ -689,11 +689,11 @@ extern struct list_head file_lock_list;
#include <linux/fcntl.h>
extern int fcntl_getlk(struct file *, struct flock __user *);
-extern int fcntl_setlk(struct file *, unsigned int, struct flock __user *);
+extern int fcntl_setlk(struct file *, unsigned int, struct flock __user *, int);
#if BITS_PER_LONG == 32
extern int fcntl_getlk64(struct file *, struct flock64 __user *);
-extern int fcntl_setlk64(struct file *, unsigned int, struct flock64 __user *);
+extern int fcntl_setlk64(struct file *, unsigned int, struct flock64 __user *, int);
#endif
extern void send_sigio(struct fown_struct *fown, int fd, int band);
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
okir@suse.de | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax
-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA. To register visit http://www.hp.com/go/dualwebinar
_______________________________________________
NFS maillist - NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH fs/locks 3 of 3] Fix race conditions with file lock vs close
2005-07-11 10:32 [PATCH fs/locks 3 of 3] Fix race conditions with file lock vs close Olaf Kirch
@ 2005-07-11 12:32 ` Peter Staubach
2005-07-11 12:54 ` Olaf Kirch
0 siblings, 1 reply; 6+ messages in thread
From: Peter Staubach @ 2005-07-11 12:32 UTC (permalink / raw)
To: Olaf Kirch; +Cc: nfs, akpm
[-- Attachment #1: Type: text/plain, Size: 758 bytes --]
Olaf Kirch wrote:
># Subject: Fix race conditions with file lock vs close
>#
># When a multithreaded application tries to obtain a lock on a remote
># file server (NFS or other), while another thread closes the file, we
># risk oopsing or running into a BUG(). This can happen if the close
># happens inbetween sys_fcntl() grabbing the file pointer, and before
># the call to __posix_lock_file.
>#
># The fix is to change fcntl_setlk* and make them check if the file
># is still open after the lock has been obtained.
>#
># Signed-off-by: Olaf Kirch <okir@suse.de>
>
A different patch was submitted upstream already to address the fcntl/close
race. It was accepted into the '-mm' kernel on June 28'th. That patch is
attached.
Thanx...
ps
[-- Attachment #2: devel --]
[-- Type: text/plain, Size: 5491 bytes --]
--- ./fs/locks.c.org 2005-06-27 08:54:55.000000000 -0400
+++ ./fs/locks.c 2005-06-27 13:24:38.447157408 -0400
@@ -1589,7 +1589,8 @@ out:
/* Apply the lock described by l to an open file descriptor.
* This implements both the F_SETLK and F_SETLKW commands of fcntl().
*/
-int fcntl_setlk(struct file *filp, unsigned int cmd, struct flock __user *l)
+int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
+ struct flock __user *l)
{
struct file_lock *file_lock = locks_alloc_lock();
struct flock flock;
@@ -1618,6 +1619,7 @@ int fcntl_setlk(struct file *filp, unsig
goto out;
}
+again:
error = flock_to_posix_lock(filp, file_lock, &flock);
if (error)
goto out;
@@ -1646,25 +1648,34 @@ int fcntl_setlk(struct file *filp, unsig
if (error)
goto out;
- if (filp->f_op && filp->f_op->lock != NULL) {
+ if (filp->f_op && filp->f_op->lock != NULL)
error = filp->f_op->lock(filp, cmd, file_lock);
- goto out;
- }
-
- for (;;) {
- error = __posix_lock_file(inode, file_lock);
- if ((error != -EAGAIN) || (cmd == F_SETLK))
+ else {
+ for (;;) {
+ error = __posix_lock_file(inode, file_lock);
+ if ((error != -EAGAIN) || (cmd == F_SETLK))
+ break;
+ error = wait_event_interruptible(file_lock->fl_wait,
+ !file_lock->fl_next);
+ if (!error)
+ continue;
+
+ locks_delete_block(file_lock);
break;
- error = wait_event_interruptible(file_lock->fl_wait,
- !file_lock->fl_next);
- if (!error)
- continue;
+ }
+ }
- locks_delete_block(file_lock);
- break;
+ /*
+ * Attempt to detect a close/fcntl race and recover by
+ * releasing the lock that was just acquired.
+ */
+ if (!error &&
+ cmd != F_UNLCK && fcheck(fd) != filp && flock.l_type != F_UNLCK) {
+ flock.l_type = F_UNLCK;
+ goto again;
}
- out:
+out:
locks_free_lock(file_lock);
return error;
}
@@ -1722,7 +1733,8 @@ out:
/* Apply the lock described by l to an open file descriptor.
* This implements both the F_SETLK and F_SETLKW commands of fcntl().
*/
-int fcntl_setlk64(struct file *filp, unsigned int cmd, struct flock64 __user *l)
+int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd,
+ struct flock64 __user *l)
{
struct file_lock *file_lock = locks_alloc_lock();
struct flock64 flock;
@@ -1751,6 +1763,7 @@ int fcntl_setlk64(struct file *filp, uns
goto out;
}
+again:
error = flock64_to_posix_lock(filp, file_lock, &flock);
if (error)
goto out;
@@ -1779,22 +1792,31 @@ int fcntl_setlk64(struct file *filp, uns
if (error)
goto out;
- if (filp->f_op && filp->f_op->lock != NULL) {
+ if (filp->f_op && filp->f_op->lock != NULL)
error = filp->f_op->lock(filp, cmd, file_lock);
- goto out;
- }
-
- for (;;) {
- error = __posix_lock_file(inode, file_lock);
- if ((error != -EAGAIN) || (cmd == F_SETLK64))
+ else {
+ for (;;) {
+ error = __posix_lock_file(inode, file_lock);
+ if ((error != -EAGAIN) || (cmd == F_SETLK64))
+ break;
+ error = wait_event_interruptible(file_lock->fl_wait,
+ !file_lock->fl_next);
+ if (!error)
+ continue;
+
+ locks_delete_block(file_lock);
break;
- error = wait_event_interruptible(file_lock->fl_wait,
- !file_lock->fl_next);
- if (!error)
- continue;
+ }
+ }
- locks_delete_block(file_lock);
- break;
+ /*
+ * Attempt to detect a close/fcntl race and recover by
+ * releasing the lock that was just acquired.
+ */
+ if (!error &&
+ cmd != F_UNLCK && fcheck(fd) != filp && flock.l_type != F_UNLCK) {
+ flock.l_type = F_UNLCK;
+ goto again;
}
out:
@@ -1886,12 +1908,7 @@ void locks_remove_flock(struct file *fil
while ((fl = *before) != NULL) {
if (fl->fl_file == filp) {
- /*
- * We might have a POSIX lock that was created at the same time
- * the filp was closed for the last time. Just remove that too,
- * regardless of ownership, since nobody can own it.
- */
- if (IS_FLOCK(fl) || IS_POSIX(fl)) {
+ if (IS_FLOCK(fl)) {
locks_delete_lock(before);
continue;
}
--- ./fs/fcntl.c.org 2005-06-27 08:54:57.000000000 -0400
+++ ./fs/fcntl.c 2005-06-27 13:24:38.471153760 -0400
@@ -290,7 +290,7 @@ static long do_fcntl(int fd, unsigned in
break;
case F_SETLK:
case F_SETLKW:
- err = fcntl_setlk(filp, cmd, (struct flock __user *) arg);
+ err = fcntl_setlk(fd, filp, cmd, (struct flock __user *) arg);
break;
case F_GETOWN:
/*
@@ -378,7 +378,8 @@ asmlinkage long sys_fcntl64(unsigned int
break;
case F_SETLK64:
case F_SETLKW64:
- err = fcntl_setlk64(filp, cmd, (struct flock64 __user *) arg);
+ err = fcntl_setlk64(fd, filp, cmd,
+ (struct flock64 __user *) arg);
break;
default:
err = do_fcntl(fd, cmd, arg, filp);
--- ./include/linux/fs.h.org 2005-06-27 08:54:57.000000000 -0400
+++ ./include/linux/fs.h 2005-06-27 13:24:38.444157864 -0400
@@ -691,11 +691,13 @@ extern struct list_head file_lock_list;
#include <linux/fcntl.h>
extern int fcntl_getlk(struct file *, struct flock __user *);
-extern int fcntl_setlk(struct file *, unsigned int, struct flock __user *);
+extern int fcntl_setlk(unsigned int, struct file *, unsigned int,
+ struct flock __user *);
#if BITS_PER_LONG == 32
extern int fcntl_getlk64(struct file *, struct flock64 __user *);
-extern int fcntl_setlk64(struct file *, unsigned int, struct flock64 __user *);
+extern int fcntl_setlk64(unsigned int, struct file *, unsigned int,
+ struct flock64 __user *);
#endif
extern void send_sigio(struct fown_struct *fown, int fd, int band);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH fs/locks 3 of 3] Fix race conditions with file lock vs close
2005-07-11 12:32 ` Peter Staubach
@ 2005-07-11 12:54 ` Olaf Kirch
2005-07-11 13:04 ` Peter Staubach
2005-07-11 13:18 ` Trond Myklebust
0 siblings, 2 replies; 6+ messages in thread
From: Olaf Kirch @ 2005-07-11 12:54 UTC (permalink / raw)
To: Peter Staubach; +Cc: nfs, akpm
On Mon, Jul 11, 2005 at 08:32:05AM -0400, Peter Staubach wrote:
> A different patch was submitted upstream already to address the fcntl/close
> race. It was accepted into the '-mm' kernel on June 28'th. That patch is
> attached.
I see, it seems this customer has been spamming just about everyone
about this problem :)
I just wonder how the following can work - your patch seems to drop
local book-keeping of locks completely, unless I misread it:
> + if (filp->f_op && filp->f_op->lock != NULL)
> error = filp->f_op->lock(filp, cmd, file_lock);
> + else {
> + for (;;) {
> + error = __posix_lock_file(inode, file_lock);
> + if ((error != -EAGAIN) || (cmd == F_SETLK))
> + break;
> + error = wait_event_interruptible(file_lock->fl_wait,
> + !file_lock->fl_next);
> + if (!error)
> + continue;
> +
> + locks_delete_block(file_lock);
> break;
> + }
> + }
This means if the application establishes an NFS lock, this will not
be reflected locally - and as a consequence, when the application just
exits, the lock will never be released.
Olaf
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
okir@suse.de | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax
-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA. To register visit http://www.hp.com/go/dualwebinar
_______________________________________________
NFS maillist - NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH fs/locks 3 of 3] Fix race conditions with file lock vs close
2005-07-11 12:54 ` Olaf Kirch
@ 2005-07-11 13:04 ` Peter Staubach
2005-07-11 13:20 ` Olaf Kirch
2005-07-11 13:18 ` Trond Myklebust
1 sibling, 1 reply; 6+ messages in thread
From: Peter Staubach @ 2005-07-11 13:04 UTC (permalink / raw)
To: Olaf Kirch; +Cc: nfs, akpm
[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]
Olaf Kirch wrote:
>On Mon, Jul 11, 2005 at 08:32:05AM -0400, Peter Staubach wrote:
>
>
>>A different patch was submitted upstream already to address the fcntl/close
>>race. It was accepted into the '-mm' kernel on June 28'th. That patch is
>>attached.
>>
>>
>
>I see, it seems this customer has been spamming just about everyone
>about this problem :)
>
>I just wonder how the following can work - your patch seems to drop
>local book-keeping of locks completely, unless I misread it:
>
>
>
>
>>+ if (filp->f_op && filp->f_op->lock != NULL)
>> error = filp->f_op->lock(filp, cmd, file_lock);
>>+ else {
>>+ for (;;) {
>>+ error = __posix_lock_file(inode, file_lock);
>>+ if ((error != -EAGAIN) || (cmd == F_SETLK))
>>+ break;
>>+ error = wait_event_interruptible(file_lock->fl_wait,
>>+ !file_lock->fl_next);
>>+ if (!error)
>>+ continue;
>>+
>>+ locks_delete_block(file_lock);
>> break;
>>+ }
>>+ }
>>
>>
>
>This means if the application establishes an NFS lock, this will not
>be reflected locally - and as a consequence, when the application just
>exits, the lock will never be released.
>
I didn't change the way that the code works in this area, I just changed the
indentation. As far as I can tell, the new code works the same way as the
old code did, except that it also doesn't leave dangling locks
around... :-)
I wrote a small program to reproduce the race that I thought that the 2.6
code was not already handling. It is attached. (Please note that it needs
to run with a large-ish delay value, something like 40 or 45 seconds. There
is some interaction with the Linux threading which I haven't had/made time
to investigate yet.) I can also attach the email that I sent upstream
which describes my analysis, if you like.
Thanx...
ps
[-- Attachment #2: repo.c --]
[-- Type: text/x-csrc, Size: 3404 bytes --]
#
#include <sys/types.h>
#include <stdio.h>
#include <pthread.h>
#include <signal.h>
#include <fcntl.h>
#include <errno.h>
#define SLEEPTIME 2
int delay = SLEEPTIME;
void do_child_process(int fd);
void *do_child_thread(void *arg);
main(int argc, char *argv[])
{
int fd;
int pid;
int error;
char *fname;
pthread_t tid;
if (argc == 1 || argc > 3) {
fprintf(stderr, "usage: %s [delay] testfile_name\n", argv[0]);
exit(1);
}
if (argc == 2)
fname = argv[1];
else {
delay = atoi(argv[1]);
fname = argv[2];
}
fd = open(fname, O_CREAT | O_RDWR, 0644);
if (fd < 0) {
fprintf(stderr, "%s: can't create %s\n", argv[0], fname);
exit(1);
}
pid = fork();
if (pid < 0) {
perror("fork");
exit(1);
}
if (pid == 0)
do_child_process(fd);
/*
* Give the child process more than enough time to lock
* the file.
*/
sleep(delay);
error = pthread_create(&tid, NULL, do_child_thread, (void *)&fd);
if (error < 0) {
perror("pthread_create");
(void) kill(pid, SIGTERM);
exit(1);
}
/*
* Give the child thread long enough to start the lock and block
* waiting for the child process.
*/
sleep(delay);
if (close(fd) < 0) {
perror("parent close");
(void) kill(pid, SIGTERM);
exit(1);
}
if (pthread_join(tid, NULL) < 0)
perror("pthread_join");
if (wait(NULL) < 0)
perror("wait");
exit(0);
}
void
do_child_process(int fd)
{
struct flock lock;
/*
* Lock the entire file to block the parent locking.
* Once the lock is acquired, wait until the parent
* process is set, with a lock pending and the file
* closed, and then unlock the file.
*/
lock.l_type = F_WRLCK;
lock.l_whence = SEEK_SET;
lock.l_start = 0;
lock.l_len = 0;
if (fcntl(fd, F_SETLKW, &lock) < 0) {
perror("child process wait lock fcntl");
exit(1);
}
/*
* Wait long enough for the parent process to spawn a thread to
* block waiting to lock the file that this process has locked
* and to close the file descriptor.
*/
sleep(3 * delay);
/*
* Now unlock the locked region.
*/
lock.l_type = F_UNLCK;
if (fcntl(fd, F_SETLKW, &lock) < 0) {
perror("child process wait unlock fcntl");
exit(1);
}
/*
* Wait long enough for the parent process to to complete
* locking the file. The file should not be locked because
* the parent process also closed the file descriptor.
*/
sleep(delay);
/*
* Try to lock the file again. This is a non-blocking request
* and should succeed immediately if everything is working
* correctly. If it fails with EAGAIN, then the lock did not
* get properly removed after all of the parent process races.
*/
lock.l_type = F_WRLCK;
if (fcntl(fd, F_SETLK, &lock) < 0) {
if (errno == EAGAIN)
printf("Dangling lock detected\n");
else {
printf(
"Oops, indeterminate results, try a larger delay?\n");
}
} else {
printf("No dangling lock detected\n");
lock.l_type = F_UNLCK;
(void) fcntl(fd, F_SETLK, &lock);
}
exit(0);
}
void *
do_child_thread(void *arg)
{
int fd = *((int *)arg);
struct flock lock;
/*
* Lock the entire file. This should block, waiting for the
* child process to release its lock. Once it does, then this
* lock request should proceed and succeed.
*/
lock.l_type = F_WRLCK;
lock.l_whence = SEEK_SET;
lock.l_start = 0;
lock.l_len = 0;
if (fcntl(fd, F_SETLKW, &lock) < 0)
perror("child thread lock fcntl");
pthread_exit(NULL);
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH fs/locks 3 of 3] Fix race conditions with file lock vs close
2005-07-11 12:54 ` Olaf Kirch
2005-07-11 13:04 ` Peter Staubach
@ 2005-07-11 13:18 ` Trond Myklebust
1 sibling, 0 replies; 6+ messages in thread
From: Trond Myklebust @ 2005-07-11 13:18 UTC (permalink / raw)
To: Olaf Kirch; +Cc: Peter Staubach, nfs, akpm
m=C3=A5 den 11.07.2005 Klokka 14:54 (+0200) skreiv Olaf Kirch:
> I just wonder how the following can work - your patch seems to drop
> local book-keeping of locks completely, unless I misread it:
>=20
>=20
> > + if (filp->f_op && filp->f_op->lock !=3D NULL)
> > error =3D filp->f_op->lock(filp, cmd, file_lock);
> > + else {
> > + for (;;) {
> > + error =3D __posix_lock_file(inode, file_lock);
> > + if ((error !=3D -EAGAIN) || (cmd =3D=3D F_SETLK))
> > + break;
> > + error =3D wait_event_interruptible(file_lock->fl_wait,
> > + !file_lock->fl_next);
> > + if (!error)
> > + continue;
> > +=09
> > + locks_delete_block(file_lock);
> > break;
> > + }
> > + }
>=20
> This means if the application establishes an NFS lock, this will not
> be reflected locally - and as a consequence, when the application just
> exits, the lock will never be released.
In recent kernels, a filesystem that overrides the ->lock() method
should provide its own lock bookkeeping. Usually that means calling
posix_lock_file but it doesn't have to.
Cheers,
Trond
-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA. To register visit http://www.hp.com/go/dualwebinar
_______________________________________________
NFS maillist - NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH fs/locks 3 of 3] Fix race conditions with file lock vs close
2005-07-11 13:04 ` Peter Staubach
@ 2005-07-11 13:20 ` Olaf Kirch
0 siblings, 0 replies; 6+ messages in thread
From: Olaf Kirch @ 2005-07-11 13:20 UTC (permalink / raw)
To: Peter Staubach; +Cc: nfs, akpm
[-- Attachment #1: Type: text/plain, Size: 1038 bytes --]
On Mon, Jul 11, 2005 at 09:04:01AM -0400, Peter Staubach wrote:
> I didn't change the way that the code works in this area, I just changed the
> indentation. As far as I can tell, the new code works the same way as the
> old code did, except that it also doesn't leave dangling locks
> around... :-)
Doh, you're right. Time for a break :)
> I wrote a small program to reproduce the race that I thought that the 2.6
> code was not already handling. It is attached. (Please note that it needs
> to run with a large-ish delay value, something like 40 or 45 seconds. There
> is some interaction with the Linux threading which I haven't had/made time
> to investigate yet.) I can also attach the email that I sent upstream
> which describes my analysis, if you like.
Thanks, I'll give it a try.
While we're on the subject of swapping test cases, here's the one I've
been using.
Cheers
Olaf
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
okir@suse.de | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax
[-- Attachment #2: fusi-lock-crasher.c --]
[-- Type: text/plain, Size: 2792 bytes --]
/* butchered test case for lock/close problems */
#include <stdio.h>
#include <stdlib.h>
#include <signal.h>
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <sys/param.h>
#include <fcntl.h>
#include <pthread.h>
#include <string.h>
#define MAXLOOPS 10000000
int fd, i = 0;
int failures = 0;
int obtained = 0;
int running = 1;
void
alrm_hndl(int sig)
{
printf ("Timeout\n");
}
int
get_lock (int fd)
{
struct flock lock_it;
int res = 0;
#if 0
lock_it.l_type = F_WRLCK;
lock_it.l_whence = SEEK_SET;
lock_it.l_start = 0;
lock_it.l_len = 0;
if (fcntl (fd, F_GETLK, &lock_it) < 0) {
if (errno != EBADF)
perror("fcntl(GETLK)");
return (-1);
}
if (lock_it.l_type != F_UNLCK)
printf("File is locked!\n");
#endif
lock_it.l_type = F_WRLCK;
lock_it.l_whence = SEEK_SET;
lock_it.l_start = 0;
lock_it.l_len = 0;
if (fcntl (fd, F_SETLK, &lock_it) < 0) {
if (errno == EINTR) {
printf("Timed out.\n");
return 0;
}
if (errno != EBADF && errno != EAGAIN)
perror ("fcntl(F_WRLCK)");
failures++;
res = -1;
} else {
obtained++;
}
return res;
}
int
release_lock (int fd)
{
struct flock lock_it;
lock_it.l_type = F_UNLCK;
lock_it.l_whence = SEEK_SET;
lock_it.l_start = 0;
lock_it.l_len = 0;
if (fcntl (fd, F_SETLKW, &lock_it) < 0) {
if (errno == EINTR)
return 0;
if (errno != EBADF)
perror ("fcntl(F_UNLCK)");
return -1;
}
return 0;
}
void *
loop(void *arg)
{
printf("Locking "); fflush(stdout);
while (running) {
if (get_lock(fd) >= 0) {
write(1, ".", 1);
/* usleep(1); */
if (release_lock(fd) < 0) {
/* printf("Unlock failed!\n"); */
}
}
}
printf("\nLock thread done, %d successful calls, %d failures\n",
obtained, failures);
return (NULL);
}
void *
c_loop(void *arg)
{
char* fn = (char*) arg;
while (running) {
close(fd);
if ((fd = open (fn, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR)) < 0)
perror ("open");
//usleep(1);
i++;
}
printf("Open thread done\n");
return NULL;
}
int
main(int argc, char** argv)
{
#define THREADS 1
struct sigaction act;
int i;
pthread_t thread[THREADS];
pthread_t close_thread;
if(argc != 2) {
fprintf(stderr, "%s <path>\n", argv[0]);
exit(-1);
}
memset(&act, 0, sizeof(act));
act.sa_handler = alrm_hndl;
if (sigaction (SIGALRM, &act, NULL) < 0) {
perror("sigaction");
return 1;
}
if ((fd = open (argv[1], O_RDWR | O_CREAT, S_IRUSR | S_IWUSR)) < 0) {
perror ("open");
return 1;
}
for (i = 0; i < THREADS; i++) {
pthread_create(&thread[i], NULL, loop, (void*) NULL);
}
usleep(100000);
pthread_create(&close_thread, NULL, c_loop, (void*) argv[1]);
sleep(5);
running = 0;
for (i = 0; i < THREADS; i++)
pthread_join(thread[i], NULL);
return 0;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2005-07-11 13:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-07-11 10:32 [PATCH fs/locks 3 of 3] Fix race conditions with file lock vs close Olaf Kirch
2005-07-11 12:32 ` Peter Staubach
2005-07-11 12:54 ` Olaf Kirch
2005-07-11 13:04 ` Peter Staubach
2005-07-11 13:20 ` Olaf Kirch
2005-07-11 13:18 ` Trond Myklebust
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.