Date: Fri, 15 Jan 2016 21:00:05 -0500 (EST) From: Rick Macklem <rmacklem@uoguelph.ca> To: Konstantin Belousov <kostikbel@gmail.com> Cc: FreeBSD Filesystems <freebsd-fs@freebsd.org>, Kirk McKusick <mckusick@mckusick.com> Subject: Re: panic ffs_truncate3 (maybe fuse being evil) Message-ID: <1817287612.162823118.1452909605928.JavaMail.zimbra@uoguelph.ca> In-Reply-To: <20160115095749.GC3942@kib.kiev.ua> References: <1696608910.154845456.1452438117036.JavaMail.zimbra@uoguelph.ca> <20160110154518.GU3625@kib.kiev.ua> <1773157955.158922767.1452698181137.JavaMail.zimbra@uoguelph.ca> <1351730674.159022044.1452699617235.JavaMail.zimbra@uoguelph.ca> <20160114092934.GL72455@kib.kiev.ua> <964333498.161527381.1452827658163.JavaMail.zimbra@uoguelph.ca> <20160115095749.GC3942@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
Kostik wrote:
> On Thu, Jan 14, 2016 at 10:14:18PM -0500, Rick Macklem wrote:
> > Kostik wrote:
> > > What is the exact value of lblkno ?
> > >
> > It was -1 for the crash I looked at. (Since all are the same backtrace and
> > happen for the same test sequence, I suspect they all are.)
> >
> > > There are two kinds of buffers with negative lblk which may exist on
> > > the UFS vnode queues. One is indirect block, and another is the metadata
> > > block. Metadata lblks are -1 and -2, indir blocks numbers are calculated
> > > by formula you can see in calculation of indir_lbn[] in ffs_truncate().
> > >
> > Thanks. I knew indirect blocks were -ve block #s, but I didn't know about
> > the metadata ones.
> s/metadata/ext attributes data/
>
> Sorry.
>
No problem. Extended attributes are metadata and I suspect there might be other
metadata handled this way someday.
> > So, if I've understood you correctly, a -1 b_lblkno is an extattr one and
> > the assert should allow it and not panic?
> Yes.
>
> >
> > I can change the assert, although it will end up about 6 lines of code;-)
> > If you think I should do this, I will do so and test it.
> It is under INVARIANTS anyway. I do see a value in checking that truncation
> is consistent. We did have many bugs there, which usually caused user data
> loss. Some caution is worth it, IMO.
>
>
I've attached the patch with the modified test for the panic.
I ran a test with the old one (with the panic changed to a printf) just before
this one and the printf happened, but no panic.
After the test, the file system fsck'd ok, so I think this might be ok.
I've added Kirk as a cc. Maybe he can review this?
I can't do commits until mid-April, so maybe you can commit it?
(I don't think there is any rush, since almost everyone will be using soft
updates. To be honest, I have no idea why I turned off soft updates at
some point for this filesystem.)
Thanks for your help, rick
[-- Attachment #2 --]
--- ufs/ffs/ffs_inode.c.sav 2016-01-10 20:11:46.406682000 -0500
+++ ufs/ffs/ffs_inode.c 2016-01-15 17:22:58.465991000 -0500
@@ -544,7 +544,12 @@ done:
BO_LOCK(bo);
if (length == 0 &&
(fs->fs_magic != FS_UFS2_MAGIC || ip->i_din2->di_extsize == 0) &&
- (bo->bo_dirty.bv_cnt > 0 || bo->bo_clean.bv_cnt > 0))
+ ((bo->bo_dirty.bv_cnt > 0 && (TAILQ_EMPTY(&bo->bo_dirty.bv_hd) ||
+ TAILQ_FIRST(&bo->bo_dirty.bv_hd)->b_lblkno >= 0 ||
+ TAILQ_FIRST(&bo->bo_dirty.bv_hd)->b_lblkno < -2)) ||
+ (bo->bo_clean.bv_cnt > 0 && (TAILQ_EMPTY(&bo->bo_clean.bv_hd) ||
+ TAILQ_FIRST(&bo->bo_clean.bv_hd)->b_lblkno >= 0 ||
+ TAILQ_FIRST(&bo->bo_clean.bv_hd)->b_lblkno < -2))))
panic("ffs_truncate3");
BO_UNLOCK(bo);
#endif /* INVARIANTS */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1817287612.162823118.1452909605928.JavaMail.zimbra>