On Wed, Nov 17, 2021 at 4:13 AM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Tue, 2021-11-16 at 17:20 +0800, xiubli@xxxxxxxxxx wrote:
> > From: Xiubo Li <xiubli@xxxxxxxxxx>
> >
> > In case truncating a file to a smaller sizeA, the sizeA will be kept
> > in truncate_size. And if truncate the file to a bigger sizeB, the
> > MDS will only increase the truncate_seq, but still using the sizeA as
> > the truncate_size.
> >
>
> Do you mean "kept in ci->i_truncate_size" ? If so, is this really the
> correct fix? I'll note this in the sources:
>
> u32 i_truncate_seq; /* last truncate to smaller size */
> u64 i_truncate_size; /* and the size we last truncated down to */
>
> Maybe the MDS ought not bump the truncate_seq unless it was truncating
> to a smaller size? If not, then that comment seems wrong at least.
>
mds does not change truncate_{seq,size} when truncating file to bigger size
>
> > So when filling the inode it will truncate the pagecache by using
> > truncate_sizeA again, which makes no sense and will trim the inocent
> > pages.
> >
>
> Is there a reproducer for this? It would be nice to put something in
> xfstests for it if so.
>
> > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
> > ---
> > fs/ceph/inode.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > index 1b4ce453d397..b4f784684e64 100644
> > --- a/fs/ceph/inode.c
> > +++ b/fs/ceph/inode.c
> > @@ -738,10 +738,11 @@ int ceph_fill_file_size(struct inode *inode, int issued,
> > * don't hold those caps, then we need to check whether
> > * the file is either opened or mmaped
> > */
> > - if ((issued & (CEPH_CAP_FILE_CACHE|
> > + if (ci->i_truncate_size != truncate_size &&
> > + ((issued & (CEPH_CAP_FILE_CACHE|
> > CEPH_CAP_FILE_BUFFER)) ||
> > mapping_mapped(inode->i_mapping) ||
> > - __ceph_is_file_opened(ci)) {
> > + __ceph_is_file_opened(ci))) {
> > ci->i_truncate_pending++;
> > queue_trunc = 1;
> > }
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
