On Thu, May 24, 2007 at 05:55:54PM +0100, David Howells wrote: > +/* > + * initialise the lock manager thread if it isn't already running > + */ > +static int afs_init_lock_manager(void) > +{ > + if (!afs_lock_manager) { > + afs_lock_manager = create_singlethread_workqueue("kafs_lockd"); > + if (!afs_lock_manager) > + return -ENOMEM; > + } > + return 0; Doesn't this need some locking? > +/* > + * request a lock on a file on the server > + */ > +static int afs_do_setlk(struct file *file, struct file_lock *fl) > +{ > + struct afs_vnode *vnode = AFS_FS_I(file->f_mapping->host); > + afs_lock_type_t type; > + struct key *key = file->private_data; > + int ret; > + > + _enter("{%x:%u},%u", vnode->fid.vid, vnode->fid.vnode, fl->fl_type); > + > + /* only whole-file locks are supported */ > + if (fl->fl_start != 0 || fl->fl_end != OFFSET_MAX) > + return -EINVAL; Do you allow upgrades and downgrades? (Just curious.) > + /* if we've already got a readlock on the server and no waiting > + * writelocks, then we might be able to instantly grant another Is that comment correct? (You don't really test for "waiting writelocks", do you?) > + * readlock */ > + if (type == AFS_LOCK_READ && > + vnode->flags & (1 << AFS_VNODE_READLOCKED)) { > + _debug("instant readlock"); > + ASSERTCMP(vnode->flags & > + ((1 << AFS_VNODE_LOCKING) | > + (1 << AFS_VNODE_WRITELOCKED)), ==, 0); > + ASSERT(!list_empty(&vnode->granted_locks)); > + goto sharing_existing_lock; > + } > + } --b. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html