On 24/05/22(Tue) 15:24, Mark Kettenis wrote:
> > Date: Tue, 24 May 2022 14:28:39 +0200
> > From: Martin Pieuchot <[email protected]>
> >
> > The softdep code path is missing a UVM cache invalidation compared to
> > the !softdep one. This is necessary to flush pages of a persisting
> > vnode.
> >
> > Since uvm_vnp_setsize() is also called later in this function for the
> > !softdep case move it to not call it twice.
> >
> > ok?
>
> I'm not sure this is correct. I'm trying to understand why you're
> moving the uvm_uvn_setsize() call. Are you just trying to call it
> twice? Or are you trying to avoid calling it at all when we end up in
> an error path?
>
> The way you moved it means we'll still call it twice for "partially
> truncated" files with softdeps. At least the way I understand the
> code is that the code will fsync the vnode and dropping down in the
> "normal" non-softdep code that will call uvm_vnp_setsize() (and
> uvn_vnp_uncache()) again. So maybe you should move the
> uvm_uvn_setsize() call into the else case?
We might want to do that indeed. I'm not sure what are the implications
of calling uvm_vnp_setsize/uncache() after VOP_FSYNC(), which might fail.
So I'd rather play safe and go with that diff.
> > Index: ufs/ffs/ffs_inode.c
> > ===================================================================
> > RCS file: /cvs/src/sys/ufs/ffs/ffs_inode.c,v
> > retrieving revision 1.81
> > diff -u -p -r1.81 ffs_inode.c
> > --- ufs/ffs/ffs_inode.c 12 Dec 2021 09:14:59 -0000 1.81
> > +++ ufs/ffs/ffs_inode.c 4 May 2022 15:32:15 -0000
> > @@ -172,11 +172,12 @@ ffs_truncate(struct inode *oip, off_t le
> > if (length > fs->fs_maxfilesize)
> > return (EFBIG);
> >
> > - uvm_vnp_setsize(ovp, length);
> > oip->i_ci.ci_lasta = oip->i_ci.ci_clen
> > = oip->i_ci.ci_cstart = oip->i_ci.ci_lastw = 0;
> >
> > if (DOINGSOFTDEP(ovp)) {
> > + uvm_vnp_setsize(ovp, length);
> > + (void) uvm_vnp_uncache(ovp);
> > if (length > 0 || softdep_slowdown(ovp)) {
> > /*
> > * If a file is only partially truncated, then
> >
> >
