[PATCH 0/6] xfs: miscellaneous libxfs cleanups

[PATCH 0/6] xfs: miscellaneous libxfs cleanups

[Darrick J. Wong]

Hi all,

This is a patchset that aims to fix various discrepancies between the
kernel and xfsprogs' copies of libxfs/.  They shouldn't affect the
behavior of either codebase while bringing the benefit that libxfs-apply
will run more smoothly.

The patches in this series port various libxfs changes into the kernel
codebase.  The penultimate patch refactors the directory freescan code
so that xfs_repair and kernel can use the same code without the macro
wrappers.  

--Darrick

[PATCH 1/6] libxfs: convert ushort to unsigned short

[Darrick J. Wong]

Since xfsprogs dropped ushort in favor of unsigned short, do that
here too.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_ialloc.c      |    4 ++--
 fs/xfs/libxfs/xfs_inode_buf.h   |    4 ++--
 fs/xfs/libxfs/xfs_log_format.h  |    4 ++--
 fs/xfs/libxfs/xfs_log_recover.h |    2 +-
 fs/xfs/xfs_log_recover.c        |    4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 51b4e0d..60e1a67 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -2344,7 +2344,7 @@ xfs_imap(
 
 		imap->im_blkno = XFS_AGB_TO_DADDR(mp, agno, agbno);
 		imap->im_len = XFS_FSB_TO_BB(mp, 1);
-		imap->im_boffset = (ushort)(offset << mp->m_sb.sb_inodelog);
+		imap->im_boffset = (unsigned short)(offset << mp->m_sb.sb_inodelog);
 		return 0;
 	}
 
