From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bombadil.infradead.org ([198.137.202.9]:45113 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751058AbcFQPGF (ORCPT ); Fri, 17 Jun 2016 11:06:05 -0400 Date: Fri, 17 Jun 2016 08:06:04 -0700 From: Christoph Hellwig To: "Darrick J. Wong" Cc: david@fromorbit.com, linux-fsdevel@vger.kernel.org, vishal.l.verma@intel.com, xfs@oss.sgi.com Subject: Re: [PATCH 006/119] xfs: port differences from xfsprogs libxfs Message-ID: <20160617150604.GA1127@infradead.org> References: <146612627129.12839.3827886950949809165.stgit@birch.djwong.org> <146612631079.12839.13685287438216197909.stgit@birch.djwong.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <146612631079.12839.13685287438216197909.stgit@birch.djwong.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: I think this needs to be split out into a patches, one for each logical change. > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c > index 99b077c..58bdca7 100644 > --- a/fs/xfs/libxfs/xfs_alloc.c > +++ b/fs/xfs/libxfs/xfs_alloc.c > @@ -2415,7 +2415,9 @@ xfs_alloc_read_agf( > be32_to_cpu(agf->agf_levels[XFS_BTNUM_CNTi]); > spin_lock_init(&pag->pagb_lock); > pag->pagb_count = 0; > +#ifdef __KERNEL__ > pag->pagb_tree = RB_ROOT; > +#endif I'd much rather have a dummy tree in libxfs than sprinkling random ifdefs. > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index 932381c..499e980 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -1425,7 +1425,7 @@ xfs_bmap_search_multi_extents( > * Else, *lastxp will be set to the index of the found > * entry; *gotp will contain the entry. > */ > -STATIC xfs_bmbt_rec_host_t * /* pointer to found extent entry */ > +xfs_bmbt_rec_host_t * /* pointer to found extent entry */ > xfs_bmap_search_extents( probably wants a comment that we keep it public for xfsprogs.. > diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c > index 1f88e1c..105979d 100644 > --- a/fs/xfs/libxfs/xfs_btree.c > +++ b/fs/xfs/libxfs/xfs_btree.c > @@ -2532,6 +2532,7 @@ error0: > return error; > } > > +#ifdef __KERNEL__ > struct xfs_btree_split_args { > struct xfs_btree_cur *cur; > int level; > @@ -2609,6 +2610,9 @@ xfs_btree_split( > destroy_work_on_stack(&args.work); > return args.result; > } > +#else /* !KERNEL */ > +#define xfs_btree_split __xfs_btree_split > +#endif I'd really prefer to avoid the ifdefs - can't we rename and move the kernel version that might be a possibility. > @@ -115,7 +115,7 @@ do { \ > __XFS_BTREE_STATS_ADD(__mp, ibt, stat, val); break; \ > case XFS_BTNUM_FINO: \ > __XFS_BTREE_STATS_ADD(__mp, fibt, stat, val); break; \ > - case XFS_BTNUM_MAX: ASSERT(0); /* fucking gcc */ ; break; \ > + case XFS_BTNUM_MAX: ASSERT(0); __mp = __mp /* fucking gcc */ ; break; \ Or add whatever gcc flag we use to silece this one to xfsprogs as well? > index 3cc3cf7..06b574d 100644 > --- a/fs/xfs/libxfs/xfs_dquot_buf.c > +++ b/fs/xfs/libxfs/xfs_dquot_buf.c > @@ -31,10 +31,16 @@ > #include "xfs_cksum.h" > #include "xfs_trace.h" > > +/* > + * XXX: kernel implementation causes ndquots calc to go real > + * bad. Just leaving the existing userspace calc here right now. > + */ > int > xfs_calc_dquots_per_chunk( > unsigned int nbblks) /* basic block units */ > { > +#ifdef __KERNEL__ > + /* kernel code that goes wrong in userspace! */ > unsigned int ndquots; > > ASSERT(nbblks > 0); > @@ -42,6 +48,10 @@ xfs_calc_dquots_per_chunk( > do_div(ndquots, sizeof(xfs_dqblk_t)); > > return ndquots; > +#else > + ASSERT(nbblks > 0); > + return BBTOB(nbblks) / sizeof(xfs_dqblk_t); > +#endif Eww. Can someone explain why we aren't always use the userspace version? Using do_div on a 32-bit variable seems rather pointless. > diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c > index 9d9559e..794fa66 100644 > --- a/fs/xfs/libxfs/xfs_inode_buf.c > +++ b/fs/xfs/libxfs/xfs_inode_buf.c > @@ -56,6 +56,17 @@ xfs_inobp_check( > } > #endif > > +bool > +xfs_dinode_good_version( > + struct xfs_mount *mp, > + __u8 version) > +{ > + if (xfs_sb_version_hascrc(&mp->m_sb)) > + return version == 3; > + > + return version == 1 || version == 2; > +} Odd that this appeared in xfsprogs only.