On Mon, Jan 14, 2013 at 04:56:29PM -0600, Ben Myers wrote:
> Hi Dave,
>
> On Mon, Jan 14, 2013 at 06:16:58PM +1100, Dave Chinner wrote:
> > Here's a first cut at bringing the userspace code in sync with the
> > current kernel code. This patch addresses libxfs and getting
> > everything to work with that update. This is currently one titanic
> > patch because this sync has been an utter nightmare to get done, and
> > I've been working on it since before christmas just to get it this
> > far. The diffstat should give some context to the size of the sync:
> >
> > 83 files changed, 8142 insertions(+), 7092 deletions(-)
> >
> > Firstly, next time *anyone* rearranges kernel code to "make it
> > easier to sync with userspace" must also change the userspace code
> > at the same time. The mess that the dir2 header re-arrangement made
> > of this sync was terrible, and it appears to be far from optimal to
> > minimise userspace sync issues. Hint: xfs_db, xfs_repair and
> > metadump use lots of the kernel code to parse and modify directory
> > structures. Given that the dir2 code was also turned upside down by
> > the removal of the dabuf infrastructure and replacement with
> > discontiguous xfs_bufs, this made for a real nightmare.
> >
> > Secondly, I've brought across the verifiers into the userspace code,
> > but they are not yet hooked up to the IO infrastructure. I'll need
> > to do that as part of introducing the CRC infrastructure to
> > userspace, as we'll still need all that sort of checking to be done
> > regardless of how the metadta is read or written...
> >
> > Finally, some notes on things that would make it easier to do syncs
> > in future by making the contents of the files shared between user
> > and kernel space contain less kernel only code:
> >
> > - readdir/getdents is not used in userspace. Move all the
> > kernel readdir code to xfs_dir2_readdir.c out of the core
> > xfs_dir2_{sf,block,node,data}.c files
> > - sync regularly. i.e. at the end of every kernel cycle.
> > - removal of attributes is not done in userspace. Move all
> > of the attr removal code to a kernel only file.
> > - listing of attributes is not done in userspace. Move all
> > of the attr listing code to a kernel only file.
> > - move all the non-mount related code out of xfs_mount.c as
> > very little of the kernel mount.c is used in userspace
> > - in-core superblock code into own file
> > - other superblock stuff into own file
> > - per-ag infrastructure into own file
> > - sync regularly. i.e. at the end of every kernel cycle.
> > - move extent tree stuff in xfs_inode.c into own file as it
> > is not used in userspace
> > - move inode flushing into own file - userspace has it's won
> > versions
> > - move inode locking into own file - userspace doesn't use
> > locking.
> > - sync regularly. i.e. at the end of every kernel cycle.
> > - work out how to handle static inline functions more
> > cleanly as userspace has need of inline functions inside
> > __KERNEL__ regions but they can't be included from the
> > header files because the structure definition in libxfs.h
> > requires definitions from the header file which includes
> > the static inline function which then fails to compile
> > because the structure is not defined. e.g. xfs_inode and
> > xfs_get/set_projid.
> > - make sure kernel/user functions pass the same parameters.
> > e.g. xfs_get/set_projid require xfs_icdinode to be passed
> > because they are called from xfs_db/xfs_repair which don't
> > have a struct xfs_inode context, while the kernel versions
> > pass struct xfs_inode....
> > - kernel infrastructure changes tend to have major impact
> > on the userspace code - changes to the userspace code
> > should be done at the same time.
> > - sync regularly. i.e. at the end of every kernel cycle.
> >
> >
> > This patch, despite it's size and complexity, seems to run through
> > xfstests without any obvious regressions. I've run with standard 4k
> > block sizes, 4k block/64k dirblock size and 512byte block/32k
> > directory block sizes without unexpected issues on a 3.8-rc2 kernel.
> > I could probably split this huge patch up into a couple of patches
> > (one to do all the discontiguous buffer support, one for all the
> > dir2 header changes and fixing the fallout, and one for the rest),
> > but right now I don't have time for that so I figured I just make
> > people aware that they are going to nee dto put some time aside for
> > review in the not too distant future...
> >
> > The next step in this work is bring across the kernel CRC
> > infrastructure so libxlog can be updated to match the kernel code.
> > Once that is in place, I'll then add all the CRC on-disk format
> > changes and I can start to test the kernel changes I have. I'm
> > hoping to have all the kernel changes ready for review by the end of
> > next week in the hope that we can get the kernel changes reviewd for
> > the 3.9 merge cycle...
>
> I'm really looking forward to seeing this code, but I don't think we should
> rush into this. If the few weeks between now and the merge window is enough
> time to get this thoroughly reviewed and tested, that's awesome. But we'll
> just have to see when it's posted.
It'll be marked as experimental, just like the initial delayed
logging enhancement was, so it's not critical that it is perfect
first time through. I'm working under a pretty tight deadline here,
and the kernel code is what I'm really concerned about - the userspace
code simply has to work well enough initially to be able to test the
kernel changes. I'd suggest that mostly ignoring the userspace code
right now and concentrating on the reviewing the kernel code when it
is posted is the best use of review and testing time right now..
> > Anyway, patch is below. Feedback and testing welcome.
>
> I would like to see this split up if possible. It is too big to review
> effectively.
That can be done later. I just don't have time to do it now - having
it work is far more important than being pretty or easy to review.
Consider this a heads up for the amount of review and testing work
that is going to be needed over the next couple of months...
FWIW, because of the size of the sync, I'm not going to spend any
significant time breaking it up into fine grained patches - 3 or 4
at most I think. Any finer grained than that is just a waste of time
and effort that coul dbe going to other things.
As it is, the last usr/kernel sync was a bit larger than this, and
it was only split into 3 patches. In terms of review, it was really
only given high level review and then *lots* of testing as a
substitute for fine grained review. It ended up working pretty well
- there were very few regressions introduced - and I suspect that
this is the best approach to take for this sync as well. We've only
just done a release, so we've got some time to get the code synced,
the CRC changes merged and test the hell out of it before we think
about a new release 3 months down the track...
TO avoid this large patch problem in future, we need to restructure
some of the kernel code to make this process easier, smaller and
simpler to verify. That will take time to do, and until it is done
the sync process is going to require painfully large patches....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