@@ -2372,7 +2372,7 @@ xfs_imap(
 
 	imap->im_blkno = XFS_AGB_TO_DADDR(mp, agno, cluster_agbno);
 	imap->im_len = XFS_FSB_TO_BB(mp, blks_per_cluster);
-	imap->im_boffset = (ushort)(offset << mp->m_sb.sb_inodelog);
+	imap->im_boffset = (unsigned short)(offset << mp->m_sb.sb_inodelog);
 
 	/*
 	 * If the inode number maps to a block outside the bounds
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index 3cfe12a..a395d0c 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -58,8 +58,8 @@ struct xfs_icdinode {
  */
 struct xfs_imap {
 	xfs_daddr_t	im_blkno;	/* starting BB of inode chunk */
-	ushort		im_len;		/* length in BBs of inode chunk */
-	ushort		im_boffset;	/* inode offset in block in bytes */
+	unsigned short		im_len;		/* length in BBs of inode chunk */
+	unsigned short		im_boffset;	/* inode offset in block in bytes */
 };
 
 int	xfs_imap_to_bp(struct xfs_mount *, struct xfs_trans *,
diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index 083cdd6..3fcea8c 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -481,8 +481,8 @@ static inline uint xfs_log_dinode_size(int version)
 typedef struct xfs_buf_log_format {
 	unsigned short	blf_type;	/* buf log item type indicator */
 	unsigned short	blf_size;	/* size of this item */
-	ushort		blf_flags;	/* misc state */
-	ushort		blf_len;	/* number of blocks in this buf */
+	unsigned short		blf_flags;	/* misc state */
+	unsigned short		blf_len;	/* number of blocks in this buf */
 	__int64_t	blf_blkno;	/* starting blkno of this buf */
 	unsigned int	blf_map_size;	/* used size of data bitmap in words */
 	unsigned int	blf_data_map[XFS_BLF_DATAMAP_SIZE]; /* dirty bitmap */
diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
index 8e385f9..d9f65e2 100644
--- a/fs/xfs/libxfs/xfs_log_recover.h
+++ b/fs/xfs/libxfs/xfs_log_recover.h
@@ -52,7 +52,7 @@ typedef struct xlog_recover {
 	struct list_head	r_itemq;	/* q for items */
 } xlog_recover_t;
 
-#define ITEM_TYPE(i)	(*(ushort *)(i)->ri_buf[0].i_addr)
+#define ITEM_TYPE(i)	(*(unsigned short *)(i)->ri_buf[0].i_addr)
 
 /*
  * This is the number of entries in the l_buf_cancel_table used during
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 9b3d7c7..cf754bc 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2025,7 +2025,7 @@ xlog_peek_buffer_cancelled(
 	struct xlog		*log,
 	xfs_daddr_t		blkno,
 	uint			len,
-	ushort			flags)
+	unsigned short			flags)
 {
 	struct list_head	*bucket;
 	struct xfs_buf_cancel	*bcp;
@@ -2065,7 +2065,7 @@ xlog_check_buffer_cancelled(
 	struct xlog		*log,
 	xfs_daddr_t		blkno,
 	uint			len,
-	ushort			flags)
+	unsigned short			flags)
 {
 	struct xfs_buf_cancel	*bcp;
 

[PATCH 2/6] libxfs: synchronize dinode_verify with userspace

[Darrick J. Wong]

The userspace version of _dinode_verify takes a raw inode number
instead of an inode itself.  Since neither version actually needs
the inode, port the changes to the kernel.  This will also reduce
the libxfs diff noise.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_inode_buf.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 134424f..3a6694b 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -380,10 +380,10 @@ xfs_log_dinode_to_disk(
 	}
 }
 
-static bool
+bool
 xfs_dinode_verify(
 	struct xfs_mount	*mp,
-	struct xfs_inode	*ip,
+	xfs_ino_t		ino,
 	struct xfs_dinode	*dip)
 {
 	uint16_t		flags;
@@ -401,7 +401,7 @@ xfs_dinode_verify(
 	if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
 			      XFS_DINODE_CRC_OFF))
 		return false;
-	if (be64_to_cpu(dip->di_ino) != ip->i_ino)
+	if (be64_to_cpu(dip->di_ino) != ino)
 		return false;
 	if (!uuid_equal(&dip->di_uuid, &mp->m_sb.sb_meta_uuid))
 		return false;
@@ -493,7 +493,7 @@ xfs_iread(
 		return error;
 
 	/* even unallocated inodes are verified */
-	if (!xfs_dinode_verify(mp, ip, dip)) {
+	if (!xfs_dinode_verify(mp, ip->i_ino, dip)) {
 		xfs_alert(mp, "%s: validation failed for inode %lld failed",
 				__func__, ip->i_ino);
 

[PATCH 3/6] libxfs: fix whitespace problems

[Darrick J. Wong]

Fix some whitespace problems that trip up my libxfs-diff script.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_rtbitmap.c |    1 -
 1 file changed, 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
index e2e1106..ea45584 100644
--- a/fs/xfs/libxfs/xfs_rtbitmap.c
+++ b/fs/xfs/libxfs/xfs_rtbitmap.c
@@ -1016,4 +1016,3 @@ xfs_rtfree_extent(
 	}
 	return 0;
 }
-

[PATCH 4/6] libxfs: fix xfs_attr_shortform_bytesfit declaration

[Darrick J. Wong]

Change the xfs_attr_shortform_bytesfit declaration to have
struct xfs_inode to avoid tripping up the libxfs-diff scanner.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_attr_leaf.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
index 4f2aed0..8ef420a 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.h
+++ b/fs/xfs/libxfs/xfs_attr_leaf.h
@@ -51,7 +51,7 @@ int	xfs_attr_shortform_getvalue(struct xfs_da_args *args);
 int	xfs_attr_shortform_to_leaf(struct xfs_da_args *args);
 int	xfs_attr_shortform_remove(struct xfs_da_args *args);
 int	xfs_attr_shortform_allfit(struct xfs_buf *bp, struct xfs_inode *dp);
-int	xfs_attr_shortform_bytesfit(xfs_inode_t *dp, int bytes);
+int	xfs_attr_shortform_bytesfit(struct xfs_inode *dp, int bytes);
 void	xfs_attr_fork_remove(struct xfs_inode *ip, struct xfs_trans *tp);
 
 /*

[PATCH 5/6] libxfs: clean up _dir2_data_freescan

[Darrick J. Wong]

Refactor the implementations of xfs_dir2_data_freescan into a
routine that takes the raw directory block parameters and
a second function that figures out the raw parameters from the
directory inode.  This enables us to use the exact same code
for both userspace and the kernel, since repair knows exactly
which directory block geometry parameters it needs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_dir2.h      |    3 +++
 fs/xfs/libxfs/xfs_dir2_data.c |   24 +++++++++++++++++-------
 2 files changed, 20 insertions(+), 7 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
index becc926..42456de 100644
--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -157,6 +157,9 @@ extern int xfs_dir2_isleaf(struct xfs_da_args *args, int *r);
 extern int xfs_dir2_shrink_inode(struct xfs_da_args *args, xfs_dir2_db_t db,
 				struct xfs_buf *bp);
 
+extern void xfs_dir2_data_freescan_hdr(struct xfs_da_geometry *geo,
+		const struct xfs_dir_ops *ops,
+		struct xfs_dir2_data_hdr *hdr, int *loghead);
 extern void xfs_dir2_data_freescan(struct xfs_inode *dp,
 		struct xfs_dir2_data_hdr *hdr, int *loghead);
 extern void xfs_dir2_data_log_entry(struct xfs_da_args *args,
diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index 725fc78..1729eb2 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -505,8 +505,9 @@ xfs_dir2_data_freeremove(
  * Given a data block, reconstruct its bestfree map.
  */
 void
-xfs_dir2_data_freescan(
-	struct xfs_inode	*dp,
+xfs_dir2_data_freescan_hdr(
+	struct xfs_da_geometry	*geo,
+	const struct xfs_dir_ops *ops,
 	struct xfs_dir2_data_hdr *hdr,
 	int			*loghead)
 {
@@ -516,7 +517,6 @@ xfs_dir2_data_freescan(
 	struct xfs_dir2_data_free *bf;
 	char			*endp;		/* end of block's data */
 	char			*p;		/* current entry pointer */
-	struct xfs_da_geometry	*geo = dp->i_mount->m_dir_geo;
 
 	ASSERT(hdr->magic == cpu_to_be32(XFS_DIR2_DATA_MAGIC) ||
 	       hdr->magic == cpu_to_be32(XFS_DIR3_DATA_MAGIC) ||
@@ -526,13 +526,13 @@ xfs_dir2_data_freescan(
 	/*
 	 * Start by clearing the table.
 	 */
-	bf = dp->d_ops->data_bestfree_p(hdr);
+	bf = ops->data_bestfree_p(hdr);
 	memset(bf, 0, sizeof(*bf) * XFS_DIR2_DATA_FD_COUNT);
 	*loghead = 1;
 	/*
 	 * Set up pointers.
 	 */
-	p = (char *)dp->d_ops->data_entry_p(hdr);
+	p = (char *)ops->data_entry_p(hdr);
 	if (hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC) ||
 	    hdr->magic == cpu_to_be32(XFS_DIR3_BLOCK_MAGIC)) {
 		btp = xfs_dir2_block_tail_p(geo, hdr);
@@ -559,12 +559,22 @@ xfs_dir2_data_freescan(
 		else {
 			dep = (xfs_dir2_data_entry_t *)p;
 			ASSERT((char *)dep - (char *)hdr ==
-			       be16_to_cpu(*dp->d_ops->data_entry_tag_p(dep)));
-			p += dp->d_ops->data_entsize(dep->namelen);
+			       be16_to_cpu(*ops->data_entry_tag_p(dep)));
+			p += ops->data_entsize(dep->namelen);
 		}
 	}
 }
 
+void
+xfs_dir2_data_freescan(
+	struct xfs_inode	*dp,
+	struct xfs_dir2_data_hdr *hdr,
+	int			*loghead)
+{
+	return xfs_dir2_data_freescan_hdr(dp->i_mount->m_dir_geo, dp->d_ops,
+			hdr, loghead);
+}
+
 /*
  * Initialize a data block at the given block number in the directory.
  * Give back the buffer for the created block.

[PATCH 6/6] xfs: don't call xfs_sb_quota_from_disk twice

[Eric Sandeen]

>From ee3754254e8c186c99b6cdd4d59f741759d04acb Mon Sep 17 00:00:00 2001

kernel commit 5ef828c4
xfs: avoid false quotacheck after unclean shutdown

made xfs_sb_from_disk() also call xfs_sb_quota_from_disk
by default.

However, when this was merged to libxfs, existing separate
calls to libxfs_sb_quota_from_disk remained, and calling it
twice in a row on a V4 superblock leads to issues, because:

        if (sbp->sb_qflags & XFS_PQUOTA_ACCT)  {
...
                sbp->sb_pquotino = sbp->sb_gquotino;
                sbp->sb_gquotino = NULLFSINO;

and after the second call, we have set both pquotino and gquotino
to NULLFSINO.

Fix this by making it safe to call twice, and also remove the extra
calls to libxfs_sb_quota_from_disk.

This is only spotted when running xfstests with "-m crc=0" because
the sb_from_disk change came about after V5 became default, and
the above behavior only exists on a V4 superblock.

Reported-by: Eryu Guan <eguan@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
 fs/xfs/libxfs/xfs_sb.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index a70aec9..7a39240 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -338,13 +338,16 @@ xfs_sb_quota_from_disk(struct xfs_sb *sbp)
 					XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
 	sbp->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
 
-	if (sbp->sb_qflags & XFS_PQUOTA_ACCT)  {
+	if (sbp->sb_qflags & XFS_PQUOTA_ACCT &&
+	    sbp->sb_gquotino != NULLFSINO)  {
 		/*
 		 * In older version of superblock, on-disk superblock only
 		 * has sb_gquotino, and in-core superblock has both sb_gquotino
 		 * and sb_pquotino. But, only one of them is supported at any
 		 * point of time. So, if PQUOTA is set in disk superblock,
-		 * copy over sb_gquotino to sb_pquotino.
+		 * copy over sb_gquotino to sb_pquotino.  The NULLFSINO test
+		 * above is to make sure we don't do this twice and wipe them
+		 * both out!
 		 */
 		sbp->sb_pquotino = sbp->sb_gquotino;
 		sbp->sb_gquotino = NULLFSINO;

Re: [PATCH 3/6] libxfs: fix whitespace problems

[Eric Sandeen]

On 11/4/16 1:31 PM, Darrick J. Wong wrote:
> Fix some whitespace problems that trip up my libxfs-diff script.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Ooh ooh I got this one!  ;)

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/libxfs/xfs_rtbitmap.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
> index e2e1106..ea45584 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
> @@ -1016,4 +1016,3 @@ xfs_rtfree_extent(
>  	}
>  	return 0;
>  }
> -
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/6] libxfs: synchronize dinode_verify with userspace

[Eric Sandeen]

On 11/4/16 1:31 PM, Darrick J. Wong wrote:
> The userspace version of _dinode_verify takes a raw inode number
> instead of an inode itself.  Since neither version actually needs
> the inode, port the changes to the kernel.  This will also reduce
> the libxfs diff noise.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_inode_buf.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 134424f..3a6694b 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -380,10 +380,10 @@ xfs_log_dinode_to_disk(
>  	}
>  }
>  
> -static bool
> +bool
>  xfs_dinode_verify(

I believe this can/should be static in both userspace & kernelspace.

Otherwise looks fine to me; not sure why the ip-vs-ino difference
exists, but it's 6 one way half a dozen the other I guess.

-Eric

>  	struct xfs_mount	*mp,
> -	struct xfs_inode	*ip,
> +	xfs_ino_t		ino,
>  	struct xfs_dinode	*dip)
>  {
>  	uint16_t		flags;
> @@ -401,7 +401,7 @@ xfs_dinode_verify(
>  	if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
>  			      XFS_DINODE_CRC_OFF))
>  		return false;
> -	if (be64_to_cpu(dip->di_ino) != ip->i_ino)
> +	if (be64_to_cpu(dip->di_ino) != ino)
>  		return false;
>  	if (!uuid_equal(&dip->di_uuid, &mp->m_sb.sb_meta_uuid))
>  		return false;
> @@ -493,7 +493,7 @@ xfs_iread(
>  		return error;
>  
>  	/* even unallocated inodes are verified */
> -	if (!xfs_dinode_verify(mp, ip, dip)) {
> +	if (!xfs_dinode_verify(mp, ip->i_ino, dip)) {
>  		xfs_alert(mp, "%s: validation failed for inode %lld failed",
>  				__func__, ip->i_ino);
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/6] libxfs: convert ushort to unsigned short

[Eric Sandeen]

On 11/4/16 1:31 PM, Darrick J. Wong wrote:
> Since xfsprogs dropped ushort in favor of unsigned short, do that
> here too.

This pushes 4 lines over 80 chars; can those be fixed up?

> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_ialloc.c      |    4 ++--
>  fs/xfs/libxfs/xfs_inode_buf.h   |    4 ++--
>  fs/xfs/libxfs/xfs_log_format.h  |    4 ++--
>  fs/xfs/libxfs/xfs_log_recover.h |    2 +-
>  fs/xfs/xfs_log_recover.c        |    4 ++--
>  5 files changed, 9 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 51b4e0d..60e1a67 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2344,7 +2344,7 @@ xfs_imap(
>  
>  		imap->im_blkno = XFS_AGB_TO_DADDR(mp, agno, agbno);
>  		imap->im_len = XFS_FSB_TO_BB(mp, 1);
> -		imap->im_boffset = (ushort)(offset << mp->m_sb.sb_inodelog);
> +		imap->im_boffset = (unsigned short)(offset << mp->m_sb.sb_inodelog);

here

>  		return 0;
>  	}
>  
> @@ -2372,7 +2372,7 @@ xfs_imap(
>  
>  	imap->im_blkno = XFS_AGB_TO_DADDR(mp, agno, cluster_agbno);
>  	imap->im_len = XFS_FSB_TO_BB(mp, blks_per_cluster);
> -	imap->im_boffset = (ushort)(offset << mp->m_sb.sb_inodelog);
> +	imap->im_boffset = (unsigned short)(offset << mp->m_sb.sb_inodelog);
>  
>  	/*
>  	 * If the inode number maps to a block outside the bounds
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index 3cfe12a..a395d0c 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -58,8 +58,8 @@ struct xfs_icdinode {
>   */
>  struct xfs_imap {
>  	xfs_daddr_t	im_blkno;	/* starting BB of inode chunk */
> -	ushort		im_len;		/* length in BBs of inode chunk */
> -	ushort		im_boffset;	/* inode offset in block in bytes */
> +	unsigned short		im_len;		/* length in BBs of inode chunk */
> +	unsigned short		im_boffset;	/* inode offset in block in bytes */

these 2

>  };
>  
>  int	xfs_imap_to_bp(struct xfs_mount *, struct xfs_trans *,
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index 083cdd6..3fcea8c 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -481,8 +481,8 @@ static inline uint xfs_log_dinode_size(int version)
>  typedef struct xfs_buf_log_format {
>  	unsigned short	blf_type;	/* buf log item type indicator */
>  	unsigned short	blf_size;	/* size of this item */
> -	ushort		blf_flags;	/* misc state */
> -	ushort		blf_len;	/* number of blocks in this buf */
> +	unsigned short		blf_flags;	/* misc state */
> +	unsigned short		blf_len;	/* number of blocks in this buf */

and here

-Eric

>  	__int64_t	blf_blkno;	/* starting blkno of this buf */
>  	unsigned int	blf_map_size;	/* used size of data bitmap in words */
>  	unsigned int	blf_data_map[XFS_BLF_DATAMAP_SIZE]; /* dirty bitmap */
> diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
> index 8e385f9..d9f65e2 100644
> --- a/fs/xfs/libxfs/xfs_log_recover.h
> +++ b/fs/xfs/libxfs/xfs_log_recover.h
> @@ -52,7 +52,7 @@ typedef struct xlog_recover {
>  	struct list_head	r_itemq;	/* q for items */
>  } xlog_recover_t;
>  
> -#define ITEM_TYPE(i)	(*(ushort *)(i)->ri_buf[0].i_addr)
> +#define ITEM_TYPE(i)	(*(unsigned short *)(i)->ri_buf[0].i_addr)
>  
>  /*
>   * This is the number of entries in the l_buf_cancel_table used during
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 9b3d7c7..cf754bc 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2025,7 +2025,7 @@ xlog_peek_buffer_cancelled(
>  	struct xlog		*log,
>  	xfs_daddr_t		blkno,
>  	uint			len,
> -	ushort			flags)
> +	unsigned short			flags)
>  {
>  	struct list_head	*bucket;
>  	struct xfs_buf_cancel	*bcp;
> @@ -2065,7 +2065,7 @@ xlog_check_buffer_cancelled(
>  	struct xlog		*log,
>  	xfs_daddr_t		blkno,
>  	uint			len,
> -	ushort			flags)
> +	unsigned short			flags)
>  {
>  	struct xfs_buf_cancel	*bcp;
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 4/6] libxfs: fix xfs_attr_shortform_bytesfit declaration

[Eric Sandeen]

On 11/4/16 1:32 PM, Darrick J. Wong wrote:
> Change the xfs_attr_shortform_bytesfit declaration to have
> struct xfs_inode to avoid tripping up the libxfs-diff scanner.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

I s'pose, though the function itself still uses xfs_inode_t ...
for another day I guess.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/libxfs/xfs_attr_leaf.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
> index 4f2aed0..8ef420a 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.h
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.h
> @@ -51,7 +51,7 @@ int	xfs_attr_shortform_getvalue(struct xfs_da_args *args);
>  int	xfs_attr_shortform_to_leaf(struct xfs_da_args *args);
>  int	xfs_attr_shortform_remove(struct xfs_da_args *args);
>  int	xfs_attr_shortform_allfit(struct xfs_buf *bp, struct xfs_inode *dp);
> -int	xfs_attr_shortform_bytesfit(xfs_inode_t *dp, int bytes);
> +int	xfs_attr_shortform_bytesfit(struct xfs_inode *dp, int bytes);
>  void	xfs_attr_fork_remove(struct xfs_inode *ip, struct xfs_trans *tp);
>  
>  /*
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 6/6] xfs: don't call xfs_sb_quota_from_disk twice

[Eric Sandeen]

On 11/4/16 1:32 PM, Eric Sandeen wrote:
>>From ee3754254e8c186c99b6cdd4d59f741759d04acb Mon Sep 17 00:00:00 2001
> 
> kernel commit 5ef828c4
> xfs: avoid false quotacheck after unclean shutdown
> 
> made xfs_sb_from_disk() also call xfs_sb_quota_from_disk
> by default.
> 
> However, when this was merged to libxfs, existing separate
> calls to libxfs_sb_quota_from_disk remained, and calling it
> twice in a row on a V4 superblock leads to issues, because:
> 
>         if (sbp->sb_qflags & XFS_PQUOTA_ACCT)  {
> ...
>                 sbp->sb_pquotino = sbp->sb_gquotino;
>                 sbp->sb_gquotino = NULLFSINO;
> 
> and after the second call, we have set both pquotino and gquotino
> to NULLFSINO.
> 
> Fix this by making it safe to call twice, and also remove the extra
> calls to libxfs_sb_quota_from_disk.
> 
> This is only spotted when running xfstests with "-m crc=0" because
> the sb_from_disk change came about after V5 became default, and
> the above behavior only exists on a V4 superblock.
> 
> Reported-by: Eryu Guan <eguan@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> Signed-off-by: Dave Chinner <david@fromorbit.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/libxfs/xfs_sb.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index a70aec9..7a39240 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -338,13 +338,16 @@ xfs_sb_quota_from_disk(struct xfs_sb *sbp)
>  					XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
>  	sbp->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
>  
> -	if (sbp->sb_qflags & XFS_PQUOTA_ACCT)  {
> +	if (sbp->sb_qflags & XFS_PQUOTA_ACCT &&
> +	    sbp->sb_gquotino != NULLFSINO)  {
>  		/*
>  		 * In older version of superblock, on-disk superblock only
>  		 * has sb_gquotino, and in-core superblock has both sb_gquotino
>  		 * and sb_pquotino. But, only one of them is supported at any
>  		 * point of time. So, if PQUOTA is set in disk superblock,
> -		 * copy over sb_gquotino to sb_pquotino.
> +		 * copy over sb_gquotino to sb_pquotino.  The NULLFSINO test
> +		 * above is to make sure we don't do this twice and wipe them
> +		 * both out!
>  		 */
>  		sbp->sb_pquotino = sbp->sb_gquotino;
>  		sbp->sb_gquotino = NULLFSINO;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/6] libxfs: convert ushort to unsigned short

[Darrick J. Wong]

On Fri, Nov 04, 2016 at 02:09:29PM -0500, Eric Sandeen wrote:
> On 11/4/16 1:31 PM, Darrick J. Wong wrote:
> > Since xfsprogs dropped ushort in favor of unsigned short, do that
> > here too.
> 
> This pushes 4 lines over 80 chars; can those be fixed up?
> 
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/libxfs/xfs_ialloc.c      |    4 ++--
> >  fs/xfs/libxfs/xfs_inode_buf.h   |    4 ++--
> >  fs/xfs/libxfs/xfs_log_format.h  |    4 ++--
> >  fs/xfs/libxfs/xfs_log_recover.h |    2 +-
> >  fs/xfs/xfs_log_recover.c        |    4 ++--
> >  5 files changed, 9 insertions(+), 9 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> > index 51b4e0d..60e1a67 100644
> > --- a/fs/xfs/libxfs/xfs_ialloc.c
> > +++ b/fs/xfs/libxfs/xfs_ialloc.c
> > @@ -2344,7 +2344,7 @@ xfs_imap(
> >  
> >  		imap->im_blkno = XFS_AGB_TO_DADDR(mp, agno, agbno);
> >  		imap->im_len = XFS_FSB_TO_BB(mp, 1);
> > -		imap->im_boffset = (ushort)(offset << mp->m_sb.sb_inodelog);
> > +		imap->im_boffset = (unsigned short)(offset << mp->m_sb.sb_inodelog);

Well xfsprogs is like this too, but I guess I can append another patch...

> here
> 
> >  		return 0;
> >  	}
> >  
> > @@ -2372,7 +2372,7 @@ xfs_imap(
> >  
> >  	imap->im_blkno = XFS_AGB_TO_DADDR(mp, agno, cluster_agbno);
> >  	imap->im_len = XFS_FSB_TO_BB(mp, blks_per_cluster);
> > -	imap->im_boffset = (ushort)(offset << mp->m_sb.sb_inodelog);
> > +	imap->im_boffset = (unsigned short)(offset << mp->m_sb.sb_inodelog);
> >  
> >  	/*
> >  	 * If the inode number maps to a block outside the bounds
> > diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> > index 3cfe12a..a395d0c 100644
> > --- a/fs/xfs/libxfs/xfs_inode_buf.h
> > +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> > @@ -58,8 +58,8 @@ struct xfs_icdinode {
> >   */
> >  struct xfs_imap {
> >  	xfs_daddr_t	im_blkno;	/* starting BB of inode chunk */
> > -	ushort		im_len;		/* length in BBs of inode chunk */
> > -	ushort		im_boffset;	/* inode offset in block in bytes */
> > +	unsigned short		im_len;		/* length in BBs of inode chunk */
> > +	unsigned short		im_boffset;	/* inode offset in block in bytes */
> 
> these 2

Fixed.

> >  };
> >  
> >  int	xfs_imap_to_bp(struct xfs_mount *, struct xfs_trans *,
> > diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> > index 083cdd6..3fcea8c 100644
> > --- a/fs/xfs/libxfs/xfs_log_format.h
> > +++ b/fs/xfs/libxfs/xfs_log_format.h
> > @@ -481,8 +481,8 @@ static inline uint xfs_log_dinode_size(int version)
> >  typedef struct xfs_buf_log_format {
> >  	unsigned short	blf_type;	/* buf log item type indicator */
> >  	unsigned short	blf_size;	/* size of this item */
> > -	ushort		blf_flags;	/* misc state */
> > -	ushort		blf_len;	/* number of blocks in this buf */
> > +	unsigned short		blf_flags;	/* misc state */
> > +	unsigned short		blf_len;	/* number of blocks in this buf */
> 
> and here

Fixed.

--D

> 
> -Eric
> 
> >  	__int64_t	blf_blkno;	/* starting blkno of this buf */
> >  	unsigned int	blf_map_size;	/* used size of data bitmap in words */
> >  	unsigned int	blf_data_map[XFS_BLF_DATAMAP_SIZE]; /* dirty bitmap */
> > diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
> > index 8e385f9..d9f65e2 100644
> > --- a/fs/xfs/libxfs/xfs_log_recover.h
> > +++ b/fs/xfs/libxfs/xfs_log_recover.h
> > @@ -52,7 +52,7 @@ typedef struct xlog_recover {
> >  	struct list_head	r_itemq;	/* q for items */
> >  } xlog_recover_t;
> >  
> > -#define ITEM_TYPE(i)	(*(ushort *)(i)->ri_buf[0].i_addr)
> > +#define ITEM_TYPE(i)	(*(unsigned short *)(i)->ri_buf[0].i_addr)
> >  
> >  /*
> >   * This is the number of entries in the l_buf_cancel_table used during
> > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > index 9b3d7c7..cf754bc 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -2025,7 +2025,7 @@ xlog_peek_buffer_cancelled(
> >  	struct xlog		*log,
> >  	xfs_daddr_t		blkno,
> >  	uint			len,
> > -	ushort			flags)
> > +	unsigned short			flags)
> >  {
> >  	struct list_head	*bucket;
> >  	struct xfs_buf_cancel	*bcp;
> > @@ -2065,7 +2065,7 @@ xlog_check_buffer_cancelled(
> >  	struct xlog		*log,
> >  	xfs_daddr_t		blkno,
> >  	uint			len,
> > -	ushort			flags)
> > +	unsigned short			flags)
> >  {
> >  	struct xfs_buf_cancel	*bcp;
> >  
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] libxfs: synchronize dinode_verify with userspace

[Darrick J. Wong]

On Fri, Nov 04, 2016 at 02:07:27PM -0500, Eric Sandeen wrote:
> On 11/4/16 1:31 PM, Darrick J. Wong wrote:
> > The userspace version of _dinode_verify takes a raw inode number
> > instead of an inode itself.  Since neither version actually needs
> > the inode, port the changes to the kernel.  This will also reduce
> > the libxfs diff noise.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/libxfs/xfs_inode_buf.c |    8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> > index 134424f..3a6694b 100644
> > --- a/fs/xfs/libxfs/xfs_inode_buf.c
> > +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> > @@ -380,10 +380,10 @@ xfs_log_dinode_to_disk(
> >  	}
> >  }
> >  
> > -static bool
> > +bool
> >  xfs_dinode_verify(
> 
> I believe this can/should be static in both userspace & kernelspace.
> 
> Otherwise looks fine to me; not sure why the ip-vs-ino difference
> exists, but it's 6 one way half a dozen the other I guess.

Hmmm, you're right.

--D

> 
> -Eric
> 
> >  	struct xfs_mount	*mp,
> > -	struct xfs_inode	*ip,
> > +	xfs_ino_t		ino,
> >  	struct xfs_dinode	*dip)
> >  {
> >  	uint16_t		flags;
> > @@ -401,7 +401,7 @@ xfs_dinode_verify(
> >  	if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
> >  			      XFS_DINODE_CRC_OFF))
> >  		return false;
> > -	if (be64_to_cpu(dip->di_ino) != ip->i_ino)
> > +	if (be64_to_cpu(dip->di_ino) != ino)
> >  		return false;
> >  	if (!uuid_equal(&dip->di_uuid, &mp->m_sb.sb_meta_uuid))
> >  		return false;
> > @@ -493,7 +493,7 @@ xfs_iread(
> >  		return error;
> >  
> >  	/* even unallocated inodes are verified */
> > -	if (!xfs_dinode_verify(mp, ip, dip)) {
> > +	if (!xfs_dinode_verify(mp, ip->i_ino, dip)) {
> >  		xfs_alert(mp, "%s: validation failed for inode %lld failed",
> >  				__func__, ip->i_ino);
> >  
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] libxfs: clean up _dir2_data_freescan

[Eric Sandeen]

On 11/4/16 1:32 PM, Darrick J. Wong wrote:
> Refactor the implementations of xfs_dir2_data_freescan into a
> routine that takes the raw directory block parameters and
> a second function that figures out the raw parameters from the
> directory inode.  This enables us to use the exact same code
> for both userspace and the kernel, since repair knows exactly
> which directory block geometry parameters it needs.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

seems fine tho I can't say I like the new function name; what
does "_hdr" mean?  They both take a *hdr ...

maybe _xfs_dir2_data_freescan or xfs_dir2_data_freescan_int?

*shrug*

-Eric

> ---
>  fs/xfs/libxfs/xfs_dir2.h      |    3 +++
>  fs/xfs/libxfs/xfs_dir2_data.c |   24 +++++++++++++++++-------
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
> index becc926..42456de 100644
> --- a/fs/xfs/libxfs/xfs_dir2.h
> +++ b/fs/xfs/libxfs/xfs_dir2.h
> @@ -157,6 +157,9 @@ extern int xfs_dir2_isleaf(struct xfs_da_args *args, int *r);
>  extern int xfs_dir2_shrink_inode(struct xfs_da_args *args, xfs_dir2_db_t db,
>  				struct xfs_buf *bp);
>  
> +extern void xfs_dir2_data_freescan_hdr(struct xfs_da_geometry *geo,
> +		const struct xfs_dir_ops *ops,
> +		struct xfs_dir2_data_hdr *hdr, int *loghead);
>  extern void xfs_dir2_data_freescan(struct xfs_inode *dp,
>  		struct xfs_dir2_data_hdr *hdr, int *loghead);
>  extern void xfs_dir2_data_log_entry(struct xfs_da_args *args,
> diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
> index 725fc78..1729eb2 100644
> --- a/fs/xfs/libxfs/xfs_dir2_data.c
> +++ b/fs/xfs/libxfs/xfs_dir2_data.c
> @@ -505,8 +505,9 @@ xfs_dir2_data_freeremove(
>   * Given a data block, reconstruct its bestfree map.
>   */
>  void
> -xfs_dir2_data_freescan(
> -	struct xfs_inode	*dp,
> +xfs_dir2_data_freescan_hdr(
> +	struct xfs_da_geometry	*geo,
> +	const struct xfs_dir_ops *ops,
>  	struct xfs_dir2_data_hdr *hdr,
>  	int			*loghead)
>  {
> @@ -516,7 +517,6 @@ xfs_dir2_data_freescan(
>  	struct xfs_dir2_data_free *bf;
>  	char			*endp;		/* end of block's data */
>  	char			*p;		/* current entry pointer */
> -	struct xfs_da_geometry	*geo = dp->i_mount->m_dir_geo;
>  
>  	ASSERT(hdr->magic == cpu_to_be32(XFS_DIR2_DATA_MAGIC) ||
>  	       hdr->magic == cpu_to_be32(XFS_DIR3_DATA_MAGIC) ||
> @@ -526,13 +526,13 @@ xfs_dir2_data_freescan(
>  	/*
>  	 * Start by clearing the table.
>  	 */
> -	bf = dp->d_ops->data_bestfree_p(hdr);
> +	bf = ops->data_bestfree_p(hdr);
>  	memset(bf, 0, sizeof(*bf) * XFS_DIR2_DATA_FD_COUNT);
>  	*loghead = 1;
>  	/*
>  	 * Set up pointers.
>  	 */
> -	p = (char *)dp->d_ops->data_entry_p(hdr);
> +	p = (char *)ops->data_entry_p(hdr);
>  	if (hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC) ||
>  	    hdr->magic == cpu_to_be32(XFS_DIR3_BLOCK_MAGIC)) {
>  		btp = xfs_dir2_block_tail_p(geo, hdr);
> @@ -559,12 +559,22 @@ xfs_dir2_data_freescan(
>  		else {
>  			dep = (xfs_dir2_data_entry_t *)p;
>  			ASSERT((char *)dep - (char *)hdr ==
> -			       be16_to_cpu(*dp->d_ops->data_entry_tag_p(dep)));
> -			p += dp->d_ops->data_entsize(dep->namelen);
> +			       be16_to_cpu(*ops->data_entry_tag_p(dep)));
> +			p += ops->data_entsize(dep->namelen);
>  		}
>  	}
>  }
>  
> +void
> +xfs_dir2_data_freescan(
> +	struct xfs_inode	*dp,
> +	struct xfs_dir2_data_hdr *hdr,
> +	int			*loghead)
> +{
> +	return xfs_dir2_data_freescan_hdr(dp->i_mount->m_dir_geo, dp->d_ops,
> +			hdr, loghead);
> +}
> +
>  /*
>   * Initialize a data block at the given block number in the directory.
>   * Give back the buffer for the created block.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 5/6] libxfs: clean up _dir2_data_freescan

[Darrick J. Wong]

On Fri, Nov 04, 2016 at 05:58:57PM -0500, Eric Sandeen wrote:
> On 11/4/16 1:32 PM, Darrick J. Wong wrote:
> > Refactor the implementations of xfs_dir2_data_freescan into a
> > routine that takes the raw directory block parameters and
> > a second function that figures out the raw parameters from the
> > directory inode.  This enables us to use the exact same code
> > for both userspace and the kernel, since repair knows exactly
> > which directory block geometry parameters it needs.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> seems fine tho I can't say I like the new function name; what
> does "_hdr" mean?  They both take a *hdr ...
> 
> maybe _xfs_dir2_data_freescan or xfs_dir2_data_freescan_int?

I'll change it to xfs_dir2_data_freescan_geo().

--D

> 
> *shrug*
> 
> -Eric
> 
> > ---
> >  fs/xfs/libxfs/xfs_dir2.h      |    3 +++
> >  fs/xfs/libxfs/xfs_dir2_data.c |   24 +++++++++++++++++-------
> >  2 files changed, 20 insertions(+), 7 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
> > index becc926..42456de 100644
> > --- a/fs/xfs/libxfs/xfs_dir2.h
> > +++ b/fs/xfs/libxfs/xfs_dir2.h
> > @@ -157,6 +157,9 @@ extern int xfs_dir2_isleaf(struct xfs_da_args *args, int *r);
> >  extern int xfs_dir2_shrink_inode(struct xfs_da_args *args, xfs_dir2_db_t db,
> >  				struct xfs_buf *bp);
> >  
> > +extern void xfs_dir2_data_freescan_hdr(struct xfs_da_geometry *geo,
> > +		const struct xfs_dir_ops *ops,
> > +		struct xfs_dir2_data_hdr *hdr, int *loghead);
> >  extern void xfs_dir2_data_freescan(struct xfs_inode *dp,
> >  		struct xfs_dir2_data_hdr *hdr, int *loghead);
> >  extern void xfs_dir2_data_log_entry(struct xfs_da_args *args,
> > diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
> > index 725fc78..1729eb2 100644
> > --- a/fs/xfs/libxfs/xfs_dir2_data.c
> > +++ b/fs/xfs/libxfs/xfs_dir2_data.c
> > @@ -505,8 +505,9 @@ xfs_dir2_data_freeremove(
> >   * Given a data block, reconstruct its bestfree map.
> >   */
> >  void
> > -xfs_dir2_data_freescan(
> > -	struct xfs_inode	*dp,
> > +xfs_dir2_data_freescan_hdr(
> > +	struct xfs_da_geometry	*geo,
> > +	const struct xfs_dir_ops *ops,
> >  	struct xfs_dir2_data_hdr *hdr,
> >  	int			*loghead)
> >  {
> > @@ -516,7 +517,6 @@ xfs_dir2_data_freescan(
> >  	struct xfs_dir2_data_free *bf;
> >  	char			*endp;		/* end of block's data */
> >  	char			*p;		/* current entry pointer */
> > -	struct xfs_da_geometry	*geo = dp->i_mount->m_dir_geo;
> >  
> >  	ASSERT(hdr->magic == cpu_to_be32(XFS_DIR2_DATA_MAGIC) ||
> >  	       hdr->magic == cpu_to_be32(XFS_DIR3_DATA_MAGIC) ||
> > @@ -526,13 +526,13 @@ xfs_dir2_data_freescan(
> >  	/*
> >  	 * Start by clearing the table.
> >  	 */
> > -	bf = dp->d_ops->data_bestfree_p(hdr);
> > +	bf = ops->data_bestfree_p(hdr);
> >  	memset(bf, 0, sizeof(*bf) * XFS_DIR2_DATA_FD_COUNT);
> >  	*loghead = 1;
> >  	/*
> >  	 * Set up pointers.
> >  	 */
> > -	p = (char *)dp->d_ops->data_entry_p(hdr);
> > +	p = (char *)ops->data_entry_p(hdr);
> >  	if (hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC) ||
> >  	    hdr->magic == cpu_to_be32(XFS_DIR3_BLOCK_MAGIC)) {
> >  		btp = xfs_dir2_block_tail_p(geo, hdr);
> > @@ -559,12 +559,22 @@ xfs_dir2_data_freescan(
> >  		else {
> >  			dep = (xfs_dir2_data_entry_t *)p;
> >  			ASSERT((char *)dep - (char *)hdr ==
> > -			       be16_to_cpu(*dp->d_ops->data_entry_tag_p(dep)));
> > -			p += dp->d_ops->data_entsize(dep->namelen);
> > +			       be16_to_cpu(*ops->data_entry_tag_p(dep)));
> > +			p += ops->data_entsize(dep->namelen);
> >  		}
> >  	}
> >  }
> >  
> > +void
> > +xfs_dir2_data_freescan(
> > +	struct xfs_inode	*dp,
> > +	struct xfs_dir2_data_hdr *hdr,
> > +	int			*loghead)
> > +{
> > +	return xfs_dir2_data_freescan_hdr(dp->i_mount->m_dir_geo, dp->d_ops,
> > +			hdr, loghead);
> > +}
> > +
> >  /*
> >   * Initialize a data block at the given block number in the directory.
> >   * Give back the buffer for the created block.
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] libxfs: clean up _dir2_data_freescan

[Eric Sandeen]

On 11/4/16 6:09 PM, Darrick J. Wong wrote:
> On Fri, Nov 04, 2016 at 05:58:57PM -0500, Eric Sandeen wrote:
>> On 11/4/16 1:32 PM, Darrick J. Wong wrote:
>>> Refactor the implementations of xfs_dir2_data_freescan into a
>>> routine that takes the raw directory block parameters and
>>> a second function that figures out the raw parameters from the
>>> directory inode.  This enables us to use the exact same code
>>> for both userspace and the kernel, since repair knows exactly
>>> which directory block geometry parameters it needs.
>>>
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>
>> seems fine tho I can't say I like the new function name; what
>> does "_hdr" mean?  They both take a *hdr ...
>>
>> maybe _xfs_dir2_data_freescan or xfs_dir2_data_freescan_int?
> 
> I'll change it to xfs_dir2_data_freescan_geo().

it gets both geo and ops, both derived from dp ...

see i.e. xfs_btree_init_block, xfs_rtmodify_summary etc for
examples of a simple args translation calling an "*_int()"
(internal?) function.  That's pretty common in the xfs code.

-Eric

> --D
> 
>>
>> *shrug*
>>
>> -Eric
>>
>>> ---
>>>  fs/xfs/libxfs/xfs_dir2.h      |    3 +++
>>>  fs/xfs/libxfs/xfs_dir2_data.c |   24 +++++++++++++++++-------
>>>  2 files changed, 20 insertions(+), 7 deletions(-)
>>>
>>>
>>> diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
>>> index becc926..42456de 100644
>>> --- a/fs/xfs/libxfs/xfs_dir2.h
>>> +++ b/fs/xfs/libxfs/xfs_dir2.h
>>> @@ -157,6 +157,9 @@ extern int xfs_dir2_isleaf(struct xfs_da_args *args, int *r);
>>>  extern int xfs_dir2_shrink_inode(struct xfs_da_args *args, xfs_dir2_db_t db,
>>>  				struct xfs_buf *bp);
>>>  
>>> +extern void xfs_dir2_data_freescan_hdr(struct xfs_da_geometry *geo,
>>> +		const struct xfs_dir_ops *ops,
>>> +		struct xfs_dir2_data_hdr *hdr, int *loghead);
>>>  extern void xfs_dir2_data_freescan(struct xfs_inode *dp,
>>>  		struct xfs_dir2_data_hdr *hdr, int *loghead);
>>>  extern void xfs_dir2_data_log_entry(struct xfs_da_args *args,
>>> diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
>>> index 725fc78..1729eb2 100644
>>> --- a/fs/xfs/libxfs/xfs_dir2_data.c
>>> +++ b/fs/xfs/libxfs/xfs_dir2_data.c
>>> @@ -505,8 +505,9 @@ xfs_dir2_data_freeremove(
>>>   * Given a data block, reconstruct its bestfree map.
>>>   */
>>>  void
>>> -xfs_dir2_data_freescan(
>>> -	struct xfs_inode	*dp,
>>> +xfs_dir2_data_freescan_hdr(
>>> +	struct xfs_da_geometry	*geo,
>>> +	const struct xfs_dir_ops *ops,
>>>  	struct xfs_dir2_data_hdr *hdr,
>>>  	int			*loghead)
>>>  {
>>> @@ -516,7 +517,6 @@ xfs_dir2_data_freescan(
>>>  	struct xfs_dir2_data_free *bf;
>>>  	char			*endp;		/* end of block's data */
>>>  	char			*p;		/* current entry pointer */
>>> -	struct xfs_da_geometry	*geo = dp->i_mount->m_dir_geo;
>>>  
>>>  	ASSERT(hdr->magic == cpu_to_be32(XFS_DIR2_DATA_MAGIC) ||
>>>  	       hdr->magic == cpu_to_be32(XFS_DIR3_DATA_MAGIC) ||
>>> @@ -526,13 +526,13 @@ xfs_dir2_data_freescan(
>>>  	/*
>>>  	 * Start by clearing the table.
>>>  	 */
>>> -	bf = dp->d_ops->data_bestfree_p(hdr);
>>> +	bf = ops->data_bestfree_p(hdr);
>>>  	memset(bf, 0, sizeof(*bf) * XFS_DIR2_DATA_FD_COUNT);
>>>  	*loghead = 1;
>>>  	/*
>>>  	 * Set up pointers.
>>>  	 */
>>> -	p = (char *)dp->d_ops->data_entry_p(hdr);
>>> +	p = (char *)ops->data_entry_p(hdr);
>>>  	if (hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC) ||
>>>  	    hdr->magic == cpu_to_be32(XFS_DIR3_BLOCK_MAGIC)) {
>>>  		btp = xfs_dir2_block_tail_p(geo, hdr);
>>> @@ -559,12 +559,22 @@ xfs_dir2_data_freescan(
>>>  		else {
>>>  			dep = (xfs_dir2_data_entry_t *)p;
>>>  			ASSERT((char *)dep - (char *)hdr ==
>>> -			       be16_to_cpu(*dp->d_ops->data_entry_tag_p(dep)));
>>> -			p += dp->d_ops->data_entsize(dep->namelen);
>>> +			       be16_to_cpu(*ops->data_entry_tag_p(dep)));
>>> +			p += ops->data_entsize(dep->namelen);
>>>  		}
>>>  	}
>>>  }
>>>  
>>> +void
>>> +xfs_dir2_data_freescan(
>>> +	struct xfs_inode	*dp,
>>> +	struct xfs_dir2_data_hdr *hdr,
>>> +	int			*loghead)
>>> +{
>>> +	return xfs_dir2_data_freescan_hdr(dp->i_mount->m_dir_geo, dp->d_ops,
>>> +			hdr, loghead);
>>> +}
>>> +
>>>  /*
>>>   * Initialize a data block at the given block number in the directory.
>>>   * Give back the buffer for the created block.
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 5/6] libxfs: clean up _dir2_data_freescan

[Darrick J. Wong]

On Fri, Nov 04, 2016 at 06:20:22PM -0500, Eric Sandeen wrote:
> On 11/4/16 6:09 PM, Darrick J. Wong wrote:
> > On Fri, Nov 04, 2016 at 05:58:57PM -0500, Eric Sandeen wrote:
> >> On 11/4/16 1:32 PM, Darrick J. Wong wrote:
> >>> Refactor the implementations of xfs_dir2_data_freescan into a
> >>> routine that takes the raw directory block parameters and
> >>> a second function that figures out the raw parameters from the
> >>> directory inode.  This enables us to use the exact same code
> >>> for both userspace and the kernel, since repair knows exactly
> >>> which directory block geometry parameters it needs.
> >>>
> >>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> >>
> >> seems fine tho I can't say I like the new function name; what
> >> does "_hdr" mean?  They both take a *hdr ...
> >>
> >> maybe _xfs_dir2_data_freescan or xfs_dir2_data_freescan_int?
> > 
> > I'll change it to xfs_dir2_data_freescan_geo().
> 
> it gets both geo and ops, both derived from dp ...
> 
> see i.e. xfs_btree_init_block, xfs_rtmodify_summary etc for
> examples of a simple args translation calling an "*_int()"
> (internal?) function.  That's pretty common in the xfs code.

Ok I'll change it.

--D

> 
> -Eric
> 
> > --D
> > 
> >>
> >> *shrug*
> >>
> >> -Eric
> >>
> >>> ---
> >>>  fs/xfs/libxfs/xfs_dir2.h      |    3 +++
> >>>  fs/xfs/libxfs/xfs_dir2_data.c |   24 +++++++++++++++++-------
> >>>  2 files changed, 20 insertions(+), 7 deletions(-)
> >>>
> >>>
> >>> diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
> >>> index becc926..42456de 100644
> >>> --- a/fs/xfs/libxfs/xfs_dir2.h
> >>> +++ b/fs/xfs/libxfs/xfs_dir2.h
> >>> @@ -157,6 +157,9 @@ extern int xfs_dir2_isleaf(struct xfs_da_args *args, int *r);
> >>>  extern int xfs_dir2_shrink_inode(struct xfs_da_args *args, xfs_dir2_db_t db,
> >>>  				struct xfs_buf *bp);
> >>>  
> >>> +extern void xfs_dir2_data_freescan_hdr(struct xfs_da_geometry *geo,
> >>> +		const struct xfs_dir_ops *ops,
> >>> +		struct xfs_dir2_data_hdr *hdr, int *loghead);
> >>>  extern void xfs_dir2_data_freescan(struct xfs_inode *dp,
> >>>  		struct xfs_dir2_data_hdr *hdr, int *loghead);
> >>>  extern void xfs_dir2_data_log_entry(struct xfs_da_args *args,
> >>> diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
> >>> index 725fc78..1729eb2 100644
> >>> --- a/fs/xfs/libxfs/xfs_dir2_data.c
> >>> +++ b/fs/xfs/libxfs/xfs_dir2_data.c
> >>> @@ -505,8 +505,9 @@ xfs_dir2_data_freeremove(
> >>>   * Given a data block, reconstruct its bestfree map.
> >>>   */
> >>>  void
> >>> -xfs_dir2_data_freescan(
> >>> -	struct xfs_inode	*dp,
> >>> +xfs_dir2_data_freescan_hdr(
> >>> +	struct xfs_da_geometry	*geo,
> >>> +	const struct xfs_dir_ops *ops,
> >>>  	struct xfs_dir2_data_hdr *hdr,
> >>>  	int			*loghead)
> >>>  {
> >>> @@ -516,7 +517,6 @@ xfs_dir2_data_freescan(
> >>>  	struct xfs_dir2_data_free *bf;
> >>>  	char			*endp;		/* end of block's data */
> >>>  	char			*p;		/* current entry pointer */
> >>> -	struct xfs_da_geometry	*geo = dp->i_mount->m_dir_geo;
> >>>  
> >>>  	ASSERT(hdr->magic == cpu_to_be32(XFS_DIR2_DATA_MAGIC) ||
> >>>  	       hdr->magic == cpu_to_be32(XFS_DIR3_DATA_MAGIC) ||
> >>> @@ -526,13 +526,13 @@ xfs_dir2_data_freescan(
> >>>  	/*
> >>>  	 * Start by clearing the table.
> >>>  	 */
> >>> -	bf = dp->d_ops->data_bestfree_p(hdr);
> >>> +	bf = ops->data_bestfree_p(hdr);
> >>>  	memset(bf, 0, sizeof(*bf) * XFS_DIR2_DATA_FD_COUNT);
> >>>  	*loghead = 1;
> >>>  	/*
> >>>  	 * Set up pointers.
> >>>  	 */
> >>> -	p = (char *)dp->d_ops->data_entry_p(hdr);
> >>> +	p = (char *)ops->data_entry_p(hdr);
> >>>  	if (hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC) ||
> >>>  	    hdr->magic == cpu_to_be32(XFS_DIR3_BLOCK_MAGIC)) {
> >>>  		btp = xfs_dir2_block_tail_p(geo, hdr);
> >>> @@ -559,12 +559,22 @@ xfs_dir2_data_freescan(
> >>>  		else {
> >>>  			dep = (xfs_dir2_data_entry_t *)p;
> >>>  			ASSERT((char *)dep - (char *)hdr ==
> >>> -			       be16_to_cpu(*dp->d_ops->data_entry_tag_p(dep)));
> >>> -			p += dp->d_ops->data_entsize(dep->namelen);
> >>> +			       be16_to_cpu(*ops->data_entry_tag_p(dep)));
> >>> +			p += ops->data_entsize(dep->namelen);
> >>>  		}
> >>>  	}
> >>>  }
> >>>  
> >>> +void
> >>> +xfs_dir2_data_freescan(
> >>> +	struct xfs_inode	*dp,
> >>> +	struct xfs_dir2_data_hdr *hdr,
> >>> +	int			*loghead)
> >>> +{
> >>> +	return xfs_dir2_data_freescan_hdr(dp->i_mount->m_dir_geo, dp->d_ops,
> >>> +			hdr, loghead);
> >>> +}
> >>> +
> >>>  /*
> >>>   * Initialize a data block at the given block number in the directory.
> >>>   * Give back the buffer for the created block.
> >>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 

Re: [PATCH 6/6] xfs: don't call xfs_sb_quota_from_disk twice

[Dave Chinner]

On Fri, Nov 04, 2016 at 11:32:14AM -0700, Eric Sandeen wrote:
> >From ee3754254e8c186c99b6cdd4d59f741759d04acb Mon Sep 17 00:00:00 2001
> 
> kernel commit 5ef828c4
> xfs: avoid false quotacheck after unclean shutdown
> 
> made xfs_sb_from_disk() also call xfs_sb_quota_from_disk
> by default.

This is another of those confusing commit messages. It references
kernel commits, but does not make it clear that this is a fix first
applied to the userspace and is not being synced back to the kernel....

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com