On Mon, 07 Nov 2011 20:05:40 +0000
Sachin Prabhu <sprabhu@xxxxxxxxxx> wrote:
> Allow flock over a cifs mount.
>
> Signed-off-by: Sachin Prabhu <sprabhu@xxxxxxxxxx>
>
> 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 <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
