| Message ID | 20210925085149.429710-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: buffer the truncate when size won't change with Fx caps issued | expand |
On Sat, 2021-09-25 at 16:51 +0800, xiubli@redhat.com wrote: > From: Xiubo Li <xiubli@redhat.com> > > If the new size is the same with current size, the MDS will do nothing > except changing the mtime/atime. We can just buffer the truncate in > this case. > > Signed-off-by: Xiubo Li <xiubli@redhat.com> > --- > fs/ceph/inode.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c > index 03530793c969..14989b961431 100644 > --- a/fs/ceph/inode.c > +++ b/fs/ceph/inode.c > @@ -2370,7 +2370,7 @@ int __ceph_setattr(struct inode *inode, struct iattr *attr, struct ceph_iattr *c > loff_t isize = i_size_read(inode); > > dout("setattr %p size %lld -> %lld\n", inode, isize, attr->ia_size); > - if ((issued & CEPH_CAP_FILE_EXCL) && attr->ia_size > isize) { > + if ((issued & CEPH_CAP_FILE_EXCL) && attr->ia_size >= isize) { > i_size_write(inode, attr->ia_size); > inode->i_blocks = calc_inode_blocks(attr->ia_size); > ci->i_reported_size = attr->ia_size; I wonder if we ought to just ignore the attr->ia_size == isize case altogether instead? Truncating to the same size should be a no-op, so we shouldn't even need to dirty caps or anything. Thoughts?
On 9/30/21 9:30 PM, Jeff Layton wrote: > On Sat, 2021-09-25 at 16:51 +0800, xiubli@redhat.com wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> If the new size is the same with current size, the MDS will do nothing >> except changing the mtime/atime. We can just buffer the truncate in >> this case. >> >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> --- >> fs/ceph/inode.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c >> index 03530793c969..14989b961431 100644 >> --- a/fs/ceph/inode.c >> +++ b/fs/ceph/inode.c >> @@ -2370,7 +2370,7 @@ int __ceph_setattr(struct inode *inode, struct iattr *attr, struct ceph_iattr *c >> loff_t isize = i_size_read(inode); >> >> dout("setattr %p size %lld -> %lld\n", inode, isize, attr->ia_size); >> - if ((issued & CEPH_CAP_FILE_EXCL) && attr->ia_size > isize) { >> + if ((issued & CEPH_CAP_FILE_EXCL) && attr->ia_size >= isize) { >> i_size_write(inode, attr->ia_size); >> inode->i_blocks = calc_inode_blocks(attr->ia_size); >> ci->i_reported_size = attr->ia_size; > I wonder if we ought to just ignore the attr->ia_size == isize case > altogether instead? Truncating to the same size should be a no-op, so we > shouldn't even need to dirty caps or anything. > > Thoughts? I agree with it. Really it's doing nothing except updating the atime/mtime. Currently this patch will just delay doing that. In some filesystems they will ignore it by doing nothing in this case. And some others may will try to release the preallocated blocks in the lower layer in this case, this makes no sense for ceph.
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c index 03530793c969..14989b961431 100644 --- a/fs/ceph/inode.c +++ b/fs/ceph/inode.c @@ -2370,7 +2370,7 @@ int __ceph_setattr(struct inode *inode, struct iattr *attr, struct ceph_iattr *c loff_t isize = i_size_read(inode); dout("setattr %p size %lld -> %lld\n", inode, isize, attr->ia_size); - if ((issued & CEPH_CAP_FILE_EXCL) && attr->ia_size > isize) { + if ((issued & CEPH_CAP_FILE_EXCL) && attr->ia_size >= isize) { i_size_write(inode, attr->ia_size); inode->i_blocks = calc_inode_blocks(attr->ia_size); ci->i_reported_size = attr->ia_size;