Messages in this thread Patch in this message | | | Date | Sun, 13 Jul 1997 04:49:49 -0400 (EDT) | From | Michel LESPINASSE <> | Subject | [VFS+NCP patch] remote NCPFS locking support |
| |
Hi,
I'd like to announce you the release of the first alpha version of a patch that introduces NCP filesystem locking in the Linux system. It is against the stable version 2.0.30, because I feared I'd get enough bugs on my own without having to merge in those of the unstable linux versions :))
The problem you'll get with an unpatched linux client is that the locking will be locally done in the VFS layer, thus preventing two processes running in the same box from write locking the same file, but the filesystem layer is not informed about this locking decisions and thus he cannot inform the netware server about them, which isnt okay in a distributed computing environment. To solve it, I had to add some new functionnalityes in both the VFS layer and the NCP filesystem code.
The patch is working for me, and it has been tested with real netware servers as well as with the mars_nwe emulator. It still has a few known problems tough, and I have some questions that I'd like to ask before coding further - some related to the NCP protocol, and some related to the VFS layer implementation.
* I'm not sure about the rights i should ask for in ncp_make_open - should i ask for a read/write connection for a write lock, and a read-only connection for a read-only lock ? and what rights should i ask if I'm unlocking ? is there any documentation about the NCP protocol that could be helpful or should I just experiment and try to figure out by myself ?
* probably the read/write operations in inode.c and the flock operation in locks.c shouldnt share their lock. currently, a process cannot read or write a file if another process is trying to lock or unlock it.
* In the next version of this patch, I intend to do the inode locking in the VFS layer only if lock_op is not NULL - that is, only for the NCP filesystem currently :) This would be a possible solution to the previous point, too. I didnt put it in my current patch because I wanted to try if these locks introduced deadlocks - they didnt on my system, but I'd like to get more than an experimental proof about this :)
* I think that my implementation of the posix locking in the VFS layer is right, but I'm not sure at all about the flock-style locking. the inode is locked during the call to locks_delete_lock, which might want to sleep on a queue that is obviously related to the locking system. This *might* introduce a deadlock, if one of the tasks that locks_delete_lock wakes up needs to access this lock before it can wake_up us again. I'd like to get advice about this from someone that has a better knowledge of the VFS layer than I have.
* My current implementation of the filesystem-layer locking is only a prototype and is basically dumb, and more attention should be required when removing locks in a real implementation. If we are removing a read lock, there might be some other process that have read locks to intersecting regions of the file, so we obviously have to check for this before unlocking the entire region. I'll be working on this very soon, but I'd like to get some feedback first :) This prototype implementation will still work fine if your application only uses write locks, anyway.
--- linux/fs/ncpfs/file.c Mon Jul 7 15:43:39 1997 +++ linux-ncp/fs/ncpfs/file.c Mon Jul 7 23:22:18 1997 @@ -250,6 +250,73 @@ return already_written; } +/* + * return codes : 0 = lock accepted by the netware server + * 1 = lock refused + * < 0 = errno code + */ + +static int +ncp_file_lock(struct file_lock *fl, int wait) +{ + struct inode *inode; + int result; + + printk("inside ncp_file_lock :\n"); + if(fl==NULL) { + printk(" fl == NULL !!!!!\n"); + return 0; + } + switch(fl->fl_type) { + case F_UNLCK: + printk(" fl_type = F_UNLCK fl_start = %d fl_end = %d wait = %d\n", + fl->fl_start, fl->fl_end, wait); + break; + case F_WRLCK: + printk(" fl_type = F_WRLCK fl_start = %d fl_end = %d wait = %d\n", + fl->fl_start, fl->fl_end, wait); + break; + case F_RDLCK: + printk(" fl_type = F_RDLCK fl_start = %d fl_end = %d wait = %d\n", + fl->fl_start, fl->fl_end, wait); + break; + default: + printk(" invalid fl_type 0x%x!!!!!\n",(int)fl->fl_type); + } + + inode = fl->fl_file->f_inode; + + if (!ncp_conn_valid(NCP_SERVER(inode))) { + printk("ncp_file_lock : not connected ???\n"); + return -EIO; + } + if (!S_ISREG(inode->i_mode)) { + printk("ncp_file_lock: lock on non-file, mode %07o\n", + inode->i_mode); + return -EINVAL; + } + + /* Here I'm not sure about the rights I should ask for.... */ + if ((result = ncp_make_open(inode, O_RDWR)) != 0) { + return result; + } + + result = ncp_lock(NCP_SERVER(inode), NCP_FINFO(inode)->file_handle, + fl->fl_start, fl->fl_end, fl->fl_type); + printk(" server reply = %x\n",result); + if(result) { + if (wait) { + /* wait 1 second so that we dont + flood the server with retryes */ + current->timeout = jiffies + HZ; + current->state = TASK_INTERRUPTIBLE; + schedule(); + } + return 1; + } else + return 0; +} + static struct file_operations ncp_file_operations = { NULL, /* lseek - default */ ncp_file_read, /* read */ @@ -276,6 +343,11 @@ NULL, /* rename */ NULL, /* readlink */ NULL, /* follow_link */ + NULL, /* readpage */ + NULL, /* writepage */ NULL, /* bmap */ - NULL /* truncate */ + NULL, /* truncate */ + NULL, /* permission */ + NULL, /* smap */ + ncp_file_lock /* lock */ }; --- linux/fs/ncpfs/ncplib_kernel.h Mon Jul 7 19:16:39 1997 +++ linux-ncp/fs/ncpfs/ncplib_kernel.h Mon Jul 7 19:17:21 1997 @@ -121,6 +121,10 @@ const char *source, int *bytes_written); int +ncp_lock(struct ncp_server *server,const char *file_id, + __u32 start, __u32 end, int type); + +int ncp_obtain_info(struct ncp_server *server, __u8 vol_num, __u32 dir_base, char *path, /* At most 1 component */ --- linux/fs/ncpfs/ncplib_kernel.c Mon Jul 7 19:12:25 1997 +++ linux-ncp/fs/ncpfs/ncplib_kernel.c Mon Jul 7 20:11:31 1997 @@ -625,3 +625,23 @@ return 0; } +int +ncp_lock(struct ncp_server *server,const char *file_id, + __u32 start, __u32 end, int type) +{ + int result; + + /* locking protocol from mars_nwe pl10 in nwconn.c */ + ncp_init_request(server); + ncp_add_byte(server,0x01); + ncp_add_mem(server,file_id,6); + ncp_add_dword(server,htonl(start)); + ncp_add_dword(server,htonl(end + 1 - start)); + ncp_add_word(server,0); + if (type == F_UNLCK) + result = ncp_request(server,0x1e); + else + result = ncp_request(server,0x1a); + ncp_unlock_server(server); + return result; +} --- linux/include/linux/fs.h Mon Jul 7 15:44:29 1997 +++ linux-ncp/include/linux/fs.h Mon Jul 7 15:45:07 1997 @@ -488,6 +488,7 @@ void (*truncate) (struct inode *); int (*permission) (struct inode *, int); int (*smap) (struct inode *,int); + int (*lock) (struct file_lock *,int); }; struct super_operations { --- linux/fs/locks.c Mon Jul 7 15:42:38 1997 +++ linux-ncp/fs/locks.c Mon Jul 7 18:43:42 1997 @@ -663,6 +663,35 @@ return (0); } +/* + * We reuse the inode locking primitives from inode.c + * They allow us for atomic file locking + * In theory, it is not necessay for the read/write operations in inode.c + * and the flock operation in locks.c to share this same inode lock + * This implementation is just for convenience and should be improved if this + * is a performance problem. + */ + +void __wait_on_inode(struct inode *); + +static inline void wait_on_inode(struct inode * inode) +{ + if (inode->i_lock) + __wait_on_inode(inode); +} + +static inline void lock_inode(struct inode * inode) +{ + wait_on_inode(inode); + inode->i_lock = 1; +} + +static inline void unlock_inode(struct inode * inode) +{ + inode->i_lock = 0; + wake_up(&inode->i_wait); +} + /* Try to create a FLOCK lock on filp. We always insert new locks at * the head of the list. */ @@ -673,6 +702,15 @@ struct file_lock *new_fl; struct file_lock **before; int change = 0; + int (*lock_op)(struct file_lock *,int); + int result; + + if (filp->f_inode->i_op) + lock_op = filp->f_inode->i_op->lock; + else + lock_op = NULL; + + wait_on_inode(filp->f_inode); before = &filp->f_inode->i_flock; @@ -691,13 +729,23 @@ /* change means that we are changing the type of an existing lock, or * or else unlocking it. */ - if (change) + result = 0; + if (change) { + filp->f_inode->i_lock = 1; + fl->fl_type = F_UNLCK; + if (lock_op) + result = lock_op(fl,0); locks_delete_lock(before, caller->fl_type != F_UNLCK); + unlock_inode(filp->f_inode); + } if (caller->fl_type == F_UNLCK) - return (0); + return (result < 0 ? result : 0); if ((new_fl = locks_alloc_lock(caller)) == NULL) return (-ENOLCK); + repeat: + wait_on_inode(filp->f_inode); + if ((fl = filp->f_inode->i_flock) && (fl->fl_flags & F_POSIX)) { locks_free_lock(new_fl); return (-EBUSY); @@ -736,7 +784,28 @@ } fl = fl->fl_next; } + + /* + * VFS-level locking is okay, so we'll try filesystem-level locking, + * and if it works we'll allow the lock and register it. + * The filesystem-level procedure might need to sleep, but the + * locking operation must stay atomic if we dont want to get problems. + */ + + filp->f_inode->i_lock = 1; + if (lock_op && (result = lock_op(new_fl, wait))) { + /* file lock was refused by the filesystem layer */ + unlock_inode(filp->f_inode); + if (result < 0) + return (result); + if (wait) + goto repeat; + else + return (-EAGAIN); + } + locks_insert_lock(&filp->f_inode->i_flock, new_fl); + unlock_inode(filp->f_inode); return (0); } @@ -761,8 +830,17 @@ struct file_lock *right = NULL; struct file_lock **before; int added = 0; + int (*lock_op)(struct file_lock *,int); + int result; + + if (filp->f_inode->i_op) + lock_op = filp->f_inode->i_op->lock; + else + lock_op = NULL; repeat: + wait_on_inode(filp->f_inode); + if ((fl = filp->f_inode->i_flock) && (fl->fl_flags & F_FLOCK)) return (-EBUSY); @@ -783,6 +861,26 @@ fl = fl->fl_next; } } + + /* + * VFS-level locking is okay, so we'll try filesystem-level locking, + * and if it works we'll allow the lock and register it. + * The filesystem-level procedure might need to sleep, but the + * locking operation must stay atomic if we dont want to get problems. + */ + + filp->f_inode->i_lock = 1; + if (lock_op && (result = lock_op(caller, wait))) { + /* file lock was refused by the filesystem layer */ + unlock_inode(filp->f_inode); + if (result < 0) + return (result); + if (wait) + goto repeat; + else + return (-EAGAIN); + } + /* * Find the first old lock with the same owner as the new lock. */ @@ -877,10 +975,14 @@ } if (!added) { - if (caller->fl_type == F_UNLCK) + if (caller->fl_type == F_UNLCK) { + unlock_inode(filp->f_inode); return (0); - if ((new_fl = locks_alloc_lock(caller)) == NULL) + } + if ((new_fl = locks_alloc_lock(caller)) == NULL) { + unlock_inode(filp->f_inode); return (-ENOLCK); + } locks_insert_lock(before, new_fl); } if (right) { @@ -892,6 +994,7 @@ if ((left = locks_alloc_lock(right)) == NULL) { if (!added) locks_delete_lock(before, 0); + unlock_inode(filp->f_inode); return (-ENOLCK); } locks_insert_lock(before, left); @@ -903,6 +1006,7 @@ left->fl_end = caller->fl_start - 1; wake_up(&left->fl_wait); } + unlock_inode(filp->f_inode); return (0); } --- linux/fs/inode.c Mon Jul 7 16:20:05 1997 +++ linux-ncp/fs/inode.c Mon Jul 7 16:26:52 1997 @@ -137,7 +137,7 @@ return start; } -static void __wait_on_inode(struct inode *); +void __wait_on_inode(struct inode *); static inline void wait_on_inode(struct inode * inode) { @@ -638,7 +638,7 @@ * updating things: only a couple of 386 instructions). This should be * much better for interrupt latency. */ -static void __wait_on_inode(struct inode * inode) +void __wait_on_inode(struct inode * inode) { struct wait_queue wait = { current, NULL };
Michel "Walken" LESPINASSE - Student at Ecole Centrale Paris (France) www Email : walken@via.ecp.fr (o o) VideoLan project : http://videolan.via.ecp.fr/ ------oOO--(_)--OOo------------------------------------------------------- During this summer, please contact me at walken@foxfiber.com instead...
| |