Message ID | 20220607112712.18023-1-jlayton@kernel.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | ceph: don't take inode lock in llseek | expand |
On 6/7/22 7:27 PM, Jeff Layton wrote: > There's no reason we need to lock the inode for write in order to handle > an llseek. I suspect this should have been dropped in 2013 when we > stopped doing vmtruncate in llseek. > > Fixes: b0d7c2231015 (ceph: introduce i_truncate_mutex) > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/ceph/file.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > index 0c13a3f23c99..7d2e9615614d 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -1994,8 +1994,6 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int whence) > loff_t i_size; > loff_t ret; > > - inode_lock(inode); > - > if (whence == SEEK_END || whence == SEEK_DATA || whence == SEEK_HOLE) { > ret = ceph_do_getattr(inode, CEPH_STAT_CAP_SIZE, false); > if (ret < 0) > @@ -2038,7 +2036,6 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int whence) > ret = vfs_setpos(file, offset, max(i_size, fsc->max_file_size)); > > out: > - inode_unlock(inode); > return ret; > } > Looks good. It seems the 'out' lable makes no sense anymore ? -- Xiubo
On Tue, 2022-06-07 at 20:23 +0800, Xiubo Li wrote: > On 6/7/22 7:27 PM, Jeff Layton wrote: > > There's no reason we need to lock the inode for write in order to handle > > an llseek. I suspect this should have been dropped in 2013 when we > > stopped doing vmtruncate in llseek. > > > > Fixes: b0d7c2231015 (ceph: introduce i_truncate_mutex) > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > fs/ceph/file.c | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > > index 0c13a3f23c99..7d2e9615614d 100644 > > --- a/fs/ceph/file.c > > +++ b/fs/ceph/file.c > > @@ -1994,8 +1994,6 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int whence) > > loff_t i_size; > > loff_t ret; > > > > - inode_lock(inode); > > - > > if (whence == SEEK_END || whence == SEEK_DATA || whence == SEEK_HOLE) { > > ret = ceph_do_getattr(inode, CEPH_STAT_CAP_SIZE, false); > > if (ret < 0) > > @@ -2038,7 +2036,6 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int whence) > > ret = vfs_setpos(file, offset, max(i_size, fsc->max_file_size)); > > > > out: > > - inode_unlock(inode); > > return ret; > > } > > > > Looks good. > > It seems the 'out' lable makes no sense anymore ? > > -- Xiubo > No, it's still used if the getattr errors out.
On 6/7/22 8:29 PM, Jeff Layton wrote: > On Tue, 2022-06-07 at 20:23 +0800, Xiubo Li wrote: >> On 6/7/22 7:27 PM, Jeff Layton wrote: >>> There's no reason we need to lock the inode for write in order to handle >>> an llseek. I suspect this should have been dropped in 2013 when we >>> stopped doing vmtruncate in llseek. >>> >>> Fixes: b0d7c2231015 (ceph: introduce i_truncate_mutex) >>> Signed-off-by: Jeff Layton <jlayton@kernel.org> >>> --- >>> fs/ceph/file.c | 3 --- >>> 1 file changed, 3 deletions(-) >>> >>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c >>> index 0c13a3f23c99..7d2e9615614d 100644 >>> --- a/fs/ceph/file.c >>> +++ b/fs/ceph/file.c >>> @@ -1994,8 +1994,6 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int whence) >>> loff_t i_size; >>> loff_t ret; >>> >>> - inode_lock(inode); >>> - >>> if (whence == SEEK_END || whence == SEEK_DATA || whence == SEEK_HOLE) { >>> ret = ceph_do_getattr(inode, CEPH_STAT_CAP_SIZE, false); >>> if (ret < 0) >>> @@ -2038,7 +2036,6 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int whence) >>> ret = vfs_setpos(file, offset, max(i_size, fsc->max_file_size)); >>> >>> out: >>> - inode_unlock(inode); >>> return ret; >>> } >>> >> Looks good. >> >> It seems the 'out' lable makes no sense anymore ? >> >> -- Xiubo >> > No, it's still used if the getattr errors out. Yeah, I mean just return the error directly instead of doing 'goto out'.
diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 0c13a3f23c99..7d2e9615614d 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -1994,8 +1994,6 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int whence) loff_t i_size; loff_t ret; - inode_lock(inode); - if (whence == SEEK_END || whence == SEEK_DATA || whence == SEEK_HOLE) { ret = ceph_do_getattr(inode, CEPH_STAT_CAP_SIZE, false); if (ret < 0) @@ -2038,7 +2036,6 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int whence) ret = vfs_setpos(file, offset, max(i_size, fsc->max_file_size)); out: - inode_unlock(inode); return ret; }
There's no reason we need to lock the inode for write in order to handle an llseek. I suspect this should have been dropped in 2013 when we stopped doing vmtruncate in llseek. Fixes: b0d7c2231015 (ceph: introduce i_truncate_mutex) Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/ceph/file.c | 3 --- 1 file changed, 3 deletions(-)