[Top] [All Lists]

Re: [PATCH 25/30 V2] libxfs: fix root inode handling inconsistencies

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 25/30 V2] libxfs: fix root inode handling inconsistencies
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Fri, 1 Nov 2013 06:03:16 -0700
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131031220438.GP4446@dastard>
References: <1383107481-28937-1-git-send-email-david@xxxxxxxxxxxxx> <1383107481-28937-26-git-send-email-david@xxxxxxxxxxxxx> <20131030102318.GA31519@xxxxxxxxxxxxx> <20131030215940.GH6188@dastard> <20131031041343.GK6188@dastard> <20131031150024.GP22359@xxxxxxxxxxxxx> <20131031220438.GP4446@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Fri, Nov 01, 2013 at 09:04:38AM +1100, Dave Chinner wrote:
> > > - if ((flags & LIBXFS_MOUNT_ROOTINOS) && sbp->sb_rootino != NULLFSINO &&
> > > + if (sbp->sb_rootino != NULLFSINO &&
> > >       xfs_sb_version_haslazysbcount(&mp->m_sb)) {
> > 
> > Oh, I hadn't noticed that LIBXFS_MOUNT_ROOTINOS does more than reading
> > the root inode.  Seems like mkfs might need the rt inodes if a file with
> > the rt flag is specified in the proto file.
> mkfs always allocates the rt inodes directly after the root
> directory is created.

True, we obviosuly create them in mkfs.  In fact looking over the code
the only call into the rtalloc code in userspace ever is mkfs
initializing those using libxfs_rtfree_extent.

So removing the call to rtmount_inodes is fine, and we should remove
the declaration of it as well as it's now unused.  In the future I
suspect we should simplify the mkfs code to not even require
xfs_rtalloc.c for just marking the inodes as entirely free, but that's
not something for this series.

> > I can't see how xfs_copy could need either the rt inodes nor the perag
> > data.
> Right, it doesn't need them, but it doesn't hurt at all to
> initialise them because all the ag headers are about to be read to
> find all the used space, anyway.

xfs_initialize_perag_data does the following:

 - read in AGI/AGF.  This is something we lazily do whenever we need it
   anyway, so no one should rely on it.
 - update the in-core superblock global counters.  Seems like the old
   xfs_check relies on this and still needs an equivalent if we care
   enough.  No one else seems to care.

<Prev in Thread] Current Thread [Next in Thread>