| Message ID | 164678220204.1200972.17408022517463940584.stgit@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | netfs: Prep for write helpers | expand |
On Tue, 2022-03-08 at 23:30 +0000, David Howells wrote: > Make afs use netfslib's tracking for the server's idea of what the current > inode size is independently of inode->i_size. We really want to use this > value when calculating the new vnode size when initiating a StoreData RPC > op rather than the size stat() presents to the user (ie. inode->i_size) as > the latter is affected by as-yet uncommitted writes. > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: linux-cachefs@redhat.com > cc: linux-afs@lists.infradead.org > > Link: https://lore.kernel.org/r/164623014626.3564931.8375344024648265358.stgit@warthog.procyon.org.uk/ # v1 > --- > > fs/afs/inode.c | 1 + > fs/afs/write.c | 7 +++---- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/afs/inode.c b/fs/afs/inode.c > index 5b5e40197655..2fe402483ad5 100644 > --- a/fs/afs/inode.c > +++ b/fs/afs/inode.c > @@ -246,6 +246,7 @@ static void afs_apply_status(struct afs_operation *op, > * idea of what the size should be that's not the same as > * what's on the server. > */ > + vnode->netfs_ctx.remote_i_size = status->size; > if (change_size) { > afs_set_i_size(vnode, status->size); > inode->i_ctime = t; > diff --git a/fs/afs/write.c b/fs/afs/write.c > index e4b47f67a408..85c9056ba9fb 100644 > --- a/fs/afs/write.c > +++ b/fs/afs/write.c > @@ -353,9 +353,10 @@ static const struct afs_operation_ops afs_store_data_operation = { > static int afs_store_data(struct afs_vnode *vnode, struct iov_iter *iter, loff_t pos, > bool laundering) > { > + struct netfs_i_context *ictx = &vnode->netfs_ctx; > struct afs_operation *op; > struct afs_wb_key *wbk = NULL; > - loff_t size = iov_iter_count(iter), i_size; > + loff_t size = iov_iter_count(iter); > int ret = -ENOKEY; > > _enter("%s{%llx:%llu.%u},%llx,%llx", > @@ -377,15 +378,13 @@ static int afs_store_data(struct afs_vnode *vnode, struct iov_iter *iter, loff_t > return -ENOMEM; > } > > - i_size = i_size_read(&vnode->vfs_inode); > - > afs_op_set_vnode(op, 0, vnode); > op->file[0].dv_delta = 1; > op->file[0].modification = true; > op->store.write_iter = iter; > op->store.pos = pos; > op->store.size = size; > - op->store.i_size = max(pos + size, i_size); > + op->store.i_size = max(pos + size, ictx->remote_i_size); Ahh ok, so if i_size is larger than is represented by this write, you'll have a zeroed out region until writeback catches up. Makes sense. > op->store.laundering = laundering; > op->mtime = vnode->vfs_inode.i_mtime; > op->flags |= AFS_OPERATION_UNINTR; > > Reviewed-by: Jeff Layton <jlayton@kernel.org>
Jeff Layton <jlayton@kernel.org> wrote: > > - op->store.i_size = max(pos + size, i_size); > > + op->store.i_size = max(pos + size, ictx->remote_i_size); > > Ahh ok, so if i_size is larger than is represented by this write, you'll > have a zeroed out region until writeback catches up. Makes sense. That's the way it was working. With this change, we track the server's idea of the file size separately from our local inode->i_size (which is updated by the modifications into the pagecache) and only expand the server's setting to the end of the data we're storing, not to our local i_size. I'm trying to avoid zeroed-out regions appearing in the file. Forcible expansion by truncate is a different matter. David
diff --git a/fs/afs/inode.c b/fs/afs/inode.c index 5b5e40197655..2fe402483ad5 100644 --- a/fs/afs/inode.c +++ b/fs/afs/inode.c @@ -246,6 +246,7 @@ static void afs_apply_status(struct afs_operation *op, * idea of what the size should be that's not the same as * what's on the server. */ + vnode->netfs_ctx.remote_i_size = status->size; if (change_size) { afs_set_i_size(vnode, status->size); inode->i_ctime = t; diff --git a/fs/afs/write.c b/fs/afs/write.c index e4b47f67a408..85c9056ba9fb 100644 --- a/fs/afs/write.c +++ b/fs/afs/write.c @@ -353,9 +353,10 @@ static const struct afs_operation_ops afs_store_data_operation = { static int afs_store_data(struct afs_vnode *vnode, struct iov_iter *iter, loff_t pos, bool laundering) { + struct netfs_i_context *ictx = &vnode->netfs_ctx; struct afs_operation *op; struct afs_wb_key *wbk = NULL; - loff_t size = iov_iter_count(iter), i_size; + loff_t size = iov_iter_count(iter); int ret = -ENOKEY; _enter("%s{%llx:%llu.%u},%llx,%llx", @@ -377,15 +378,13 @@ static int afs_store_data(struct afs_vnode *vnode, struct iov_iter *iter, loff_t return -ENOMEM; } - i_size = i_size_read(&vnode->vfs_inode); - afs_op_set_vnode(op, 0, vnode); op->file[0].dv_delta = 1; op->file[0].modification = true; op->store.write_iter = iter; op->store.pos = pos; op->store.size = size; - op->store.i_size = max(pos + size, i_size); + op->store.i_size = max(pos + size, ictx->remote_i_size); op->store.laundering = laundering; op->mtime = vnode->vfs_inode.i_mtime; op->flags |= AFS_OPERATION_UNINTR;
Make afs use netfslib's tracking for the server's idea of what the current inode size is independently of inode->i_size. We really want to use this value when calculating the new vnode size when initiating a StoreData RPC op rather than the size stat() presents to the user (ie. inode->i_size) as the latter is affected by as-yet uncommitted writes. Signed-off-by: David Howells <dhowells@redhat.com> cc: linux-cachefs@redhat.com cc: linux-afs@lists.infradead.org Link: https://lore.kernel.org/r/164623014626.3564931.8375344024648265358.stgit@warthog.procyon.org.uk/ # v1 --- fs/afs/inode.c | 1 + fs/afs/write.c | 7 +++---- 2 files changed, 4 insertions(+), 4 deletions(-)