- use the FL_SLEEP semantics instead of a wait argument for the posix_lock_file interface. - remove the guts of posix_lock_file into posix_juggle_locks. rewrite so you don't have to spend 3 weeks looking at the code before you're convinced it's correct. diff -u linux-2.5.15-flock/fs/locks.c linux-2.5.15-flock/fs/locks.c --- linux-2.5.15-flock/fs/locks.c Tue May 14 13:49:45 2002 +++ linux-2.5.15-flock/fs/locks.c Thu May 16 14:52:40 2002 @@ -114,6 +114,11 @@ * Stephen Rothwell , June, 2000. */ +/* Each inode's list of locks first has 0 or more leases, + * then 0 or more whole-file locks (unsorted), + * then 0 or more range locks (sorted first on owner, then on location in file) + */ + #include #include #include @@ -825,6 +830,74 @@ return error; } +/* + * posix_juggle_locks - Attempt to merge POSIX locks + * @caller: new lock + * @fl: existing lock + * @returns 0 to continue looking, 1 to stop looking and -1 to indicate + * that we deleted the lock we were looking + * + * Attempt to merge these two locks. Due to the heinous POSIX locking + * semantics, we may end up having to split an existing lock into three + * pieces, so we need an extra lock. + */ +int posix_juggle_locks(struct file_lock *caller, struct file_lock *fl, + struct file_lock **before, struct file_lock *extra) +{ + if (caller->fl_type == fl->fl_type) { + if (fl->fl_end < caller->fl_start - 1) + return 0; + if (fl->fl_start > caller->fl_end + 1) + return 1; + + /* The new and old lock are of the same type and should be + * merged. Extend the new lock to be the union of the + * two locks and delete the existing lock. + */ + if (fl->fl_start < caller->fl_start) { + caller->fl_start = fl->fl_start; + } + if (fl->fl_end > caller->fl_end) { + caller->fl_end = fl->fl_end; + } + locks_delete_lock(before); + return -1; + } else { + if (fl->fl_end < caller->fl_start) + return 0; + if (fl->fl_start > caller->fl_end) + return 1; + + /* We have an overlap of some type. */ + if (caller->fl_start <= fl->fl_start) { + if (fl->fl_end <= caller->fl_end) { + /* We completely replace this lock. Just delete it. */ + locks_delete_lock(before); + return -1; + } + fl->fl_start = caller->fl_end + 1; + /* Maybe the changed size & type allows others to progress */ + locks_wake_up_blocks(fl); + return 1; + } + + if (fl->fl_end <= caller->fl_end) { + fl->fl_end = caller->fl_start - 1; + /* Maybe the changed size & type allows others to progress */ + locks_wake_up_blocks(fl); + return 1; + } + + /* The new lock splits the old lock. POSIX, we hatesss them */ + locks_copy_lock(extra, fl); + fl->fl_end = caller->fl_start - 1; + extra->fl_start = caller->fl_end + 1; + locks_insert_lock(&fl->fl_next, extra); + locks_wake_up_blocks(fl); + return 1; + } +} + /** * posix_lock_file: * @filp: The file to apply the lock to @@ -834,46 +907,35 @@ * Add a POSIX style lock to a file. * We merge adjacent locks whenever possible. POSIX locks are sorted by owner * task, then by starting address - * - * Kai Petzke writes: - * To make freeing a lock much faster, we keep a pointer to the lock before the - * actual one. But the real gain of the new coding was, that lock_it() and - * unlock_it() became one function. - * - * 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 *caller) { struct file_lock *fl; - struct file_lock *new_fl, *new_fl2; - struct file_lock *left = NULL; - struct file_lock *right = NULL; + struct file_lock *extra; struct file_lock **before; - struct inode * inode = filp->f_dentry->d_inode; - int error, added = 0; + struct inode *inode = filp->f_dentry->d_inode; + int error, found = 0; /* - * We may need two file_lock structures for this operation, - * so we get them in advance to avoid races. + * We may need an extra file_lock structure for this operation, + * so get it in advance to avoid atomic allocation. */ - new_fl = locks_alloc_lock(0); - new_fl2 = locks_alloc_lock(0); error = -ENOLCK; /* "no luck" */ - if (!(new_fl && new_fl2)) + extra = locks_alloc_lock(0); + if (!extra) goto out_nolock; lock_kernel(); if (caller->fl_type != F_UNLCK) { - repeat: + repeat: for (fl = inode->i_flock; fl != NULL; fl = fl->fl_next) { - if (!(fl->fl_flags & FL_POSIX)) + if (!IS_POSIX(fl)) continue; if (!posix_locks_conflict(caller, fl)) continue; error = -EAGAIN; - if (!wait) + if (!(caller->fl_flags & FL_SLEEP)) goto out; error = -EDEADLK; if (posix_locks_deadlock(caller, fl)) @@ -886,138 +948,38 @@ } } - /* - * We've allocated the new locks in advance, so there are no - * errors possible (and no blocking operations) from here on. - * - * Find the first old lock with the same owner as the new lock. - */ - - before = &inode->i_flock; - - /* First skip locks owned by other processes. - */ - while ((fl = *before) && (!(fl->fl_flags & FL_POSIX) || - !locks_same_owner(caller, fl))) { - before = &fl->fl_next; - } - - /* Process locks with this owner. - */ - while ((fl = *before) && locks_same_owner(caller, 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) - 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) + for_each_lock(inode, before) { + int result; + struct file_lock *fl; + again: + fl = *before; + if (!IS_POSIX(fl)) + continue; + if (!locks_same_owner(caller, fl)) { + if (found) break; - - /* If we come here, the new and old lock are of the - * same type and adjacent or overlapping. Make one - * 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; else - caller->fl_start = fl->fl_start; - if (fl->fl_end < caller->fl_end) - fl->fl_end = caller->fl_end; - else - caller->fl_end = fl->fl_end; - if (added) { - locks_delete_lock(before); continue; - } - caller = fl; - added = 1; - } - else { - /* Processing for different lock types is a bit - * more complex. - */ - if (fl->fl_end < caller->fl_start) - goto next_lock; - if (fl->fl_start > caller->fl_end) - break; - if (caller->fl_type == F_UNLCK) - added = 1; - if (fl->fl_start < caller->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) { - right = fl; - break; - } - if (fl->fl_start >= caller->fl_start) { - /* The new lock completely replaces an old - * one (This may happen several times). - */ - if (added) { - locks_delete_lock(before); - continue; - } - /* Replace the old lock with the new one. - * Wake up anybody waiting for the old one, - * as the change in lock type might satisfy - * 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; - added = 1; - } } - /* Go on to next lock. - */ - next_lock: - before = &fl->fl_next; + found = 1; + result = posix_juggle_locks(caller, fl, before, extra); + if (result == -1) + goto again; + if (result == 1) + break; } + locks_insert_lock(before, caller); error = 0; - if (!added) { - if (caller->fl_type == F_UNLCK) - goto out; - locks_copy_lock(new_fl, caller); - locks_insert_lock(before, new_fl); - new_fl = NULL; - } - if (right) { - if (left == right) { - /* The new lock breaks the old one in two pieces, - * so we have to use the second new lock. - */ - left = new_fl2; - new_fl2 = NULL; - locks_copy_lock(left, right); - locks_insert_lock(before, left); - } - right->fl_start = caller->fl_end + 1; - locks_wake_up_blocks(right); - } - if (left) { - left->fl_end = caller->fl_start - 1; - locks_wake_up_blocks(left); - } + out: unlock_kernel(); out_nolock: /* * Free any unused locks. */ - if (new_fl) - locks_free_lock(new_fl); - if (new_fl2) - locks_free_lock(new_fl2); + if (!extra->fl_next) + locks_free_lock(extra); return error; } @@ -1429,6 +1391,9 @@ 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) { @@ -1452,7 +1417,7 @@ if (error < 0) goto out; } - error = posix_lock_file(filp, file_lock, cmd == F_SETLKW); + error = posix_lock_file(filp, file_lock); out: locks_free_lock(file_lock); @@ -1549,6 +1514,9 @@ 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) { @@ -1572,7 +1540,7 @@ if (error < 0) goto out; } - error = posix_lock_file(filp, file_lock, cmd == F_SETLKW64); + error = posix_lock_file(filp, file_lock); out: locks_free_lock(file_lock); diff -u linux-2.5.15-flock/include/linux/fs.h linux-2.5.15-flock/include/linux/fs.h --- linux-2.5.15-flock/include/linux/fs.h Tue May 14 08:31:58 2002 +++ linux-2.5.15-flock/include/linux/fs.h Thu May 16 14:51:02 2002 @@ -548,7 +548,7 @@ 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 int posix_locks_deadlock(struct file_lock *, struct file_lock *); only in patch2: --- linux-2.5.15/fs/lockd/svcsubs.c Thu May 9 16:21:50 2002 +++ linux-2.5.15-flock/fs/lockd/svcsubs.c Thu May 16 14:55:30 2002 @@ -176,7 +176,7 @@ 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; only in patch2: --- linux-2.5.15/fs/lockd/svclock.c Thu May 9 16:23:11 2002 +++ linux-2.5.15-flock/fs/lockd/svclock.c Thu May 16 14:55:01 2002 @@ -242,7 +242,7 @@ 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); + posix_lock_file(&block->b_file->f_file, fl); block->b_granted = 0; } else { dprintk("lockd: unblocking blocked lock\n"); @@ -324,7 +324,7 @@ 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 +428,7 @@ 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; } @@ -533,7 +533,7 @@ * 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);