On Fri, Jun 15, 2018 at 07:31:26AM -0400, Brian Foster wrote:
> On Fri, Jun 15, 2018 at 11:43:14AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > A log recovery failure has been reproduced where a symlink inode has
> > a zero length in extent form. It was caused by a shutdown during a
> > combined fstress+fsmark workload.
> >
> > To fix it, we have to allow zero length symlink inodes through
> > xfs_dinode_verify() during log recovery. We already specifically
> > check and allow this case in the shortform symlink fork verifier,
> > but in this case we don't get that far, and the inode is not in
> > shortform format.
> >
> > Update the dinode verifier to handle this case, and change the
> > symlink fork verifier to only allow this case to exist during log
> > recovery.
> >
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > ---
>
> Seems Ok to me, but before we restrict some of the existing checks to
> log recovery I am curious about one thing. xfs_inactive_symlink() has
> this:
>
> /*
> * Zero length symlinks _can_ exist.
> */
> pathlen = (int)ip->i_d.di_size;
> if (!pathlen) {
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
> return 0;
> }
>
> I'm not quite sure what case that covers, but it seems slightly
> inconsistent with the fork verifer change (simply because that path is
> not exclusive to the read from disk case), at least. Any idea?
Yeah, that's what I'm trying to chase down right now. I had the
verifier fire on inode writeback during generic/269. I don't know
yet where these zero length symlinks are coming from, and none of
the comments (there's a couple that say the above)
actually give any hint to their source.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
