| Message ID | 20180802185219.4122-1-paulo@paulo.ac (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] cifs: Fix handling of OFD locks | expand |
2018-08-02 11:52 GMT-07:00 Paulo Alcantara <paulo@paulo.ac>: > OFD locks never conflict each other. I guess it should be mentioned that OFD locks never conflicts if they are set through the same FD. > > Signed-off-by: Paulo Alcantara <palcantara@suse.de> > --- > fs/cifs/cifsglob.h | 1 + > fs/cifs/cifsproto.h | 4 -- > fs/cifs/file.c | 109 +++++++++++++++++++++++++++++--------------- > 3 files changed, 74 insertions(+), 40 deletions(-) > > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index 0553929e8339..bb55e9aa4fe3 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -1069,6 +1069,7 @@ struct cifsLockInfo { > __u64 length; > __u32 pid; > __u32 type; > + unsigned int flags; /* file lock flags */ > }; > > /* > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h > index 20adda4de83b..e6a72a3f597b 100644 > --- a/fs/cifs/cifsproto.h > +++ b/fs/cifs/cifsproto.h > @@ -218,10 +218,6 @@ extern void cifs_umount(struct cifs_sb_info *); > extern void cifs_mark_open_files_invalid(struct cifs_tcon *tcon); > extern void cifs_reopen_persistent_handles(struct cifs_tcon *tcon); > > -extern bool cifs_find_lock_conflict(struct cifsFileInfo *cfile, __u64 offset, > - __u64 length, __u8 type, > - struct cifsLockInfo **conf_lock, > - int rw_check); > extern void cifs_add_pending_open(struct cifs_fid *fid, > struct tcon_link *tlink, > struct cifs_pending_open *open); > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index 8d41ca7bfcf1..58ee94550e42 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -864,14 +864,15 @@ int cifs_closedir(struct inode *inode, struct file *file) > } > > static struct cifsLockInfo * > -cifs_lock_init(__u64 offset, __u64 length, __u8 type) > +cifs_lock_init(struct file_lock *fl, __u8 type) > { > struct cifsLockInfo *lock = > kmalloc(sizeof(struct cifsLockInfo), GFP_KERNEL); > if (!lock) > return lock; > - lock->offset = offset; > - lock->length = length; > + lock->offset = fl->fl_start; > + lock->length = fl->fl_end - fl->fl_start + 1; > + lock->flags = fl->fl_flags; > lock->type = type; > lock->pid = current->tgid; > INIT_LIST_HEAD(&lock->blist); > @@ -893,30 +894,60 @@ cifs_del_lock_waiters(struct cifsLockInfo *lock) > #define CIFS_READ_OP 1 > #define CIFS_WRITE_OP 2 > > -/* @rw_check : 0 - no op, 1 - read, 2 - write */ > -static bool > -cifs_find_fid_lock_conflict(struct cifs_fid_locks *fdlocks, __u64 offset, > - __u64 length, __u8 type, struct cifsFileInfo *cfile, > - struct cifsLockInfo **conf_lock, int rw_check) > +static inline bool locks_overlap(struct cifsLockInfo *li, __u64 offset, > + __u64 length) > +{ > + return offset + length > li->offset && > + offset < li->offset + li->length; > +} > + > +static inline bool > +locks_conflict(struct cifsFileInfo *cfile, struct cifsFileInfo *cur_cfile, > + struct cifsLockInfo *li, __u64 offset, __u64 length, > + __u8 type, unsigned int flags, struct TCP_Server_Info *server, > + int rw_check) > +{ > + if (!locks_overlap(li, offset, length)) > + return false; > + if (!server->ops->compare_fids(cfile, cur_cfile)) > + return true; If FIDs don't match locks are considered conflicting which is not true for shared locks. > + if (current->tgid != li->pid) > + return true; > + > + if (rw_check == CIFS_LOCK_OP) { > + if ((flags & FL_OFDLCK) && (li->flags & FL_OFDLCK)) > + return false; > + if ((flags & FL_POSIX) && (li->flags & FL_POSIX)) > + return false; Here you are changing the behavior for POSIX locks that doesn't reflect the patch title/message. > + if (li->type & server->vals->exclusive_lock_type) > + return true; With the change above locking a file with a shared lock already locked by an exclusive lock from the same FD and PID won't be allowed which contradict LockFileEx semantics (https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-lockfileex). > + if (type & server->vals->exclusive_lock_type) > + return true; > + return false; > + } > + > + /* shared lock prevents write op through the same fid */ > + if (!(li->type & server->vals->shared_lock_type) || > + rw_check != CIFS_WRITE_OP) > + return false; > + > + return true; > +} > + > +static inline bool __fid_lock_conflicts(struct cifs_fid_locks *fdlocks, > + __u64 offset, __u64 length, > + __u8 type, unsigned int flags, > + struct cifsFileInfo *cfile, > + struct cifsLockInfo **conf_lock, > + int rw_check) > { > struct cifsLockInfo *li; > struct cifsFileInfo *cur_cfile = fdlocks->cfile; > struct TCP_Server_Info *server = tlink_tcon(cfile->tlink)->ses->server; > > list_for_each_entry(li, &fdlocks->locks, llist) { > - if (offset + length <= li->offset || > - offset >= li->offset + li->length) > - continue; > - if (rw_check != CIFS_LOCK_OP && current->tgid == li->pid && > - server->ops->compare_fids(cfile, cur_cfile)) { > - /* shared lock prevents write op through the same fid */ > - if (!(li->type & server->vals->shared_lock_type) || > - rw_check != CIFS_WRITE_OP) > - continue; > - } > - if ((type & server->vals->shared_lock_type) && > - ((server->ops->compare_fids(cfile, cur_cfile) && > - current->tgid == li->pid) || type == li->type)) > + if (!locks_conflict(cfile, cur_cfile, li, offset, length, type, > + flags, server, rw_check)) I am remembering that the existing logic is trying to match the SMB server behavior for SMB locks (see LockFileEx), so we won't have conflicts when we get a oplock/lease break and push byte-range locks to the server. This change will break the existing behavior for both POSIX and OFD locks: while you allow locks to be set locally, server will reject them and we will end up in a mixed state. Is it desired behavior? > continue; > if (conf_lock) > *conf_lock = li; > @@ -925,18 +956,19 @@ cifs_find_fid_lock_conflict(struct cifs_fid_locks *fdlocks, __u64 offset, > return false; > } > > -bool > -cifs_find_lock_conflict(struct cifsFileInfo *cfile, __u64 offset, __u64 length, > - __u8 type, struct cifsLockInfo **conf_lock, > - int rw_check) > +static bool cifs_find_lock_conflict(struct cifsFileInfo *cfile, __u64 offset, > + __u64 length, __u8 type, unsigned int flags, > + struct cifsLockInfo **conf_lock, > + int rw_check) > { > bool rc = false; > struct cifs_fid_locks *cur; > struct cifsInodeInfo *cinode = CIFS_I(d_inode(cfile->dentry)); > > list_for_each_entry(cur, &cinode->llist, llist) { > - rc = cifs_find_fid_lock_conflict(cur, offset, length, type, > - cfile, conf_lock, rw_check); > + rc = __fid_lock_conflicts(cur, offset, length, type, > + flags, cfile, conf_lock, > + rw_check); > if (rc) > break; > } > @@ -964,7 +996,8 @@ cifs_lock_test(struct cifsFileInfo *cfile, __u64 offset, __u64 length, > down_read(&cinode->lock_sem); > > exist = cifs_find_lock_conflict(cfile, offset, length, type, > - &conf_lock, CIFS_LOCK_OP); > + flock->fl_flags, &conf_lock, > + CIFS_LOCK_OP); > if (exist) { > flock->fl_start = conf_lock->offset; > flock->fl_end = conf_lock->offset + conf_lock->length - 1; > @@ -1011,7 +1044,8 @@ cifs_lock_add_if(struct cifsFileInfo *cfile, struct cifsLockInfo *lock, > down_write(&cinode->lock_sem); > > exist = cifs_find_lock_conflict(cfile, lock->offset, lock->length, > - lock->type, &conf_lock, CIFS_LOCK_OP); > + lock->type, lock->flags, &conf_lock, > + CIFS_LOCK_OP); > if (!exist && cinode->can_cache_brlcks) { > list_add_tail(&lock->llist, &cfile->llist->locks); > up_write(&cinode->lock_sem); > @@ -1307,8 +1341,15 @@ static void > cifs_read_flock(struct file_lock *flock, __u32 *type, int *lock, int *unlock, > bool *wait_flag, struct TCP_Server_Info *server) > { > + unsigned int flags = FL_POSIX | FL_OFDLCK | FL_FLOCK | FL_SLEEP | > + FL_ACCESS | FL_LEASE | FL_CLOSE; > + > + if (flock->fl_flags & ~flags) > + cifs_dbg(FYI, "Unknown lock flags 0x%x\n", flock->fl_flags); > if (flock->fl_flags & FL_POSIX) > cifs_dbg(FYI, "Posix\n"); > + if (flock->fl_flags & FL_OFDLCK) > + cifs_dbg(FYI, "OFD lock\n"); > if (flock->fl_flags & FL_FLOCK) > cifs_dbg(FYI, "Flock\n"); > if (flock->fl_flags & FL_SLEEP) { > @@ -1319,10 +1360,6 @@ cifs_read_flock(struct file_lock *flock, __u32 *type, int *lock, int *unlock, > cifs_dbg(FYI, "Process suspended by mandatory locking - not implemented yet\n"); > if (flock->fl_flags & FL_LEASE) > cifs_dbg(FYI, "Lease on file - not implemented yet\n"); > - if (flock->fl_flags & > - (~(FL_POSIX | FL_FLOCK | FL_SLEEP | > - FL_ACCESS | FL_LEASE | FL_CLOSE))) > - cifs_dbg(FYI, "Unknown lock flags 0x%x\n", flock->fl_flags); > > *type = server->vals->large_lock_type; > if (flock->fl_type == F_WRLCK) { > @@ -1584,7 +1621,7 @@ cifs_setlk(struct file *file, struct file_lock *flock, __u32 type, > if (lock) { > struct cifsLockInfo *lock; > > - lock = cifs_lock_init(flock->fl_start, length, type); > + lock = cifs_lock_init(flock, type); > if (!lock) > return -ENOMEM; > > @@ -2817,7 +2854,7 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from) > goto out; > > if (!cifs_find_lock_conflict(cfile, iocb->ki_pos, iov_iter_count(from), > - server->vals->exclusive_lock_type, NULL, > + server->vals->exclusive_lock_type, 0, NULL, > CIFS_WRITE_OP)) > rc = __generic_file_write_iter(iocb, from); > else > @@ -3388,7 +3425,7 @@ cifs_strict_readv(struct kiocb *iocb, struct iov_iter *to) > down_read(&cinode->lock_sem); > if (!cifs_find_lock_conflict(cfile, iocb->ki_pos, iov_iter_count(to), > tcon->ses->server->vals->shared_lock_type, > - NULL, CIFS_READ_OP)) > + 0, NULL, CIFS_READ_OP)) > rc = generic_file_read_iter(iocb, to); > up_read(&cinode->lock_sem); > return rc; > -- > 2.18.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Best regards, Pavel Shilovsky -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Pavel, Thanks for the review! On August 2, 2018 7:17:42 PM GMT-03:00, Pavel Shilovsky <piastryyy@gmail.com> wrote: >2018-08-02 11:52 GMT-07:00 Paulo Alcantara <paulo@paulo.ac>: >> OFD locks never conflict each other. > >I guess it should be mentioned that OFD locks never conflicts if they >are set through the same FD. OK. > >> >> Signed-off-by: Paulo Alcantara <palcantara@suse.de> >> --- >> fs/cifs/cifsglob.h | 1 + >> fs/cifs/cifsproto.h | 4 -- >> fs/cifs/file.c | 109 >+++++++++++++++++++++++++++++--------------- >> 3 files changed, 74 insertions(+), 40 deletions(-) >> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h >> index 0553929e8339..bb55e9aa4fe3 100644 >> --- a/fs/cifs/cifsglob.h >> +++ b/fs/cifs/cifsglob.h >> @@ -1069,6 +1069,7 @@ struct cifsLockInfo { >> __u64 length; >> __u32 pid; >> __u32 type; >> + unsigned int flags; /* file lock flags */ >> }; >> >> /* >> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h >> index 20adda4de83b..e6a72a3f597b 100644 >> --- a/fs/cifs/cifsproto.h >> +++ b/fs/cifs/cifsproto.h >> @@ -218,10 +218,6 @@ extern void cifs_umount(struct cifs_sb_info *); >> extern void cifs_mark_open_files_invalid(struct cifs_tcon *tcon); >> extern void cifs_reopen_persistent_handles(struct cifs_tcon *tcon); >> >> -extern bool cifs_find_lock_conflict(struct cifsFileInfo *cfile, >__u64 offset, >> - __u64 length, __u8 type, >> - struct cifsLockInfo **conf_lock, >> - int rw_check); >> extern void cifs_add_pending_open(struct cifs_fid *fid, >> struct tcon_link *tlink, >> struct cifs_pending_open *open); >> diff --git a/fs/cifs/file.c b/fs/cifs/file.c >> index 8d41ca7bfcf1..58ee94550e42 100644 >> --- a/fs/cifs/file.c >> +++ b/fs/cifs/file.c >> @@ -864,14 +864,15 @@ int cifs_closedir(struct inode *inode, struct >file *file) >> } >> >> static struct cifsLockInfo * >> -cifs_lock_init(__u64 offset, __u64 length, __u8 type) >> +cifs_lock_init(struct file_lock *fl, __u8 type) >> { >> struct cifsLockInfo *lock = >> kmalloc(sizeof(struct cifsLockInfo), GFP_KERNEL); >> if (!lock) >> return lock; >> - lock->offset = offset; >> - lock->length = length; >> + lock->offset = fl->fl_start; >> + lock->length = fl->fl_end - fl->fl_start + 1; >> + lock->flags = fl->fl_flags; >> lock->type = type; >> lock->pid = current->tgid; >> INIT_LIST_HEAD(&lock->blist); >> @@ -893,30 +894,60 @@ cifs_del_lock_waiters(struct cifsLockInfo >*lock) >> #define CIFS_READ_OP 1 >> #define CIFS_WRITE_OP 2 >> >> -/* @rw_check : 0 - no op, 1 - read, 2 - write */ >> -static bool >> -cifs_find_fid_lock_conflict(struct cifs_fid_locks *fdlocks, __u64 >offset, >> - __u64 length, __u8 type, struct >cifsFileInfo *cfile, >> - struct cifsLockInfo **conf_lock, int >rw_check) >> +static inline bool locks_overlap(struct cifsLockInfo *li, __u64 >offset, >> + __u64 length) >> +{ >> + return offset + length > li->offset && >> + offset < li->offset + li->length; >> +} >> + >> +static inline bool >> +locks_conflict(struct cifsFileInfo *cfile, struct cifsFileInfo >*cur_cfile, >> + struct cifsLockInfo *li, __u64 offset, __u64 length, >> + __u8 type, unsigned int flags, struct TCP_Server_Info >*server, >> + int rw_check) >> +{ >> + if (!locks_overlap(li, offset, length)) >> + return false; >> + if (!server->ops->compare_fids(cfile, cur_cfile)) >> + return true; > >If FIDs don't match locks are considered conflicting which is not true >for shared locks. > >> + if (current->tgid != li->pid) >> + return true; >> + >> + if (rw_check == CIFS_LOCK_OP) { >> + if ((flags & FL_OFDLCK) && (li->flags & FL_OFDLCK)) >> + return false; >> + if ((flags & FL_POSIX) && (li->flags & FL_POSIX)) >> + return false; > >Here you are changing the behavior for POSIX locks that doesn't >reflect the patch title/message. > >> + if (li->type & server->vals->exclusive_lock_type) >> + return true; > >With the change above locking a file with a shared lock already locked >by an exclusive lock from the same FD and PID won't be allowed which >contradict LockFileEx semantics >(https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-lockfileex). You're right. I will fix it. > >> + if (type & server->vals->exclusive_lock_type) >> + return true; >> + return false; >> + } >> + >> + /* shared lock prevents write op through the same fid */ >> + if (!(li->type & server->vals->shared_lock_type) || >> + rw_check != CIFS_WRITE_OP) >> + return false; >> + >> + return true; >> +} >> + >> +static inline bool __fid_lock_conflicts(struct cifs_fid_locks >*fdlocks, >> + __u64 offset, __u64 length, >> + __u8 type, unsigned int >flags, >> + struct cifsFileInfo *cfile, >> + struct cifsLockInfo >**conf_lock, >> + int rw_check) >> { >> struct cifsLockInfo *li; >> struct cifsFileInfo *cur_cfile = fdlocks->cfile; >> struct TCP_Server_Info *server = >tlink_tcon(cfile->tlink)->ses->server; >> >> list_for_each_entry(li, &fdlocks->locks, llist) { >> - if (offset + length <= li->offset || >> - offset >= li->offset + li->length) >> - continue; >> - if (rw_check != CIFS_LOCK_OP && current->tgid == >li->pid && >> - server->ops->compare_fids(cfile, cur_cfile)) { >> - /* shared lock prevents write op through the >same fid */ >> - if (!(li->type & >server->vals->shared_lock_type) || >> - rw_check != CIFS_WRITE_OP) >> - continue; >> - } >> - if ((type & server->vals->shared_lock_type) && >> - ((server->ops->compare_fids(cfile, cur_cfile) && >> - current->tgid == li->pid) || type == li->type)) >> + if (!locks_conflict(cfile, cur_cfile, li, offset, >length, type, >> + flags, server, rw_check)) > >I am remembering that the existing logic is trying to match the SMB >server behavior for SMB locks (see LockFileEx), so we won't have >conflicts when we get a oplock/lease break and push byte-range locks >to the server. > >This change will break the existing behavior for both POSIX and OFD >locks: while you allow locks to be set locally, server will reject >them and we will end up in a mixed state. Is it desired behavior? Nope, it isn't. Your comments make sense and the logic is broken. I will fix it in next version. > > >> continue; >> if (conf_lock) >> *conf_lock = li; >> @@ -925,18 +956,19 @@ cifs_find_fid_lock_conflict(struct >cifs_fid_locks *fdlocks, __u64 offset, >> return false; >> } >> >> -bool >> -cifs_find_lock_conflict(struct cifsFileInfo *cfile, __u64 offset, >__u64 length, >> - __u8 type, struct cifsLockInfo **conf_lock, >> - int rw_check) >> +static bool cifs_find_lock_conflict(struct cifsFileInfo *cfile, >__u64 offset, >> + __u64 length, __u8 type, unsigned >int flags, >> + struct cifsLockInfo **conf_lock, >> + int rw_check) >> { >> bool rc = false; >> struct cifs_fid_locks *cur; >> struct cifsInodeInfo *cinode = >CIFS_I(d_inode(cfile->dentry)); >> >> list_for_each_entry(cur, &cinode->llist, llist) { >> - rc = cifs_find_fid_lock_conflict(cur, offset, length, >type, >> - cfile, conf_lock, >rw_check); >> + rc = __fid_lock_conflicts(cur, offset, length, type, >> + flags, cfile, conf_lock, >> + rw_check); >> if (rc) >> break; >> } >> @@ -964,7 +996,8 @@ cifs_lock_test(struct cifsFileInfo *cfile, __u64 >offset, __u64 length, >> down_read(&cinode->lock_sem); >> >> exist = cifs_find_lock_conflict(cfile, offset, length, type, >> - &conf_lock, CIFS_LOCK_OP); >> + flock->fl_flags, &conf_lock, >> + CIFS_LOCK_OP); >> if (exist) { >> flock->fl_start = conf_lock->offset; >> flock->fl_end = conf_lock->offset + conf_lock->length >- 1; >> @@ -1011,7 +1044,8 @@ cifs_lock_add_if(struct cifsFileInfo *cfile, >struct cifsLockInfo *lock, >> down_write(&cinode->lock_sem); >> >> exist = cifs_find_lock_conflict(cfile, lock->offset, >lock->length, >> - lock->type, &conf_lock, >CIFS_LOCK_OP); >> + lock->type, lock->flags, >&conf_lock, >> + CIFS_LOCK_OP); >> if (!exist && cinode->can_cache_brlcks) { >> list_add_tail(&lock->llist, &cfile->llist->locks); >> up_write(&cinode->lock_sem); >> @@ -1307,8 +1341,15 @@ static void >> cifs_read_flock(struct file_lock *flock, __u32 *type, int *lock, int >*unlock, >> bool *wait_flag, struct TCP_Server_Info *server) >> { >> + unsigned int flags = FL_POSIX | FL_OFDLCK | FL_FLOCK | >FL_SLEEP | >> + FL_ACCESS | FL_LEASE | FL_CLOSE; >> + >> + if (flock->fl_flags & ~flags) >> + cifs_dbg(FYI, "Unknown lock flags 0x%x\n", >flock->fl_flags); >> if (flock->fl_flags & FL_POSIX) >> cifs_dbg(FYI, "Posix\n"); >> + if (flock->fl_flags & FL_OFDLCK) >> + cifs_dbg(FYI, "OFD lock\n"); >> if (flock->fl_flags & FL_FLOCK) >> cifs_dbg(FYI, "Flock\n"); >> if (flock->fl_flags & FL_SLEEP) { >> @@ -1319,10 +1360,6 @@ cifs_read_flock(struct file_lock *flock, __u32 >*type, int *lock, int *unlock, >> cifs_dbg(FYI, "Process suspended by mandatory locking >- not implemented yet\n"); >> if (flock->fl_flags & FL_LEASE) >> cifs_dbg(FYI, "Lease on file - not implemented >yet\n"); >> - if (flock->fl_flags & >> - (~(FL_POSIX | FL_FLOCK | FL_SLEEP | >> - FL_ACCESS | FL_LEASE | FL_CLOSE))) >> - cifs_dbg(FYI, "Unknown lock flags 0x%x\n", >flock->fl_flags); >> >> *type = server->vals->large_lock_type; >> if (flock->fl_type == F_WRLCK) { >> @@ -1584,7 +1621,7 @@ cifs_setlk(struct file *file, struct file_lock >*flock, __u32 type, >> if (lock) { >> struct cifsLockInfo *lock; >> >> - lock = cifs_lock_init(flock->fl_start, length, type); >> + lock = cifs_lock_init(flock, type); >> if (!lock) >> return -ENOMEM; >> >> @@ -2817,7 +2854,7 @@ cifs_writev(struct kiocb *iocb, struct iov_iter >*from) >> goto out; >> >> if (!cifs_find_lock_conflict(cfile, iocb->ki_pos, >iov_iter_count(from), >> - >server->vals->exclusive_lock_type, NULL, >> + >server->vals->exclusive_lock_type, 0, NULL, >> CIFS_WRITE_OP)) >> rc = __generic_file_write_iter(iocb, from); >> else >> @@ -3388,7 +3425,7 @@ cifs_strict_readv(struct kiocb *iocb, struct >iov_iter *to) >> down_read(&cinode->lock_sem); >> if (!cifs_find_lock_conflict(cfile, iocb->ki_pos, >iov_iter_count(to), >> >tcon->ses->server->vals->shared_lock_type, >> - NULL, CIFS_READ_OP)) >> + 0, NULL, CIFS_READ_OP)) >> rc = generic_file_read_iter(iocb, to); >> up_read(&cinode->lock_sem); >> return rc; >> -- >> 2.18.0 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" >in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > >Best regards, >Pavel Shilovsky
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 0553929e8339..bb55e9aa4fe3 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -1069,6 +1069,7 @@ struct cifsLockInfo { __u64 length; __u32 pid; __u32 type; + unsigned int flags; /* file lock flags */ }; /* diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index 20adda4de83b..e6a72a3f597b 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -218,10 +218,6 @@ extern void cifs_umount(struct cifs_sb_info *); extern void cifs_mark_open_files_invalid(struct cifs_tcon *tcon); extern void cifs_reopen_persistent_handles(struct cifs_tcon *tcon); -extern bool cifs_find_lock_conflict(struct cifsFileInfo *cfile, __u64 offset, - __u64 length, __u8 type, - struct cifsLockInfo **conf_lock, - int rw_check); extern void cifs_add_pending_open(struct cifs_fid *fid, struct tcon_link *tlink, struct cifs_pending_open *open); diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 8d41ca7bfcf1..58ee94550e42 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -864,14 +864,15 @@ int cifs_closedir(struct inode *inode, struct file *file) } static struct cifsLockInfo * -cifs_lock_init(__u64 offset, __u64 length, __u8 type) +cifs_lock_init(struct file_lock *fl, __u8 type) { struct cifsLockInfo *lock = kmalloc(sizeof(struct cifsLockInfo), GFP_KERNEL); if (!lock) return lock; - lock->offset = offset; - lock->length = length; + lock->offset = fl->fl_start; + lock->length = fl->fl_end - fl->fl_start + 1; + lock->flags = fl->fl_flags; lock->type = type; lock->pid = current->tgid; INIT_LIST_HEAD(&lock->blist); @@ -893,30 +894,60 @@ cifs_del_lock_waiters(struct cifsLockInfo *lock) #define CIFS_READ_OP 1 #define CIFS_WRITE_OP 2 -/* @rw_check : 0 - no op, 1 - read, 2 - write */ -static bool -cifs_find_fid_lock_conflict(struct cifs_fid_locks *fdlocks, __u64 offset, - __u64 length, __u8 type, struct cifsFileInfo *cfile, - struct cifsLockInfo **conf_lock, int rw_check) +static inline bool locks_overlap(struct cifsLockInfo *li, __u64 offset, + __u64 length) +{ + return offset + length > li->offset && + offset < li->offset + li->length; +} + +static inline bool +locks_conflict(struct cifsFileInfo *cfile, struct cifsFileInfo *cur_cfile, + struct cifsLockInfo *li, __u64 offset, __u64 length, + __u8 type, unsigned int flags, struct TCP_Server_Info *server, + int rw_check) +{ + if (!locks_overlap(li, offset, length)) + return false; + if (!server->ops->compare_fids(cfile, cur_cfile)) + return true; + if (current->tgid != li->pid) + return true; + + if (rw_check == CIFS_LOCK_OP) { + if ((flags & FL_OFDLCK) && (li->flags & FL_OFDLCK)) + return false; + if ((flags & FL_POSIX) && (li->flags & FL_POSIX)) + return false; + if (li->type & server->vals->exclusive_lock_type) + return true; + if (type & server->vals->exclusive_lock_type) + return true; + return false; + } + + /* shared lock prevents write op through the same fid */ + if (!(li->type & server->vals->shared_lock_type) || + rw_check != CIFS_WRITE_OP) + return false; + + return true; +} + +static inline bool __fid_lock_conflicts(struct cifs_fid_locks *fdlocks, + __u64 offset, __u64 length, + __u8 type, unsigned int flags, + struct cifsFileInfo *cfile, + struct cifsLockInfo **conf_lock, + int rw_check) { struct cifsLockInfo *li; struct cifsFileInfo *cur_cfile = fdlocks->cfile; struct TCP_Server_Info *server = tlink_tcon(cfile->tlink)->ses->server; list_for_each_entry(li, &fdlocks->locks, llist) { - if (offset + length <= li->offset || - offset >= li->offset + li->length) - continue; - if (rw_check != CIFS_LOCK_OP && current->tgid == li->pid && - server->ops->compare_fids(cfile, cur_cfile)) { - /* shared lock prevents write op through the same fid */ - if (!(li->type & server->vals->shared_lock_type) || - rw_check != CIFS_WRITE_OP) - continue; - } - if ((type & server->vals->shared_lock_type) && - ((server->ops->compare_fids(cfile, cur_cfile) && - current->tgid == li->pid) || type == li->type)) + if (!locks_conflict(cfile, cur_cfile, li, offset, length, type, + flags, server, rw_check)) continue; if (conf_lock) *conf_lock = li; @@ -925,18 +956,19 @@ cifs_find_fid_lock_conflict(struct cifs_fid_locks *fdlocks, __u64 offset, return false; } -bool -cifs_find_lock_conflict(struct cifsFileInfo *cfile, __u64 offset, __u64 length, - __u8 type, struct cifsLockInfo **conf_lock, - int rw_check) +static bool cifs_find_lock_conflict(struct cifsFileInfo *cfile, __u64 offset, + __u64 length, __u8 type, unsigned int flags, + struct cifsLockInfo **conf_lock, + int rw_check) { bool rc = false; struct cifs_fid_locks *cur; struct cifsInodeInfo *cinode = CIFS_I(d_inode(cfile->dentry)); list_for_each_entry(cur, &cinode->llist, llist) { - rc = cifs_find_fid_lock_conflict(cur, offset, length, type, - cfile, conf_lock, rw_check); + rc = __fid_lock_conflicts(cur, offset, length, type, + flags, cfile, conf_lock, + rw_check); if (rc) break; } @@ -964,7 +996,8 @@ cifs_lock_test(struct cifsFileInfo *cfile, __u64 offset, __u64 length, down_read(&cinode->lock_sem); exist = cifs_find_lock_conflict(cfile, offset, length, type, - &conf_lock, CIFS_LOCK_OP); + flock->fl_flags, &conf_lock, + CIFS_LOCK_OP); if (exist) { flock->fl_start = conf_lock->offset; flock->fl_end = conf_lock->offset + conf_lock->length - 1; @@ -1011,7 +1044,8 @@ cifs_lock_add_if(struct cifsFileInfo *cfile, struct cifsLockInfo *lock, down_write(&cinode->lock_sem); exist = cifs_find_lock_conflict(cfile, lock->offset, lock->length, - lock->type, &conf_lock, CIFS_LOCK_OP); + lock->type, lock->flags, &conf_lock, + CIFS_LOCK_OP); if (!exist && cinode->can_cache_brlcks) { list_add_tail(&lock->llist, &cfile->llist->locks); up_write(&cinode->lock_sem); @@ -1307,8 +1341,15 @@ static void cifs_read_flock(struct file_lock *flock, __u32 *type, int *lock, int *unlock, bool *wait_flag, struct TCP_Server_Info *server) { + unsigned int flags = FL_POSIX | FL_OFDLCK | FL_FLOCK | FL_SLEEP | + FL_ACCESS | FL_LEASE | FL_CLOSE; + + if (flock->fl_flags & ~flags) + cifs_dbg(FYI, "Unknown lock flags 0x%x\n", flock->fl_flags); if (flock->fl_flags & FL_POSIX) cifs_dbg(FYI, "Posix\n"); + if (flock->fl_flags & FL_OFDLCK) + cifs_dbg(FYI, "OFD lock\n"); if (flock->fl_flags & FL_FLOCK) cifs_dbg(FYI, "Flock\n"); if (flock->fl_flags & FL_SLEEP) { @@ -1319,10 +1360,6 @@ cifs_read_flock(struct file_lock *flock, __u32 *type, int *lock, int *unlock, cifs_dbg(FYI, "Process suspended by mandatory locking - not implemented yet\n"); if (flock->fl_flags & FL_LEASE) cifs_dbg(FYI, "Lease on file - not implemented yet\n"); - if (flock->fl_flags & - (~(FL_POSIX | FL_FLOCK | FL_SLEEP | - FL_ACCESS | FL_LEASE | FL_CLOSE))) - cifs_dbg(FYI, "Unknown lock flags 0x%x\n", flock->fl_flags); *type = server->vals->large_lock_type; if (flock->fl_type == F_WRLCK) { @@ -1584,7 +1621,7 @@ cifs_setlk(struct file *file, struct file_lock *flock, __u32 type, if (lock) { struct cifsLockInfo *lock; - lock = cifs_lock_init(flock->fl_start, length, type); + lock = cifs_lock_init(flock, type); if (!lock) return -ENOMEM; @@ -2817,7 +2854,7 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from) goto out; if (!cifs_find_lock_conflict(cfile, iocb->ki_pos, iov_iter_count(from), - server->vals->exclusive_lock_type, NULL, + server->vals->exclusive_lock_type, 0, NULL, CIFS_WRITE_OP)) rc = __generic_file_write_iter(iocb, from); else @@ -3388,7 +3425,7 @@ cifs_strict_readv(struct kiocb *iocb, struct iov_iter *to) down_read(&cinode->lock_sem); if (!cifs_find_lock_conflict(cfile, iocb->ki_pos, iov_iter_count(to), tcon->ses->server->vals->shared_lock_type, - NULL, CIFS_READ_OP)) + 0, NULL, CIFS_READ_OP)) rc = generic_file_read_iter(iocb, to); up_read(&cinode->lock_sem); return rc;
OFD locks never conflict each other. Signed-off-by: Paulo Alcantara <palcantara@suse.de> --- fs/cifs/cifsglob.h | 1 + fs/cifs/cifsproto.h | 4 -- fs/cifs/file.c | 109 +++++++++++++++++++++++++++++--------------- 3 files changed, 74 insertions(+), 40 deletions(-)