On Mon, 07 Nov 2011 20:05:40 +0000 Sachin Prabhu <[email protected]> wrote:
> Allow flock over a cifs mount. > > Signed-off-by: Sachin Prabhu <[email protected]> > > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c > index 8f1fe32..56d3b36 100644 > --- a/fs/cifs/cifsfs.c > +++ b/fs/cifs/cifsfs.c > @@ -830,6 +830,7 @@ const struct file_operations cifs_file_ops = { > .open = cifs_open, > .release = cifs_close, > .lock = cifs_lock, > + .flock = cifs_flock, > .fsync = cifs_fsync, > .flush = cifs_flush, > .mmap = cifs_file_mmap, > @@ -849,6 +850,7 @@ const struct file_operations cifs_file_strict_ops = { > .open = cifs_open, > .release = cifs_close, > .lock = cifs_lock, > + .flock = cifs_flock, > .fsync = cifs_strict_fsync, > .flush = cifs_flush, > .mmap = cifs_file_strict_mmap, > @@ -869,6 +871,7 @@ const struct file_operations cifs_file_direct_ops = { > .open = cifs_open, > .release = cifs_close, > .lock = cifs_lock, > + .flock = cifs_flock, > .fsync = cifs_fsync, > .flush = cifs_flush, > .mmap = cifs_file_mmap, > diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h > index 30ff560..e119e9d 100644 > --- a/fs/cifs/cifsfs.h > +++ b/fs/cifs/cifsfs.h > @@ -87,6 +87,7 @@ extern ssize_t cifs_user_writev(struct kiocb *iocb, const > struct iovec *iov, > extern ssize_t cifs_strict_writev(struct kiocb *iocb, const struct iovec > *iov, > unsigned long nr_segs, loff_t pos); > extern int cifs_lock(struct file *, int, struct file_lock *); > +extern int cifs_flock(struct file *, int, struct file_lock *); > extern int cifs_fsync(struct file *, loff_t, loff_t, int); > extern int cifs_strict_fsync(struct file *, loff_t, loff_t, int); > extern int cifs_flush(struct file *, fl_owner_t id); > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index c1f063c..dd34f0b 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -43,6 +43,23 @@ > #include "cifs_fs_sb.h" > #include "fscache.h" > > +static int do_vfs_lock(struct file *file, struct file_lock *fl) > +{ > + int res = 0; > + > + switch (fl->fl_flags & (FL_POSIX|FL_FLOCK)) { > + case FL_POSIX: > + res = posix_lock_file_wait(file, fl); > + break; > + case FL_FLOCK: > + res = flock_lock_file_wait(file, fl); > + break; > + default: > + BUG(); Is this really BUG-worthy? Maybe an error return with a WARN or something would be more appropriate. I generally prefer not to BUG unless there really is no other option as a lot of enterprise distros set panic_on_oops. > + } > + return res; > +} > + > static inline int cifs_convert_flags(unsigned int flags) > { > if ((flags & O_ACCMODE) == O_RDONLY) > @@ -814,7 +831,7 @@ cifs_posix_lock_set(struct file *file, struct file_lock > *flock) > struct cifsInodeInfo *cinode = CIFS_I(file->f_path.dentry->d_inode); > int rc = 1; > > - if ((flock->fl_flags & FL_POSIX) == 0) > + if ((flock->fl_flags & (FL_POSIX | FL_FLOCK)) == 0) > return rc; > > mutex_lock(&cinode->lock_mutex); > @@ -822,7 +839,7 @@ cifs_posix_lock_set(struct file *file, struct file_lock > *flock) > mutex_unlock(&cinode->lock_mutex); > return rc; > } > - rc = posix_lock_file_wait(file, flock); > + rc = do_vfs_lock(file, flock); > mutex_unlock(&cinode->lock_mutex); > return rc; > } > @@ -1231,12 +1248,27 @@ cifs_setlk(struct file *file, struct file_lock > *flock, __u8 type, > struct cifs_tcon *tcon = tlink_tcon(cfile->tlink); > struct cifsInodeInfo *cinode = CIFS_I(file->f_path.dentry->d_inode); > __u16 netfid = cfile->netfid; > + unsigned char fl_flags = flock->fl_flags; > + > + if (unlock) { > + /*Check for existance of lock*/ > + flock->fl_flags |= FL_EXISTS; > + if (do_vfs_lock(flock->fl_file, flock) == -ENOENT) > + goto out; > + } else { > + /*Check if we can obtain lock*/ > + flock->fl_flags |= FL_ACCESS; > + rc = do_vfs_lock(flock->fl_file, flock); > + if (rc < 0) > + goto out; > + } > + flock->fl_flags = fl_flags; > > if (posix_lck) { > int posix_lock_type; > > rc = cifs_posix_lock_set(file, flock); > - if (!rc || rc < 0) > + if (rc <= 0) > return rc; > > if (type & LOCKING_ANDX_SHARED_LOCK) > @@ -1250,7 +1282,7 @@ cifs_setlk(struct file *file, struct file_lock *flock, > __u8 type, > rc = CIFSSMBPosixLock(xid, tcon, netfid, current->tgid, > 0 /* set */, length, flock, > posix_lock_type, wait_flag); > - goto out; > + goto out_lock; > } > > if (lock) { > @@ -1259,7 +1291,7 @@ cifs_setlk(struct file *file, struct file_lock *flock, > __u8 type, > if (rc < 0) > return rc; > else if (!rc) > - goto out; > + goto out_lock; > > rc = CIFSSMBLock(xid, tcon, netfid, current->tgid, length, > flock->fl_start, 0, 1, type, wait_flag, 0); > @@ -1271,9 +1303,14 @@ cifs_setlk(struct file *file, struct file_lock > *flock, __u8 type, > } else if (unlock) > rc = cifs_unlock_range(cfile, flock, xid); > > +out_lock: > + if (rc == 0 && lock) { > + /*We have to sleep here*/ nit: there should be spaces between the *'s and comment. I'd imagine checkpatch.pl would catch that, but maybe not? > + flock->fl_flags |= FL_SLEEP; > + do_vfs_lock(file, flock); > + } > out: > - if (flock->fl_flags & FL_POSIX) > - posix_lock_file_wait(file, flock); > + flock->fl_flags = fl_flags; > return rc; > } > > @@ -1334,6 +1371,22 @@ int cifs_lock(struct file *file, int cmd, struct > file_lock *flock) > return rc; > } > > +int cifs_flock(struct file *file, int cmd, struct file_lock *flock) > +{ > + int rc; > + > + rc = cifs_lock(file, cmd, flock); > + > + /* > + * flock requires -EWOULDBLOCK in case of conflicting locks > + * and LOCK_NB is used > + */ > + if ((rc == -EACCES) && !(flock->fl_flags & FL_SLEEP)) nit: these extra parens are not needed > + rc = -EWOULDBLOCK; > + > + return rc; > +} > + > /* update the file size (if needed) after a write */ > void > cifs_update_eof(struct cifsInodeInfo *cifsi, loff_t offset, > > Aside from the nits and the questionable BUG call, I think this looks like nice work. -- Jeff Layton <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
