lkml.org 
[lkml]   [2002]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] sleeping file locks

Whole pile of changes mixed in together here:

- Add FL_SLEEP flag to indicate we intend to sleep and therefore desire
to be placed on the block list. Use it for POSIX & flock locks.
- Remove locks_block_on.
- Change posix_unblock_lock to eliminate a race that will appear once we
don't use the BKL any more.
- Update the comment for locks_same_owner() and rename it to
posix_same_owner().
- Change locks_mandatory_area() to allocate its lock on the stack and
call posix_lock_file() instead of repeating that logic.
- Rename the "caller" parameter to posix_lock_file() to "request"
to better show that this is not to be inserted directly.
- Redo some of the proc code a little. Stop exposing kernel addresses
to userspace (whoever thought _that_ was a good idea?!) and show how
we should be printing the device name. The last part is ifdeffed
out to avoid breaking lslk.
- Remove FL_BROKEN. And there was much rejoicing.

diff -urpNX dontdiff linux-2.5.33/fs/lockd/svclock.c linux-2.5.33-willy/fs/lockd/svclock.c
--- linux-2.5.33/fs/lockd/svclock.c 2002-08-28 06:42:34.000000000 -0700
+++ linux-2.5.33-willy/fs/lockd/svclock.c 2002-09-07 22:07:43.000000000 -0700
@@ -237,17 +237,8 @@ nlmsvc_delete_block(struct nlm_block *bl

/* Remove block from list */
nlmsvc_remove_block(block);
-
- /* If granted, unlock it, else remove from inode block list */
- if (unlock && block->b_granted) {
- dprintk("lockd: deleting granted lock\n");
- fl->fl_type = F_UNLCK;
- posix_lock_file(&block->b_file->f_file, fl, 0);
- block->b_granted = 0;
- } else {
- dprintk("lockd: unblocking blocked lock\n");
- posix_unblock_lock(fl);
- }
+ posix_unblock_lock(&file->f_file, fl);
+ block->b_granted = 0;

/* If the block is in the middle of a GRANT callback,
* don't kill it yet. */
@@ -324,7 +315,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, stru

again:
if (!(conflock = posix_test_lock(&file->f_file, &lock->fl))) {
- error = posix_lock_file(&file->f_file, &lock->fl, 0);
+ error = posix_lock_file(&file->f_file, &lock->fl);

if (block)
nlmsvc_delete_block(block, 0);
@@ -428,7 +419,7 @@ nlmsvc_unlock(struct nlm_file *file, str
nlmsvc_cancel_blocked(file, lock);

lock->fl.fl_type = F_UNLCK;
- error = posix_lock_file(&file->f_file, &lock->fl, 0);
+ error = posix_lock_file(&file->f_file, &lock->fl);

return (error < 0)? nlm_lck_denied_nolocks : nlm_granted;
}
@@ -532,7 +523,7 @@ nlmsvc_grant_blocked(struct nlm_block *b
* following yields an error, this is most probably due to low
* memory. Retry the lock in a few seconds.
*/
- if ((error = posix_lock_file(&file->f_file, &lock->fl, 0)) < 0) {
+ if ((error = posix_lock_file(&file->f_file, &lock->fl)) < 0) {
printk(KERN_WARNING "lockd: unexpected error %d in %s!\n",
-error, __FUNCTION__);
nlmsvc_insert_block(block, 10 * HZ);
diff -urpNX dontdiff linux-2.5.33/fs/lockd/svcsubs.c linux-2.5.33-willy/fs/lockd/svcsubs.c
--- linux-2.5.33/fs/lockd/svcsubs.c 2002-08-01 14:16:06.000000000 -0700
+++ linux-2.5.33-willy/fs/lockd/svcsubs.c 2002-09-07 22:07:43.000000000 -0700
@@ -176,7 +176,7 @@ again:
lock.fl_type = F_UNLCK;
lock.fl_start = 0;
lock.fl_end = OFFSET_MAX;
- if (posix_lock_file(&file->f_file, &lock, 0) < 0) {
+ if (posix_lock_file(&file->f_file, &lock) < 0) {
printk("lockd: unlock failure in %s:%d\n",
__FILE__, __LINE__);
return 1;
diff -urpNX dontdiff linux-2.5.33/fs/locks.c linux-2.5.33-willy/fs/locks.c
--- linux-2.5.33/fs/locks.c 2002-09-02 19:24:07.000000000 -0700
+++ linux-2.5.33-willy/fs/locks.c 2002-09-07 22:16:36.000000000 -0700
@@ -404,15 +412,14 @@ static inline int locks_overlap(struct f
}

/*
- * Check whether two locks have the same owner
- * N.B. Do we need the test on PID as well as owner?
- * (Clone tasks should be considered as one "owner".)
+ * Check whether two locks have the same owner. The apparently superfluous
+ * check for fl_pid enables us to distinguish between locks set by lockd.
*/
static inline int
-locks_same_owner(struct file_lock *fl1, struct file_lock *fl2)
+posix_same_owner(struct file_lock *fl1, struct file_lock *fl2)
{
return (fl1->fl_owner == fl2->fl_owner) &&
- (fl1->fl_pid == fl2->fl_pid);
+ (fl1->fl_pid == fl2->fl_pid);
}

/* Remove waiter from blocker's block list.
@@ -531,7 +536,7 @@ static int posix_locks_conflict(struct f
/* POSIX locks owned by the same process do not conflict with
* each other.
*/
- if (!IS_POSIX(sys_fl) || locks_same_owner(caller_fl, sys_fl))
+ if (!IS_POSIX(sys_fl) || posix_same_owner(caller_fl, sys_fl))
return (0);

/* Check whether they overlap */
@@ -575,15 +580,6 @@ static int interruptible_sleep_on_locked
return result;
}

-static int locks_block_on(struct file_lock *blocker, struct file_lock *waiter)
-{
- int result;
- locks_insert_block(blocker, waiter);
- result = interruptible_sleep_on_locked(&waiter->fl_wait, 0);
- locks_delete_block(waiter);
- return result;
-}
-
static int locks_block_on_timeout(struct file_lock *blocker, struct file_lock *waiter, int time)
{
int result;
@@ -675,56 +671,37 @@ int locks_mandatory_area(int read_write,
struct file *filp, loff_t offset,
size_t count)
{
- struct file_lock *fl;
- struct file_lock *new_fl = locks_alloc_lock(0);
+ struct file_lock fl;
int error;

- if (new_fl == NULL)
- return -ENOMEM;
-
- new_fl->fl_owner = current->files;
- new_fl->fl_pid = current->pid;
- new_fl->fl_file = filp;
- new_fl->fl_flags = FL_POSIX | FL_ACCESS;
- new_fl->fl_type = (read_write == FLOCK_VERIFY_WRITE) ? F_WRLCK : F_RDLCK;
- new_fl->fl_start = offset;
- new_fl->fl_end = offset + count - 1;
-
- error = 0;
- lock_kernel();
-
-repeat:
- /* Search the lock list for this inode for locks that conflict with
- * the proposed read/write.
- */
- for (fl = inode->i_flock; fl != NULL; fl = fl->fl_next) {
- if (!IS_POSIX(fl))
- continue;
- if (fl->fl_start > new_fl->fl_end)
+ fl.fl_owner = current->files;
+ fl.fl_pid = current->pid;
+ fl.fl_file = filp;
+ fl.fl_flags = FL_POSIX | FL_ACCESS | FL_SLEEP;
+ fl.fl_type = (read_write == FLOCK_VERIFY_WRITE) ? F_WRLCK : F_RDLCK;
+ fl.fl_start = offset;
+ fl.fl_end = offset + count - 1;
+
+ for (;;) {
+ error = posix_lock_file(filp, &fl);
+ if (error != -EAGAIN)
break;
- if (posix_locks_conflict(new_fl, fl)) {
- error = -EAGAIN;
- if (filp && (filp->f_flags & O_NONBLOCK))
- break;
- error = -EDEADLK;
- if (posix_locks_deadlock(new_fl, fl))
- break;
-
- error = locks_block_on(fl, new_fl);
- if (error != 0)
- break;
-
+ error = wait_event_interruptible(fl.fl_wait, !fl.fl_next);
+ if (!error) {
/*
* If we've been sleeping someone might have
* changed the permissions behind our back.
*/
- if ((inode->i_mode & (S_ISGID | S_IXGRP)) != S_ISGID)
- break;
- goto repeat;
+ if ((inode->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID)
+ continue;
}
+
+ lock_kernel();
+ locks_delete_block(&fl);
+ unlock_kernel();
+ break;
}
- locks_free_lock(new_fl);
- unlock_kernel();
+
return error;
}

@@ -732,8 +709,7 @@ repeat:
* at the head of the list, but that's secret knowledge known only to
* flock_lock_file and posix_lock_file.
*/
-static int flock_lock_file(struct file *filp, struct file_lock *new_fl,
- unsigned int wait)
+static int flock_lock_file(struct file *filp, struct file_lock *new_fl)
{
struct file_lock **before;
struct inode * inode = filp->f_dentry->d_inode;
@@ -764,7 +740,6 @@ static int flock_lock_file(struct file *
return 0;

lock_kernel();
-repeat:
for_each_lock(inode, before) {
struct file_lock *fl = *before;
if (IS_POSIX(fl))
@@ -774,12 +749,10 @@ repeat:
if (!flock_locks_conflict(new_fl, fl))
continue;
error = -EAGAIN;
- if (!wait)
- goto out;
- error = locks_block_on(fl, new_fl);
- if (error != 0)
- goto out;
- goto repeat;
+ if (new_fl->fl_flags & FL_SLEEP) {
+ locks_insert_block(fl, new_fl);
+ }
+ goto out;
}
locks_insert_lock(&inode->i_flock, new_fl);
error = 0;
@@ -807,8 +780,7 @@ out:
* To all purists: Yes, I use a few goto's. Just pass on to the next function.
*/

-int posix_lock_file(struct file *filp, struct file_lock *caller,
- unsigned int wait)
+int posix_lock_file(struct file *filp, struct file_lock *request)
{
struct file_lock *fl;
struct file_lock *new_fl, *new_fl2;
@@ -829,27 +801,29 @@ int posix_lock_file(struct file *filp, s
goto out_nolock;

lock_kernel();
- if (caller->fl_type != F_UNLCK) {
- repeat:
- for (fl = inode->i_flock; fl != NULL; fl = fl->fl_next) {
+ if (request->fl_type != F_UNLCK) {
+ for_each_lock(inode, before) {
+ struct file_lock *fl = *before;
if (!IS_POSIX(fl))
continue;
- if (!posix_locks_conflict(caller, fl))
+ if (!posix_locks_conflict(request, fl))
continue;
error = -EAGAIN;
- if (!wait)
+ if (!(request->fl_flags & FL_SLEEP))
goto out;
error = -EDEADLK;
- if (posix_locks_deadlock(caller, fl))
+ if (posix_locks_deadlock(request, fl))
goto out;
-
- error = locks_block_on(fl, caller);
- if (error != 0)
- goto out;
- goto repeat;
+ error = -EAGAIN;
+ locks_insert_block(fl, request);
+ goto out;
}
}

+ /* If we're just looking for a conflict, we're done. */
+ if (request->fl_flags & FL_ACCESS)
+ goto out;
+
/*
* We've allocated the new locks in advance, so there are no
* errors possible (and no blocking operations) from here on.
@@ -859,25 +833,23 @@ int posix_lock_file(struct file *filp, s

before = &inode->i_flock;

- /* First skip locks owned by other processes.
- */
+ /* First skip locks owned by other processes. */
while ((fl = *before) && (!IS_POSIX(fl) ||
- !locks_same_owner(caller, fl))) {
+ !posix_same_owner(request, fl))) {
before = &fl->fl_next;
}

- /* Process locks with this owner.
- */
- while ((fl = *before) && locks_same_owner(caller, fl)) {
+ /* Process locks with this owner. */
+ while ((fl = *before) && posix_same_owner(request, fl)) {
/* Detect adjacent or overlapping regions (if same lock type)
*/
- if (caller->fl_type == fl->fl_type) {
- if (fl->fl_end < caller->fl_start - 1)
+ if (request->fl_type == fl->fl_type) {
+ if (fl->fl_end < request->fl_start - 1)
goto next_lock;
/* If the next lock in the list has entirely bigger
* addresses than the new one, insert the lock here.
*/
- if (fl->fl_start > caller->fl_end + 1)
+ if (fl->fl_start > request->fl_end + 1)
break;

/* If we come here, the new and old lock are of the
@@ -885,41 +857,41 @@ int posix_lock_file(struct file *filp, s
* lock yielding from the lower start address of both
* locks to the higher end address.
*/
- if (fl->fl_start > caller->fl_start)
- fl->fl_start = caller->fl_start;
+ if (fl->fl_start > request->fl_start)
+ fl->fl_start = request->fl_start;
else
- caller->fl_start = fl->fl_start;
- if (fl->fl_end < caller->fl_end)
- fl->fl_end = caller->fl_end;
+ request->fl_start = fl->fl_start;
+ if (fl->fl_end < request->fl_end)
+ fl->fl_end = request->fl_end;
else
- caller->fl_end = fl->fl_end;
+ request->fl_end = fl->fl_end;
if (added) {
locks_delete_lock(before);
continue;
}
- caller = fl;
+ request = fl;
added = 1;
}
else {
/* Processing for different lock types is a bit
* more complex.
*/
- if (fl->fl_end < caller->fl_start)
+ if (fl->fl_end < request->fl_start)
goto next_lock;
- if (fl->fl_start > caller->fl_end)
+ if (fl->fl_start > request->fl_end)
break;
- if (caller->fl_type == F_UNLCK)
+ if (request->fl_type == F_UNLCK)
added = 1;
- if (fl->fl_start < caller->fl_start)
+ if (fl->fl_start < request->fl_start)
left = fl;
/* If the next lock in the list has a higher end
* address than the new one, insert the new one here.
*/
- if (fl->fl_end > caller->fl_end) {
+ if (fl->fl_end > request->fl_end) {
right = fl;
break;
}
- if (fl->fl_start >= caller->fl_start) {
+ if (fl->fl_start >= request->fl_start) {
/* The new lock completely replaces an old
* one (This may happen several times).
*/
@@ -933,11 +905,11 @@ int posix_lock_file(struct file *filp, s
* their needs.
*/
locks_wake_up_blocks(fl);
- fl->fl_start = caller->fl_start;
- fl->fl_end = caller->fl_end;
- fl->fl_type = caller->fl_type;
- fl->fl_u = caller->fl_u;
- caller = fl;
+ fl->fl_start = request->fl_start;
+ fl->fl_end = request->fl_end;
+ fl->fl_type = request->fl_type;
+ fl->fl_u = request->fl_u;
+ request = fl;
added = 1;
}
}
@@ -949,9 +921,9 @@ int posix_lock_file(struct file *filp, s

error = 0;
if (!added) {
- if (caller->fl_type == F_UNLCK)
+ if (request->fl_type == F_UNLCK)
goto out;
- locks_copy_lock(new_fl, caller);
+ locks_copy_lock(new_fl, request);
locks_insert_lock(before, new_fl);
new_fl = NULL;
}
@@ -965,11 +937,11 @@ int posix_lock_file(struct file *filp, s
locks_copy_lock(left, right);
locks_insert_lock(before, left);
}
- right->fl_start = caller->fl_end + 1;
+ right->fl_start = request->fl_end + 1;
locks_wake_up_blocks(right);
}
if (left) {
- left->fl_end = caller->fl_start - 1;
+ left->fl_end = request->fl_start - 1;
locks_wake_up_blocks(left);
}
out:
@@ -1323,22 +1295,35 @@ asmlinkage long sys_flock(unsigned int f
goto out_putf;

error = flock_make_lock(filp, &lock, cmd);
- if (error < 0)
+ if (error)
goto out_putf;

- error = security_ops->file_lock(filp, lock->fl_type);
+ error = security_ops->file_lock(filp, cmd);
if (error)
- goto out_putf;
+ goto out_free;

- error = flock_lock_file(filp, lock,
- (cmd & (LOCK_UN | LOCK_NB)) ? 0 : 1);
+ for (;;) {
+ error = flock_lock_file(filp, lock);
+ if ((error != -EAGAIN) || (cmd & LOCK_NB))
+ break;
+ error = wait_event_interruptible(lock->fl_wait, !lock->fl_next);
+ if (!error)
+ continue;

- if (error)
+ lock_kernel();
+ locks_delete_block(lock);
+ unlock_kernel();
+ break;
+ }
+
+ out_free:
+ if (error) {
locks_free_lock(lock);
+ }

-out_putf:
+ out_putf:
fput(filp);
-out:
+ out:
return error;
}

@@ -1442,6 +1427,9 @@ int fcntl_setlk(struct file *filp, unsig
error = flock_to_posix_lock(filp, file_lock, &flock);
if (error)
goto out;
+ if (cmd == F_SETLKW) {
+ file_lock->fl_flags |= FL_SLEEP;
+ }

error = -EBADF;
switch (flock.l_type) {
@@ -1469,10 +1457,26 @@ int fcntl_setlk(struct file *filp, unsig
if (error < 0)
goto out;
}
- error = posix_lock_file(filp, file_lock, cmd == F_SETLKW);
+
+ for (;;) {
+ error = posix_lock_file(filp, file_lock);
+ if ((error != -EAGAIN) || (cmd == F_SETLK))
+ break;
+ error = wait_event_interruptible(file_lock->fl_wait,
+ !file_lock->fl_next);
+ if (!error)
+ continue;
+
+ lock_kernel();
+ locks_delete_block(file_lock);
+ unlock_kernel();
+ break;
+ }

out:
- locks_free_lock(file_lock);
+ if (error) {
+ locks_free_lock(file_lock);
+ }
return error;
}

@@ -1565,6 +1569,9 @@ int fcntl_setlk64(struct file *filp, uns
error = flock64_to_posix_lock(filp, file_lock, &flock);
if (error)
goto out;
+ if (cmd == F_SETLKW64) {
+ file_lock->fl_flags |= FL_SLEEP;
+ }

error = -EBADF;
switch (flock.l_type) {
@@ -1592,10 +1599,27 @@ int fcntl_setlk64(struct file *filp, uns
if (error < 0)
goto out;
}
- error = posix_lock_file(filp, file_lock, cmd == F_SETLKW64);
+
+ for (;;) {
+ error = posix_lock_file(filp, file_lock);
+ if ((error != -EAGAIN) || (cmd == F_SETLK))
+ break;
+ error = wait_event_interruptible(file_lock->fl_wait,
+ !file_lock->fl_next);
+ if (!error)
+ continue;
+
+ lock_kernel();
+ locks_delete_block(file_lock);
+ unlock_kernel();
+ break;
+ }
+

out:
- locks_free_lock(file_lock);
+ if (error) {
+ locks_free_lock(file_lock);
+ }
return error;
}
#endif /* BITS_PER_LONG == 32 */
@@ -1630,7 +1654,7 @@ void locks_remove_posix(struct file *fil
/* Ignore any error -- we must remove the locks anyway */
}

- posix_lock_file(filp, &lock, 0);
+ posix_lock_file(filp, &lock);
}

/*
@@ -1684,10 +1708,21 @@ posix_block_lock(struct file_lock *block
* lockd needs to block waiting for locks.
*/
void
-posix_unblock_lock(struct file_lock *waiter)
+posix_unblock_lock(struct file *filp, struct file_lock *waiter)
{
- if (!list_empty(&waiter->fl_block))
+ /*
+ * A remote machine may cancel the lock request after it's been
+ * granted locally. If that happens, we need to delete the lock.
+ */
+ lock_kernel();
+ if (waiter->fl_next) {
locks_delete_block(waiter);
+ unlock_kernel();
+ } else {
+ unlock_kernel();
+ waiter->fl_type = F_UNLCK;
+ posix_lock_file(filp, waiter);
+ }
}

static void lock_get_status(char* out, struct file_lock *fl, int id, char *pfx)
@@ -1733,21 +1768,28 @@ static void lock_get_status(char* out, s
? (fl->fl_type & F_UNLCK) ? "UNLCK" : "READ "
: (fl->fl_type & F_WRLCK) ? "WRITE" : "READ ");
}
- /*
- * NOTE: it should be inode->i_sb->s_id, not kdevname(...).
- */
- out += sprintf(out, "%d %s:%ld ",
- fl->fl_pid,
- inode ? kdevname(to_kdev_t(inode->i_dev)) : "<none>",
- inode ? inode->i_ino : 0);
- out += sprintf(out, "%Ld ", fl->fl_start);
- if (fl->fl_end == OFFSET_MAX)
- out += sprintf(out, "EOF ");
- else
- out += sprintf(out, "%Ld ", fl->fl_end);
- sprintf(out, "%08lx %08lx %08lx %08lx %08lx\n",
- (long)fl, (long)fl->fl_link.prev, (long)fl->fl_link.next,
- (long)fl->fl_next, (long)fl->fl_block.next);
+#if WE_CAN_BREAK_LSLK_NOW
+ if (inode) {
+ out += sprintf(out, "%d %s:%ld ", fl->fl_pid,
+ inode->i_sb->s_id, inode->i_ino);
+ } else {
+ out += sprintf(out, "%d <none>:0 ", fl->fl_pid);
+ }
+#else
+ /* kdevname is a broken interface. but we expose it to userspace */
+ out += sprintf(out, "%d %s:%ld ", fl->fl_pid,
+ inode ? kdevname(to_kdev_t(inode->i_dev)) : "<none>",
+ inode ? inode->i_ino : 0);
+#endif
+ if (IS_POSIX(fl)) {
+ if (fl->fl_end == OFFSET_MAX)
+ out += sprintf(out, "%Ld EOF\n", fl->fl_start);
+ else
+ out += sprintf(out, "%Ld %Ld\n", fl->fl_start,
+ fl->fl_end);
+ } else {
+ out += sprintf(out, "0 EOF\n");
+ }
}

static void move_lock_status(char **p, off_t* pos, off_t offset)
diff -urpNX dontdiff linux-2.5.33/include/linux/fs.h linux-2.5.33-willy/include/linux/fs.h
--- linux-2.5.33/include/linux/fs.h 2002-08-28 06:42:36.000000000 -0700
+++ linux-2.5.33-willy/include/linux/fs.h 2002-09-07 22:07:43.000000000 -0700
@@ -525,10 +525,10 @@ extern int init_private_file(struct file

#define FL_POSIX 1
#define FL_FLOCK 2
-#define FL_BROKEN 4 /* broken flock() emulation */
-#define FL_ACCESS 8 /* for processes suspended by mandatory locking */
+#define FL_ACCESS 8 /* not trying to lock, just looking */
#define FL_LOCKD 16 /* lock held by rpc.lockd */
#define FL_LEASE 32 /* lease held on this file */
+#define FL_SLEEP 128 /* A blocking lock */

/*
* The POSIX file lock owner is determined by
@@ -590,9 +590,9 @@ extern void locks_copy_lock(struct file_
extern void locks_remove_posix(struct file *, fl_owner_t);
extern void locks_remove_flock(struct file *);
extern struct file_lock *posix_test_lock(struct file *, struct file_lock *);
-extern int posix_lock_file(struct file *, struct file_lock *, unsigned int);
+extern int posix_lock_file(struct file *, struct file_lock *);
extern void posix_block_lock(struct file_lock *, struct file_lock *);
-extern void posix_unblock_lock(struct file_lock *);
+extern void posix_unblock_lock(struct file *, struct file_lock *);
extern int posix_locks_deadlock(struct file_lock *, struct file_lock *);
extern int __get_lease(struct inode *inode, unsigned int flags);
extern time_t lease_get_mtime(struct inode *);
--
Revolutions do not require corporate support.
-
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/

\
 
 \ /
  Last update: 2005-03-22 13:28    [W:0.371 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site