[VFS+NCP patch] remote NCPFS locking support

Michel LESPINASSE (walken@foxfiber.com)
Sun, 13 Jul 1997 04:49:49 -0400 (EDT)


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...