| Message ID | 1448563859-21922-3-git-send-email-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 26, 2015 at 07:50:56PM +0100, Christoph Hellwig wrote: > Pass a loff_t end for the last byte instead of the 32-bit count > parameter to allow full file clones even on 32-bit architectures. > While we're at it also drop the pointless inode argument and simplify > the read/write selection. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/locks.c | 22 +++++++++------------- > fs/read_write.c | 5 ++--- > include/linux/fs.h | 28 +++++++++++++--------------- > 3 files changed, 24 insertions(+), 31 deletions(-) > > diff --git a/fs/locks.c b/fs/locks.c > index 0d2b326..d503669 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -1227,21 +1227,17 @@ int locks_mandatory_locked(struct file *file) > > /** > * locks_mandatory_area - Check for a conflicting lock > - * @read_write: %FLOCK_VERIFY_WRITE for exclusive access, %FLOCK_VERIFY_READ > - * for shared > - * @inode: the file to check > * @filp: how the file was opened (if it was) > - * @offset: start of area to check > - * @count: length of area to check > + * @start: first byte in the file to check > + * @end: lastbyte in the file to check > + * @write: %true if checking for write access > * > * Searches the inode's list of locks to find any POSIX locks which conflict. > - * This function is called from rw_verify_area() and > - * locks_verify_truncate(). > */ > -int locks_mandatory_area(int read_write, struct inode *inode, > - struct file *filp, loff_t offset, > - size_t count) > +int locks_mandatory_area(struct file *filp, loff_t start, loff_t end, > + bool write) > { > + struct inode *inode = file_inode(filp); > struct file_lock fl; > int error; > bool sleep = false; > @@ -1252,9 +1248,9 @@ int locks_mandatory_area(int read_write, struct inode *inode, > fl.fl_flags = FL_POSIX | FL_ACCESS; > if (filp && !(filp->f_flags & O_NONBLOCK)) > sleep = true; > - fl.fl_type = (read_write == FLOCK_VERIFY_WRITE) ? F_WRLCK : F_RDLCK; > - fl.fl_start = offset; > - fl.fl_end = offset + count - 1; > + fl.fl_type = write ? F_WRLCK : F_RDLCK; > + fl.fl_start = start; > + fl.fl_end = end; > > for (;;) { > if (filp) { > diff --git a/fs/read_write.c b/fs/read_write.c > index c81ef39..48157dd 100644 > --- a/fs/read_write.c > +++ b/fs/read_write.c > @@ -396,9 +396,8 @@ int rw_verify_area(int read_write, struct file *file, const loff_t *ppos, size_t > } > > if (unlikely(inode->i_flctx && mandatory_lock(inode))) { > - retval = locks_mandatory_area( > - read_write == READ ? FLOCK_VERIFY_READ : FLOCK_VERIFY_WRITE, > - inode, file, pos, count); > + retval = locks_mandatory_area(file, pos, pos + count - 1, > + read_write == READ ? false : true); > if (retval < 0) > return retval; > } > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 870a76e..e640f791 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2030,12 +2030,9 @@ extern struct kobject *fs_kobj; > > #define MAX_RW_COUNT (INT_MAX & PAGE_CACHE_MASK) > > -#define FLOCK_VERIFY_READ 1 > -#define FLOCK_VERIFY_WRITE 2 > - > #ifdef CONFIG_FILE_LOCKING > extern int locks_mandatory_locked(struct file *); > -extern int locks_mandatory_area(int, struct inode *, struct file *, loff_t, size_t); > +extern int locks_mandatory_area(struct file *, loff_t, loff_t, bool); > > /* > * Candidates for mandatory locking have the setgid bit set > @@ -2068,14 +2065,16 @@ static inline int locks_verify_truncate(struct inode *inode, > struct file *filp, > loff_t size) > { > - if (inode->i_flctx && mandatory_lock(inode)) > - return locks_mandatory_area( > - FLOCK_VERIFY_WRITE, inode, filp, > - size < inode->i_size ? size : inode->i_size, > - (size < inode->i_size ? inode->i_size - size > - : size - inode->i_size) > - ); > - return 0; > + if (!inode->i_flctx || !mandatory_lock(inode)) > + return 0; > + > + if (size < inode->i_size) { > + return locks_mandatory_area(filp, size, inode->i_size - 1, > + true); > + } else { > + return locks_mandatory_area(filp, inode->i_size, size - 1, > + true); I feel like these callers would be just slightly more self-documenting if that last parameter was F_WRLCK instead of true. But I could live with it either way, patch looks like an improvement--ACK. --b. > + } > } > > static inline int break_lease(struct inode *inode, unsigned int mode) > @@ -2144,9 +2143,8 @@ static inline int locks_mandatory_locked(struct file *file) > return 0; > } > > -static inline int locks_mandatory_area(int rw, struct inode *inode, > - struct file *filp, loff_t offset, > - size_t count) > +static inline int locks_mandatory_area(struct file *filp, loff_t start, > + loff_t end, bool write) > { > return 0; > } > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 30, 2015 at 05:38:30PM -0500, J. Bruce Fields wrote: > > + if (size < inode->i_size) { > > + return locks_mandatory_area(filp, size, inode->i_size - 1, > > + true); > > + } else { > > + return locks_mandatory_area(filp, inode->i_size, size - 1, > > + true); > > I feel like these callers would be just slightly more self-documenting > if that last parameter was F_WRLCK instead of true. Sure, I can change that forthe next version. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/locks.c b/fs/locks.c index 0d2b326..d503669 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -1227,21 +1227,17 @@ int locks_mandatory_locked(struct file *file) /** * locks_mandatory_area - Check for a conflicting lock - * @read_write: %FLOCK_VERIFY_WRITE for exclusive access, %FLOCK_VERIFY_READ - * for shared - * @inode: the file to check * @filp: how the file was opened (if it was) - * @offset: start of area to check - * @count: length of area to check + * @start: first byte in the file to check + * @end: lastbyte in the file to check + * @write: %true if checking for write access * * Searches the inode's list of locks to find any POSIX locks which conflict. - * This function is called from rw_verify_area() and - * locks_verify_truncate(). */ -int locks_mandatory_area(int read_write, struct inode *inode, - struct file *filp, loff_t offset, - size_t count) +int locks_mandatory_area(struct file *filp, loff_t start, loff_t end, + bool write) { + struct inode *inode = file_inode(filp); struct file_lock fl; int error; bool sleep = false; @@ -1252,9 +1248,9 @@ int locks_mandatory_area(int read_write, struct inode *inode, fl.fl_flags = FL_POSIX | FL_ACCESS; if (filp && !(filp->f_flags & O_NONBLOCK)) sleep = true; - fl.fl_type = (read_write == FLOCK_VERIFY_WRITE) ? F_WRLCK : F_RDLCK; - fl.fl_start = offset; - fl.fl_end = offset + count - 1; + fl.fl_type = write ? F_WRLCK : F_RDLCK; + fl.fl_start = start; + fl.fl_end = end; for (;;) { if (filp) { diff --git a/fs/read_write.c b/fs/read_write.c index c81ef39..48157dd 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -396,9 +396,8 @@ int rw_verify_area(int read_write, struct file *file, const loff_t *ppos, size_t } if (unlikely(inode->i_flctx && mandatory_lock(inode))) { - retval = locks_mandatory_area( - read_write == READ ? FLOCK_VERIFY_READ : FLOCK_VERIFY_WRITE, - inode, file, pos, count); + retval = locks_mandatory_area(file, pos, pos + count - 1, + read_write == READ ? false : true); if (retval < 0) return retval; } diff --git a/include/linux/fs.h b/include/linux/fs.h index 870a76e..e640f791 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2030,12 +2030,9 @@ extern struct kobject *fs_kobj; #define MAX_RW_COUNT (INT_MAX & PAGE_CACHE_MASK) -#define FLOCK_VERIFY_READ 1 -#define FLOCK_VERIFY_WRITE 2 - #ifdef CONFIG_FILE_LOCKING extern int locks_mandatory_locked(struct file *); -extern int locks_mandatory_area(int, struct inode *, struct file *, loff_t, size_t); +extern int locks_mandatory_area(struct file *, loff_t, loff_t, bool); /* * Candidates for mandatory locking have the setgid bit set @@ -2068,14 +2065,16 @@ static inline int locks_verify_truncate(struct inode *inode, struct file *filp, loff_t size) { - if (inode->i_flctx && mandatory_lock(inode)) - return locks_mandatory_area( - FLOCK_VERIFY_WRITE, inode, filp, - size < inode->i_size ? size : inode->i_size, - (size < inode->i_size ? inode->i_size - size - : size - inode->i_size) - ); - return 0; + if (!inode->i_flctx || !mandatory_lock(inode)) + return 0; + + if (size < inode->i_size) { + return locks_mandatory_area(filp, size, inode->i_size - 1, + true); + } else { + return locks_mandatory_area(filp, inode->i_size, size - 1, + true); + } } static inline int break_lease(struct inode *inode, unsigned int mode) @@ -2144,9 +2143,8 @@ static inline int locks_mandatory_locked(struct file *file) return 0; } -static inline int locks_mandatory_area(int rw, struct inode *inode, - struct file *filp, loff_t offset, - size_t count) +static inline int locks_mandatory_area(struct file *filp, loff_t start, + loff_t end, bool write) { return 0; }
Pass a loff_t end for the last byte instead of the 32-bit count parameter to allow full file clones even on 32-bit architectures. While we're at it also drop the pointless inode argument and simplify the read/write selection. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/locks.c | 22 +++++++++------------- fs/read_write.c | 5 ++--- include/linux/fs.h | 28 +++++++++++++--------------- 3 files changed, 24 insertions(+), 31 deletions(-)