[RFC PATCH 00/13] Add parent pointer attributes

[RFC PATCH 00/13] Add parent pointer attributes

[Allison Henderson]

Hi all,

This is my first pass at parent pointer attributes for xfs.  It's also
my first patch set for xfs in general, so I would very much appreciate
the feedback.  I got in touch with Brian Foster a while back, who was 
gracious enough to give me what he had done thus far with it.  From 
there I've finished out the remove and rename routines.  This 
implementation uses the following on disk format:

name={parent inode, parent inode gen, dirent offset}
value={dirent filename}

I know folks have a lot of opinions about the format, so I'd
like to get peoples thoughts on it now that we have some code
to look at and discuss.  Thanks!

Questions, comments and feedback are welcome!

Allison Henderson


Allison Henderson (3):
  Add the extra space requirements for parent pointer attributes when
    calculating the minimum log size during mkfs
  Add parent pointers to rename
  Add the parent pointer support to the superblock version 5.

Brian Foster (1):
  xfs_bmap_add_attrfork(): re-add error handling from set_attrforkoff()
    call

Dave Chinner (5):
  xfs: define parent pointer xattr format
  :xfs: extent transaction reservations for parent attributes
  xfs: parent pointer attribute creation
  xfs: add parent attributes to link
  xfs: remove parent pointers in unlink

Mark Tinguely (4):
  xfs: get directory offset when adding directory name
  xfs: get directory offset when removing directory name
  xfs: get directory offset when replacing a directory name
  xfs: add parent pointer support to attribute code

 fs/xfs/Makefile                |   1 +
 fs/xfs/libxfs/xfs_attr.c       | 382 +++++++++++++++++++++++++++++++----------
 fs/xfs/libxfs/xfs_bmap.c       |  51 +++---
 fs/xfs/libxfs/xfs_bmap.h       |   1 +
 fs/xfs/libxfs/xfs_da_btree.h   |   1 +
 fs/xfs/libxfs/xfs_da_format.h  |  12 +-
 fs/xfs/libxfs/xfs_dir2.c       |  41 +++--
 fs/xfs/libxfs/xfs_dir2.h       |   9 +-
 fs/xfs/libxfs/xfs_dir2_block.c |   9 +-
 fs/xfs/libxfs/xfs_dir2_leaf.c  |   8 +-
 fs/xfs/libxfs/xfs_dir2_node.c  |   8 +-
 fs/xfs/libxfs/xfs_dir2_sf.c    |   6 +
 fs/xfs/libxfs/xfs_format.h     |  43 ++++-
 fs/xfs/libxfs/xfs_fs.h         |   1 +
 fs/xfs/libxfs/xfs_log_rlimit.c |  34 ++++
 fs/xfs/libxfs/xfs_parent.c     | 163 ++++++++++++++++++
 fs/xfs/libxfs/xfs_trans_resv.c | 103 ++++++++---
 fs/xfs/xfs_attr.h              |  30 ++++
 fs/xfs/xfs_fsops.c             |   4 +-
 fs/xfs/xfs_inode.c             | 146 ++++++++++++----
 fs/xfs/xfs_qm.c                |   2 +-
 fs/xfs/xfs_qm.h                |   1 +
 fs/xfs/xfs_symlink.c           |   2 +-
 23 files changed, 850 insertions(+), 208 deletions(-)
 create mode 100644 fs/xfs/libxfs/xfs_parent.c

-- 
2.7.4

[RFC PATCH 01/13] xfs: get directory offset when adding directory name

[Allison Henderson]

From: Mark Tinguely <tinguely@sgi.com>

Return the directory offset information when adding an entry to the
directory.

This offset will be used as the parent pointer offset in xfs_create,
xfs_symlink, xfs_link and xfs_rename.

[dchinner: forward ported and cleaned up]
[dchinner: no s-o-b from Mark]
[bfoster: rebased, use args->geo in dir code]
[achender: rebased, chaged __uint32_t to xfs_dir2_dataptr_t]

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 ae6de17... bce96d6... M	fs/xfs/libxfs/xfs_da_btree.h
:100644 100644 ccf9783... a1ca460... M	fs/xfs/libxfs/xfs_dir2.c
:100644 100644 21c8f8b... e349900... M	fs/xfs/libxfs/xfs_dir2.h
:100644 100644 43c902f... 79684d5... M	fs/xfs/libxfs/xfs_dir2_block.c
:100644 100644 27297a6... 2ac7a7e... M	fs/xfs/libxfs/xfs_dir2_leaf.c
:100644 100644 682e2bf... 8bc91f8... M	fs/xfs/libxfs/xfs_dir2_node.c
:100644 100644 be8b975... 489bdef... M	fs/xfs/libxfs/xfs_dir2_sf.c
:100644 100644 ceef77c... cea1c56... M	fs/xfs/xfs_inode.c
:100644 100644 23a50d7... f0c3fb5... M	fs/xfs/xfs_symlink.c
 fs/xfs/libxfs/xfs_da_btree.h   | 1 +
 fs/xfs/libxfs/xfs_dir2.c       | 8 ++++++--
 fs/xfs/libxfs/xfs_dir2.h       | 3 ++-
 fs/xfs/libxfs/xfs_dir2_block.c | 1 +
 fs/xfs/libxfs/xfs_dir2_leaf.c  | 2 ++
 fs/xfs/libxfs/xfs_dir2_node.c  | 2 ++
 fs/xfs/libxfs/xfs_dir2_sf.c    | 2 ++
 fs/xfs/xfs_inode.c             | 9 +++++----
 fs/xfs/xfs_symlink.c           | 2 +-
 9 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index ae6de17..bce96d6 100644
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -86,6 +86,7 @@ typedef struct xfs_da_args {
 	int		rmtvaluelen2;	/* remote attr value length in bytes */
 	int		op_flags;	/* operation flags */
 	enum xfs_dacmp	cmpresult;	/* name compare result for lookups */
+	xfs_dir2_dataptr_t offset;	/* OUT: offset in directory */
 } xfs_da_args_t;
 
 /*
diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index ccf9783..a1ca460 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -268,7 +268,8 @@ xfs_dir_createname(
 	xfs_ino_t		inum,		/* new entry inode number */
 	xfs_fsblock_t		*first,		/* bmap's firstblock */
 	struct xfs_defer_ops	*dfops,		/* bmap's freeblock list */
-	xfs_extlen_t		total)		/* bmap's total block count */
+	xfs_extlen_t		total,		/* bmap's total block count */
+	xfs_dir2_dataptr_t	*offset)	/* OUT entry's dir offset */
 {
 	struct xfs_da_args	*args;
 	int			rval;
@@ -323,6 +324,9 @@ xfs_dir_createname(
 	else
 		rval = xfs_dir2_node_addname(args);
 
+	/* return the location that this entry was place in the parent inode */
+	if (offset)
+		*offset = args->offset;
 out_free:
 	kmem_free(args);
 	return rval;
@@ -570,7 +574,7 @@ xfs_dir_canenter(
 	xfs_inode_t	*dp,
 	struct xfs_name	*name)		/* name of entry to add */
 {
-	return xfs_dir_createname(tp, dp, name, 0, NULL, NULL, 0);
+	return xfs_dir_createname(tp, dp, name, 0, NULL, NULL, 0, NULL);
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
index 21c8f8b..e349900 100644
--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -131,7 +131,8 @@ extern int xfs_dir_init(struct xfs_trans *tp, struct xfs_inode *dp,
 extern int xfs_dir_createname(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name, xfs_ino_t inum,
 				xfs_fsblock_t *first,
-				struct xfs_defer_ops *dfops, xfs_extlen_t tot);
+				struct xfs_defer_ops *dfops, xfs_extlen_t tot,
+				xfs_dir2_dataptr_t *offset);
 extern int xfs_dir_lookup(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name, xfs_ino_t *inum,
 				struct xfs_name *ci_name);
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index 43c902f..79684d5 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -552,6 +552,7 @@ xfs_dir2_block_addname(
 	dp->d_ops->data_put_ftype(dep, args->filetype);
 	tagp = dp->d_ops->data_entry_tag_p(dep);
 	*tagp = cpu_to_be16((char *)dep - (char *)hdr);
+	args->offset = xfs_dir2_byte_to_dataptr((char *)dep - (char *)hdr);
 	/*
 	 * Clean up the bestfree array and log the header, tail, and entry.
 	 */
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index 27297a6..2ac7a7e 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -863,6 +863,8 @@ xfs_dir2_leaf_addname(
 	dp->d_ops->data_put_ftype(dep, args->filetype);
 	tagp = dp->d_ops->data_entry_tag_p(dep);
 	*tagp = cpu_to_be16((char *)dep - (char *)hdr);
+	args->offset = xfs_dir2_db_off_to_dataptr(args->geo, use_block,
+						(char *)dep - (char *)hdr);
 	/*
 	 * Need to scan fix up the bestfree table.
 	 */
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 682e2bf..8bc91f8 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -2022,6 +2022,8 @@ xfs_dir2_node_addname_int(
 	dp->d_ops->data_put_ftype(dep, args->filetype);
 	tagp = dp->d_ops->data_entry_tag_p(dep);
 	*tagp = cpu_to_be16((char *)dep - (char *)hdr);
+	args->offset = xfs_dir2_db_off_to_dataptr(args->geo, dbno,
+						  (char *)dep - (char *)hdr);
 	xfs_dir2_data_log_entry(args, dbp, dep);
 	/*
 	 * Rescan the block for bestfree if needed.
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index be8b975..489bdef 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -407,6 +407,7 @@ xfs_dir2_sf_addname_easy(
 	memcpy(sfep->name, args->name, sfep->namelen);
 	dp->d_ops->sf_put_ino(sfp, sfep, args->inumber);
 	dp->d_ops->sf_put_ftype(sfep, args->filetype);
+	args->offset = xfs_dir2_byte_to_dataptr(offset);
 
 	/*
 	 * Update the header and inode.
@@ -498,6 +499,7 @@ xfs_dir2_sf_addname_hard(
 	memcpy(sfep->name, args->name, sfep->namelen);
 	dp->d_ops->sf_put_ino(sfp, sfep, args->inumber);
 	dp->d_ops->sf_put_ftype(sfep, args->filetype);
+	args->offset = xfs_dir2_byte_to_dataptr(offset);
 	sfp->count++;
 	if (args->inumber > XFS_DIR2_MAX_SHORT_INUM && !objchange)
 		sfp->i8count++;
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index ceef77c..cea1c56 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1250,7 +1250,8 @@ xfs_create(
 
 	error = xfs_dir_createname(tp, dp, name, ip->i_ino,
 					&first_block, &dfops, resblks ?
-					resblks - XFS_IALLOC_SPACE_RES(mp) : 0);
+					resblks - XFS_IALLOC_SPACE_RES(mp) : 0,
+					NULL);
 	if (error) {
 		ASSERT(error != -ENOSPC);
 		goto out_trans_cancel;
@@ -1493,7 +1494,7 @@ xfs_link(
 	}
 
 	error = xfs_dir_createname(tp, tdp, target_name, sip->i_ino,
-					&first_block, &dfops, resblks);
+				   &first_block, &dfops, resblks, NULL);
 	if (error)
 		goto error_return;
 	xfs_trans_ichgtime(tp, tdp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
@@ -3013,8 +3014,8 @@ xfs_rename(
 		 * to account for the ".." reference from the new entry.
 		 */
 		error = xfs_dir_createname(tp, target_dp, target_name,
-						src_ip->i_ino, &first_block,
-						&dfops, spaceres);
+					   src_ip->i_ino, &first_block, &dfops,
+					   spaceres, NULL);
 		if (error)
 			goto out_bmap_cancel;
 
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 23a50d7..f0c3fb5 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -363,7 +363,7 @@ xfs_symlink(
 	 * Create the directory entry for the symlink.
 	 */
 	error = xfs_dir_createname(tp, dp, link_name, ip->i_ino,
-					&first_block, &dfops, resblks);
+				   &first_block, &dfops, resblks, NULL);
 	if (error)
 		goto out_bmap_cancel;
 	xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-- 
2.7.4

[RFC PATCH 02/13] xfs: get directory offset when removing directory name

[Allison Henderson]

From: Mark Tinguely <tinguely@sgi.com>

Return the directory offset information when removing an entry to the
directory.

This offset will be used as the parent pointer offset in xfs_remove.

[dchinner: forward ported and cleaned up]
[achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t]

Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 a1ca460... a34956e... M	fs/xfs/libxfs/xfs_dir2.c
:100644 100644 e349900... ee98b2a... M	fs/xfs/libxfs/xfs_dir2.h
:100644 100644 79684d5... 4dbe2fc... M	fs/xfs/libxfs/xfs_dir2_block.c
:100644 100644 2ac7a7e... 197e627... M	fs/xfs/libxfs/xfs_dir2_leaf.c
:100644 100644 8bc91f8... 13d5244... M	fs/xfs/libxfs/xfs_dir2_node.c
:100644 100644 489bdef... 9e90c22... M	fs/xfs/libxfs/xfs_dir2_sf.c
:100644 100644 cea1c56... f58f62a... M	fs/xfs/xfs_inode.c
 fs/xfs/libxfs/xfs_dir2.c       | 15 +++++++++------
 fs/xfs/libxfs/xfs_dir2.h       |  3 ++-
 fs/xfs/libxfs/xfs_dir2_block.c |  4 ++--
 fs/xfs/libxfs/xfs_dir2_leaf.c  |  5 +++--
 fs/xfs/libxfs/xfs_dir2_node.c  |  5 +++--
 fs/xfs/libxfs/xfs_dir2_sf.c    |  2 ++
 fs/xfs/xfs_inode.c             |  7 ++++---
 7 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index a1ca460..a34956e 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -443,13 +443,14 @@ xfs_dir_lookup(
  */
 int
 xfs_dir_removename(
-	xfs_trans_t	*tp,
-	xfs_inode_t	*dp,
-	struct xfs_name	*name,
-	xfs_ino_t	ino,
-	xfs_fsblock_t	*first,		/* bmap's firstblock */
+	xfs_trans_t		*tp,
+	xfs_inode_t		*dp,
+	struct xfs_name		*name,
+	xfs_ino_t		ino,
+	xfs_fsblock_t		*first,		/* bmap's firstblock */
 	struct xfs_defer_ops	*dfops,		/* bmap's freeblock list */
-	xfs_extlen_t	total)		/* bmap's total block count */
+	xfs_extlen_t		total,		/* bmap's total block count */
+	xfs_dir2_dataptr_t	*offset)	/* OUT: offset in directory */
 {
 	struct xfs_da_args *args;
 	int		rval;
@@ -495,6 +496,8 @@ xfs_dir_removename(
 		rval = xfs_dir2_leaf_removename(args);
 	else
 		rval = xfs_dir2_node_removename(args);
+	if (offset)
+		*offset = args->offset;
 out_free:
 	kmem_free(args);
 	return rval;
diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
index e349900..ee98b2a 100644
--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -139,7 +139,8 @@ extern int xfs_dir_lookup(struct xfs_trans *tp, struct xfs_inode *dp,
 extern int xfs_dir_removename(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name, xfs_ino_t ino,
 				xfs_fsblock_t *first,
-				struct xfs_defer_ops *dfops, xfs_extlen_t tot);
+				struct xfs_defer_ops *dfops, xfs_extlen_t tot,
+				xfs_dir2_dataptr_t *offset);
 extern int xfs_dir_replace(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name, xfs_ino_t inum,
 				xfs_fsblock_t *first,
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index 79684d5..4dbe2fc 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -791,9 +791,9 @@ xfs_dir2_block_removename(
 	/*
 	 * Point to the data entry using the leaf entry.
 	 */
+	args->offset = be32_to_cpu(blp[ent].address);
 	dep = (xfs_dir2_data_entry_t *)((char *)hdr +
-			xfs_dir2_dataptr_to_off(args->geo,
-						be32_to_cpu(blp[ent].address)));
+			xfs_dir2_dataptr_to_off(args->geo, args->offset));
 	/*
 	 * Mark the data entry's space free.
 	 */
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index 2ac7a7e..197e627 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -1383,9 +1383,10 @@ xfs_dir2_leaf_removename(
 	 * Point to the leaf entry, use that to point to the data entry.
 	 */
 	lep = &ents[index];
-	db = xfs_dir2_dataptr_to_db(args->geo, be32_to_cpu(lep->address));
+	args->offset = be32_to_cpu(lep->address);
+	db = xfs_dir2_dataptr_to_db(args->geo, args->offset);
 	dep = (xfs_dir2_data_entry_t *)((char *)hdr +
-		xfs_dir2_dataptr_to_off(args->geo, be32_to_cpu(lep->address)));
+		xfs_dir2_dataptr_to_off(args->geo, args->offset));
 	needscan = needlog = 0;
 	oldbest = be16_to_cpu(bf[0].length);
 	ltp = xfs_dir2_leaf_tail_p(args->geo, leaf);
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 8bc91f8..13d5244 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -1238,9 +1238,10 @@ xfs_dir2_leafn_remove(
 	/*
 	 * Extract the data block and offset from the entry.
 	 */
-	db = xfs_dir2_dataptr_to_db(args->geo, be32_to_cpu(lep->address));
+	args->offset = be32_to_cpu(lep->address);
+	db = xfs_dir2_dataptr_to_db(args->geo, args->offset);
 	ASSERT(dblk->blkno == db);
-	off = xfs_dir2_dataptr_to_off(args->geo, be32_to_cpu(lep->address));
+	off = xfs_dir2_dataptr_to_off(args->geo, args->offset);
 	ASSERT(dblk->index == off);
 
 	/*
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index 489bdef..9e90c22 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -919,6 +919,8 @@ xfs_dir2_sf_removename(
 								XFS_CMP_EXACT) {
 			ASSERT(dp->d_ops->sf_get_ino(sfp, sfep) ==
 			       args->inumber);
+			args->offset = xfs_dir2_byte_to_dataptr(
+						xfs_dir2_sf_get_offset(sfep));
 			break;
 		}
 	}
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index cea1c56..f58f62a 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2621,8 +2621,8 @@ xfs_remove(
 		goto out_trans_cancel;
 
 	xfs_defer_init(&dfops, &first_block);
-	error = xfs_dir_removename(tp, dp, name, ip->i_ino,
-					&first_block, &dfops, resblks);
+	error = xfs_dir_removename(tp, dp, name, ip->i_ino, &first_block,
+				   &dfops, resblks, NULL);
 	if (error) {
 		ASSERT(error != -ENOENT);
 		goto out_bmap_cancel;
@@ -3132,7 +3132,8 @@ xfs_rename(
 					&first_block, &dfops, spaceres);
 	} else
 		error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino,
-					   &first_block, &dfops, spaceres);
+					   &first_block, &dfops, spaceres,
+					   NULL);
 	if (error)
 		goto out_bmap_cancel;
 
-- 
2.7.4

[RFC PATCH 03/13] xfs: get directory offset when replacing a directory name

[Allison Henderson]

From: Mark Tinguely <tinguely@sgi.com>

Return the directory offset information when replacing an entry to the
directory.

This offset will be used as the parent pointer offset in xfs_rename.

[dchinner: forward ported and cleaned up]
[achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t]

Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 a34956e... f5d0d3d... M	fs/xfs/libxfs/xfs_dir2.c
:100644 100644 ee98b2a... 67ce660... M	fs/xfs/libxfs/xfs_dir2.h
:100644 100644 4dbe2fc... 69dfe64... M	fs/xfs/libxfs/xfs_dir2_block.c
:100644 100644 197e627... 770b93f... M	fs/xfs/libxfs/xfs_dir2_leaf.c
:100644 100644 13d5244... 860a612... M	fs/xfs/libxfs/xfs_dir2_node.c
:100644 100644 9e90c22... 295458f... M	fs/xfs/libxfs/xfs_dir2_sf.c
:100644 100644 f58f62a... 52c331e... M	fs/xfs/xfs_inode.c
 fs/xfs/libxfs/xfs_dir2.c       | 16 ++++++++++------
 fs/xfs/libxfs/xfs_dir2.h       |  3 ++-
 fs/xfs/libxfs/xfs_dir2_block.c |  4 ++--
 fs/xfs/libxfs/xfs_dir2_leaf.c  |  1 +
 fs/xfs/libxfs/xfs_dir2_node.c  |  1 +
 fs/xfs/libxfs/xfs_dir2_sf.c    |  2 ++
 fs/xfs/xfs_inode.c             | 28 +++++++++++++---------------
 7 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index a34956e..f5d0d3d 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -508,13 +508,14 @@ xfs_dir_removename(
  */
 int
 xfs_dir_replace(
-	xfs_trans_t	*tp,
-	xfs_inode_t	*dp,
-	struct xfs_name	*name,		/* name of entry to replace */
-	xfs_ino_t	inum,		/* new inode number */
-	xfs_fsblock_t	*first,		/* bmap's firstblock */
+	xfs_trans_t		*tp,
+	xfs_inode_t		*dp,
+	struct xfs_name		*name,		/* name of entry to replace */
+	xfs_ino_t		inum,		/* new inode number */
+	xfs_fsblock_t		*first,		/* bmap's firstblock */
 	struct xfs_defer_ops	*dfops,		/* bmap's freeblock list */
-	xfs_extlen_t	total)		/* bmap's total block count */
+	xfs_extlen_t		total,		/* bmap's total block count */
+	xfs_dir2_dataptr_t	*offset)	/* OUT: offset in directory */
 {
 	struct xfs_da_args *args;
 	int		rval;
@@ -563,6 +564,9 @@ xfs_dir_replace(
 		rval = xfs_dir2_leaf_replace(args);
 	else
 		rval = xfs_dir2_node_replace(args);
+
+	if (offset)
+		*offset = args->offset;
 out_free:
 	kmem_free(args);
 	return rval;
diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
index ee98b2a..67ce660 100644
--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -144,7 +144,8 @@ extern int xfs_dir_removename(struct xfs_trans *tp, struct xfs_inode *dp,
 extern int xfs_dir_replace(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name, xfs_ino_t inum,
 				xfs_fsblock_t *first,
-				struct xfs_defer_ops *dfops, xfs_extlen_t tot);
+				struct xfs_defer_ops *dfops, xfs_extlen_t tot,
+				xfs_dir2_dataptr_t *offset);
 extern int xfs_dir_canenter(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name);
 
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index 4dbe2fc..69dfe64 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -865,9 +865,9 @@ xfs_dir2_block_replace(
 	/*
 	 * Point to the data entry we need to change.
 	 */
+	args->offset = be32_to_cpu(blp[ent].address);
 	dep = (xfs_dir2_data_entry_t *)((char *)hdr +
-			xfs_dir2_dataptr_to_off(args->geo,
-						be32_to_cpu(blp[ent].address)));
+			xfs_dir2_dataptr_to_off(args->geo, args->offset));
 	ASSERT(be64_to_cpu(dep->inumber) != args->inumber);
 	/*
 	 * Change the inode number to the new value.
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index 197e627..770b93f 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -1518,6 +1518,7 @@ xfs_dir2_leaf_replace(
 	/*
 	 * Point to the data entry.
 	 */
+	args->offset = be32_to_cpu(lep->address);
 	dep = (xfs_dir2_data_entry_t *)
 	      ((char *)dbp->b_addr +
 	       xfs_dir2_dataptr_to_off(args->geo, be32_to_cpu(lep->address)));
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 13d5244..860a612 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -2237,6 +2237,7 @@ xfs_dir2_node_replace(
 		hdr = state->extrablk.bp->b_addr;
 		ASSERT(hdr->magic == cpu_to_be32(XFS_DIR2_DATA_MAGIC) ||
 		       hdr->magic == cpu_to_be32(XFS_DIR3_DATA_MAGIC));
+		args->offset = be32_to_cpu(lep->address);
 		dep = (xfs_dir2_data_entry_t *)
 		      ((char *)hdr +
 		       xfs_dir2_dataptr_to_off(args->geo,
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index 9e90c22..295458f 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -1045,6 +1045,8 @@ xfs_dir2_sf_replace(
 				ASSERT(args->inumber != ino);
 				dp->d_ops->sf_put_ino(sfp, sfep, args->inumber);
 				dp->d_ops->sf_put_ftype(sfep, args->filetype);
+				args->offset = xfs_dir2_byte_to_dataptr(
+						  xfs_dir2_sf_get_offset(sfep));
 				break;
 			}
 		}
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index f58f62a..52c331e 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2756,16 +2756,14 @@ xfs_cross_rename(
 	int		dp2_flags = 0;
 
 	/* Swap inode number for dirent in first parent */
-	error = xfs_dir_replace(tp, dp1, name1,
-				ip2->i_ino,
-				first_block, dfops, spaceres);
+	error = xfs_dir_replace(tp, dp1, name1, ip2->i_ino, first_block, dfops,
+				spaceres, NULL);
 	if (error)
 		goto out_trans_abort;
 
 	/* Swap inode number for dirent in second parent */
-	error = xfs_dir_replace(tp, dp2, name2,
-				ip1->i_ino,
-				first_block, dfops, spaceres);
+	error = xfs_dir_replace(tp, dp2, name2, ip1->i_ino, first_block, dfops,
+				spaceres, NULL);
 	if (error)
 		goto out_trans_abort;
 
@@ -2779,8 +2777,8 @@ xfs_cross_rename(
 
 		if (S_ISDIR(VFS_I(ip2)->i_mode)) {
 			error = xfs_dir_replace(tp, ip2, &xfs_name_dotdot,
-						dp1->i_ino, first_block,
-						dfops, spaceres);
+						dp1->i_ino, first_block, dfops,
+						spaceres, NULL);
 			if (error)
 				goto out_trans_abort;
 
@@ -2806,8 +2804,8 @@ xfs_cross_rename(
 
 		if (S_ISDIR(VFS_I(ip1)->i_mode)) {
 			error = xfs_dir_replace(tp, ip1, &xfs_name_dotdot,
-						dp2->i_ino, first_block,
-						dfops, spaceres);
+						dp2->i_ino, first_block, dfops,
+						spaceres, NULL);
 			if (error)
 				goto out_trans_abort;
 
@@ -3054,8 +3052,8 @@ xfs_rename(
 		 * name at the destination directory, remove it first.
 		 */
 		error = xfs_dir_replace(tp, target_dp, target_name,
-					src_ip->i_ino,
-					&first_block, &dfops, spaceres);
+					src_ip->i_ino, &first_block, &dfops,
+					spaceres, NULL);
 		if (error)
 			goto out_bmap_cancel;
 
@@ -3089,8 +3087,8 @@ xfs_rename(
 		 * directory.
 		 */
 		error = xfs_dir_replace(tp, src_ip, &xfs_name_dotdot,
-					target_dp->i_ino,
-					&first_block, &dfops, spaceres);
+					target_dp->i_ino, &first_block, &dfops,
+					spaceres, NULL);
 		ASSERT(error != -EEXIST);
 		if (error)
 			goto out_bmap_cancel;
@@ -3129,7 +3127,7 @@ xfs_rename(
 	 */
 	if (wip) {
 		error = xfs_dir_replace(tp, src_dp, src_name, wip->i_ino,
-					&first_block, &dfops, spaceres);
+					&first_block, &dfops, spaceres, NULL);
 	} else
 		error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino,
 					   &first_block, &dfops, spaceres,
-- 
2.7.4

[RFC PATCH 04/13] xfs: add parent pointer support to attribute code

[Allison Henderson]

From: Mark Tinguely <tinguely@sgi.com>

Add the new parent attribute type. XFS_ATTR_PARENT is used only for
parent pointer entries; it uses reserved blocks like XFS_ATTR_ROOT.

[dchinner: forward ported and cleaned up]
[achender: rebased]

Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 de7b9bd... 646d41b... M	fs/xfs/libxfs/xfs_attr.c
:100644 100644 3771edc... 5f94c84... M	fs/xfs/libxfs/xfs_da_format.h
:100644 100644 5d5a5e2... 3031a09... M	fs/xfs/xfs_attr.h
 fs/xfs/libxfs/xfs_attr.c      |  9 +++++----
 fs/xfs/libxfs/xfs_da_format.h | 12 ++++++++----
 fs/xfs/xfs_attr.h             |  2 ++
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index de7b9bd..646d41b 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -216,7 +216,7 @@ xfs_attr_set(
 	struct xfs_defer_ops	dfops;
 	struct xfs_trans_res	tres;
 	xfs_fsblock_t		firstblock;
-	int			rsvd = (flags & ATTR_ROOT) != 0;
+	bool			rsvd = (flags & (ATTR_ROOT | ATTR_PARENT)) != 0;
 	int			error, err2, local;
 
 	XFS_STATS_INC(mp, xs_attr_set);
@@ -395,6 +395,7 @@ xfs_attr_remove(
 	struct xfs_defer_ops	dfops;
 	xfs_fsblock_t		firstblock;
 	int			error;
+	int			rsvd = 0;
 
 	XFS_STATS_INC(mp, xs_attr_remove);
 
@@ -423,10 +424,10 @@ xfs_attr_remove(
 	 * Root fork attributes can use reserved data blocks for this
 	 * operation if necessary
 	 */
+	if (flags & (ATTR_ROOT | ATTR_PARENT))
+		rsvd = XFS_TRANS_RESERVE;
 	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_attrrm,
-			XFS_ATTRRM_SPACE_RES(mp), 0,
-			(flags & ATTR_ROOT) ? XFS_TRANS_RESERVE : 0,
-			&args.trans);
+				XFS_ATTRRM_SPACE_RES(mp), 0, rsvd, &args.trans);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
index 3771edc..5f94c84 100644
--- a/fs/xfs/libxfs/xfs_da_format.h
+++ b/fs/xfs/libxfs/xfs_da_format.h
@@ -758,24 +758,28 @@ struct xfs_attr3_icleaf_hdr {
 #define	XFS_ATTR_LOCAL_BIT	0	/* attr is stored locally */
 #define	XFS_ATTR_ROOT_BIT	1	/* limit access to trusted attrs */
 #define	XFS_ATTR_SECURE_BIT	2	/* limit access to secure attrs */
+#define XFS_ATTR_PARENT_BIT	3	/* parent pointer secure attrs */
 #define	XFS_ATTR_INCOMPLETE_BIT	7	/* attr in middle of create/delete */
 #define XFS_ATTR_LOCAL		(1 << XFS_ATTR_LOCAL_BIT)
 #define XFS_ATTR_ROOT		(1 << XFS_ATTR_ROOT_BIT)
 #define XFS_ATTR_SECURE		(1 << XFS_ATTR_SECURE_BIT)
+#define XFS_ATTR_PARENT		(1 << XFS_ATTR_PARENT_BIT)
 #define XFS_ATTR_INCOMPLETE	(1 << XFS_ATTR_INCOMPLETE_BIT)
 
 /*
  * Conversion macros for converting namespace bits from argument flags
  * to ondisk flags.
  */
-#define XFS_ATTR_NSP_ARGS_MASK		(ATTR_ROOT | ATTR_SECURE)
-#define XFS_ATTR_NSP_ONDISK_MASK	(XFS_ATTR_ROOT | XFS_ATTR_SECURE)
+#define XFS_ATTR_NSP_ARGS_MASK		(ATTR_ROOT | ATTR_SECURE | XFS_ATTR_PARENT)
+#define XFS_ATTR_NSP_ONDISK_MASK	(XFS_ATTR_ROOT | XFS_ATTR_SECURE | XFS_ATTR_PARENT)
 #define XFS_ATTR_NSP_ONDISK(flags)	((flags) & XFS_ATTR_NSP_ONDISK_MASK)
 #define XFS_ATTR_NSP_ARGS(flags)	((flags) & XFS_ATTR_NSP_ARGS_MASK)
 #define XFS_ATTR_NSP_ARGS_TO_ONDISK(x)	(((x) & ATTR_ROOT ? XFS_ATTR_ROOT : 0) |\
-					 ((x) & ATTR_SECURE ? XFS_ATTR_SECURE : 0))
+					 ((x) & ATTR_SECURE ? XFS_ATTR_SECURE : 0) | \
+					 ((x) & ATTR_PARENT ? XFS_ATTR_PARENT : 0))
 #define XFS_ATTR_NSP_ONDISK_TO_ARGS(x)	(((x) & XFS_ATTR_ROOT ? ATTR_ROOT : 0) |\
-					 ((x) & XFS_ATTR_SECURE ? ATTR_SECURE : 0))
+					 ((x) & XFS_ATTR_SECURE ? ATTR_SECURE : 0) | \
+					 ((x) & XFS_ATTR_PARENT ? ATTR_PARENT : 0))
 
 /*
  * Alignment for namelist and valuelist entries (since they are mixed
diff --git a/fs/xfs/xfs_attr.h b/fs/xfs/xfs_attr.h
index 5d5a5e2..3031a09 100644
--- a/fs/xfs/xfs_attr.h
+++ b/fs/xfs/xfs_attr.h
@@ -44,6 +44,7 @@ struct xfs_attr_list_context;
 #define ATTR_SECURE	0x0008	/* use attrs in security namespace */
 #define ATTR_CREATE	0x0010	/* pure create: fail if attr already exists */
 #define ATTR_REPLACE	0x0020	/* pure set: fail if attr does not exist */
+#define ATTR_PARENT	0x0040	/*  use attrs in parent namespace */
 
 #define ATTR_KERNOTIME	0x1000	/* [kernel] don't update inode timestamps */
 #define ATTR_KERNOVAL	0x2000	/* [kernel] get attr size only, not value */
@@ -55,6 +56,7 @@ struct xfs_attr_list_context;
 	{ ATTR_SECURE,		"SECURE" }, \
 	{ ATTR_CREATE,		"CREATE" }, \
 	{ ATTR_REPLACE,		"REPLACE" }, \
+	{ ATTR_PARENT,		"PARENT" }, \
 	{ ATTR_KERNOTIME,	"KERNOTIME" }, \
 	{ ATTR_KERNOVAL,	"KERNOVAL" }
 
-- 
2.7.4

[RFC PATCH 05/13] xfs: define parent pointer xattr format

[Allison Henderson]

From: Dave Chinner <dchinner@redhat.com>

We need to define the parent pointer attribute format before we
start adding support for it into all the code that needs to use it.
The EA format we will use encodes the following information:

	name={parent inode #, parent inode generation, dirent offset}
	value={dirent filename}

The inode/gen gives all the information we need to reliably identify
the parent without requiring child->parent lock ordering, and allows
userspace to do pathname component level reconstruction without the
kernel ever needing to verify the parent itself as part of ioctl
calls.

By using the dirent offset in the EA name, we have a method of
knowing the exact parent pointer EA we need to modify/remove in
rename/unlink without an unbound EA name search.

By keeping the dirent name in the value, we have enough information
to be able to validate and reconstruct damaged directory trees.
While the diroffset of a filename alone is not unique enough to
identify the child, the {diroffset,filename,child_inode} tuple is
sufficient. That is, if the diroffset gets reused and points to a
different filename, we can detect that from the contents of EA. If a
link of the same name is created, then we can check whether it
points at the same inode as the parent EA we current have.

[achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t]

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 d4d9bef... 91e2d5a... M	fs/xfs/libxfs/xfs_format.h
 fs/xfs/libxfs/xfs_format.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index d4d9bef..91e2d5a 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -18,6 +18,8 @@
 #ifndef __XFS_FORMAT_H__
 #define __XFS_FORMAT_H__
 
+#include "xfs_da_format.h"
+
 /*
  * XFS On Disk Format Definitions
  *
@@ -1721,4 +1723,29 @@ struct xfs_acl {
 #define SGI_ACL_FILE_SIZE	(sizeof(SGI_ACL_FILE)-1)
 #define SGI_ACL_DEFAULT_SIZE	(sizeof(SGI_ACL_DEFAULT)-1)
 
+/*
+ * Parent pointer attribute format definition
+ *
+ * EA name encodes the parent inode number, generation and the offset of
+ * the dirent that points to the child inode. The EA value contains the
+ * same name as the dirent in the parent directory.
+ */
+struct xfs_parent_name_rec {
+	__be64	p_ino;
+	__be32	p_gen;
+	__be32	p_diroffset;
+};
+
+/*
+ * incore version of the above, also contains name pointers so callers
+ * can pass/obtain all the parent pointer information in a single structure
+ */
+struct xfs_parent_name_irec {
+	uint64_t		p_ino;
+	uint32_t		p_gen;
+	xfs_dir2_dataptr_t	p_diroffset;
+	const char		*p_name;
+	uint32_t		p_namelen;
+};
+
 #endif /* __XFS_FORMAT_H__ */
-- 
2.7.4

[RFC PATCH 06/13] :xfs: extent transaction reservations for parent attributes

[Allison Henderson]

From: Dave Chinner <dchinner@redhat.com>

We need to add, remove or modify parent pointer attributes during
create/link/unlink/rename operations atomically with the dirents in the parent
directories being modified. This means they need to be modified in the same
transaction as the parent directories, and so we need to add the required
space for the attribute modifications to the transaction reservations.

[achender: rebased, added xfs_sb_version_hasparent stub]

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 91e2d5a... 6cedd75... M	fs/xfs/libxfs/xfs_format.h
:100644 100644 6bd916b... 54399e2... M	fs/xfs/libxfs/xfs_trans_resv.c
 fs/xfs/libxfs/xfs_format.h     |   5 ++
 fs/xfs/libxfs/xfs_trans_resv.c | 103 ++++++++++++++++++++++++++++++++---------
 2 files changed, 85 insertions(+), 23 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 91e2d5a..6cedd75 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -561,6 +561,11 @@ static inline bool xfs_sb_version_hasreflink(struct xfs_sb *sbp)
 		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_REFLINK);
 }
 
+static inline bool xfs_sb_version_hasparent(struct xfs_sb *sbp)
+{
+	return false; /* We'll enable this at the end of the set */
+}
+
 /*
  * end of superblock version macros
  */
diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index 6bd916b..54399e2 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -802,29 +802,30 @@ xfs_calc_sb_reservation(
 	return xfs_calc_buf_res(1, mp->m_sb.sb_sectsize);
 }
 
+/*
+ * Namespace reservations.
+ *
+ * These get tricky when parent pointers are enabled as we have attribute
+ * modifications occurring from within these transactions. Rather than confuse
+ * each of these reservation calculations with the conditional attribute
+ * reservations, add them here in a clear and concise manner. This assumes that
+ * the attribute reservations have already been calculated.
+ *
+ * Note that we only include the static attribute reservation here; the runtime
+ * reservation will have to be modified by the size of the attributes being
+ * added/removed/modified. See the comments on the attribute reservation
+ * calculations for more details.
+ *
+ * Note for rename: rename will vastly overestimate requirements. This will be
+ * addressed later when modifications are made to ensure parent attribute
+ * modifications can be done atomically with the rename operation.
+ */
 void
-xfs_trans_resv_calc(
+xfs_calc_namespace_reservations(
 	struct xfs_mount	*mp,
 	struct xfs_trans_resv	*resp)
 {
-	/*
-	 * The following transactions are logged in physical format and
-	 * require a permanent reservation on space.
-	 */
-	resp->tr_write.tr_logres = xfs_calc_write_reservation(mp);
-	if (xfs_sb_version_hasreflink(&mp->m_sb))
-		resp->tr_write.tr_logcount = XFS_WRITE_LOG_COUNT_REFLINK;
-	else
-		resp->tr_write.tr_logcount = XFS_WRITE_LOG_COUNT;
-	resp->tr_write.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
-
-	resp->tr_itruncate.tr_logres = xfs_calc_itruncate_reservation(mp);
-	if (xfs_sb_version_hasreflink(&mp->m_sb))
-		resp->tr_itruncate.tr_logcount =
-				XFS_ITRUNCATE_LOG_COUNT_REFLINK;
-	else
-		resp->tr_itruncate.tr_logcount = XFS_ITRUNCATE_LOG_COUNT;
-	resp->tr_itruncate.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
+	ASSERT(resp->tr_attrsetm.tr_logres > 0);
 
 	resp->tr_rename.tr_logres = xfs_calc_rename_reservation(mp);
 	resp->tr_rename.tr_logcount = XFS_RENAME_LOG_COUNT;
@@ -846,15 +847,69 @@ xfs_trans_resv_calc(
 	resp->tr_create.tr_logcount = XFS_CREATE_LOG_COUNT;
 	resp->tr_create.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
+	resp->tr_mkdir.tr_logres = xfs_calc_mkdir_reservation(mp);
+	resp->tr_mkdir.tr_logcount = XFS_MKDIR_LOG_COUNT;
+	resp->tr_mkdir.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
+
+	if (!xfs_sb_version_hasparent(&mp->m_sb))
+		return;
+
+	/* rename can add/remove/modify 2 parent attributes */
+	resp->tr_rename.tr_logres += 2 * max(resp->tr_attrsetm.tr_logres,
+					     resp->tr_attrrm.tr_logres);
+	resp->tr_rename.tr_logcount += 2 * max(resp->tr_attrsetm.tr_logcount,
+					       resp->tr_attrrm.tr_logcount);
+
+	/* create will add 1 parent attribute */
+	resp->tr_create.tr_logres += resp->tr_attrsetm.tr_logres;
+	resp->tr_create.tr_logcount += resp->tr_attrsetm.tr_logcount;
+
+	/* mkdir will add 1 parent attribute */
+	resp->tr_mkdir.tr_logres += resp->tr_attrsetm.tr_logres;
+	resp->tr_mkdir.tr_logcount += resp->tr_attrsetm.tr_logcount;
+
+	/* link will add 1 parent attribute */
+	resp->tr_link.tr_logres += resp->tr_attrsetm.tr_logres;
+	resp->tr_link.tr_logcount += resp->tr_attrsetm.tr_logcount;
+
+	/* symlink will add 1 parent attribute */
+	resp->tr_symlink.tr_logres += resp->tr_attrsetm.tr_logres;
+	resp->tr_symlink.tr_logcount += resp->tr_attrsetm.tr_logcount;
+
+	/* remove will remove 1 parent attribute */
+	resp->tr_remove.tr_logres += resp->tr_attrrm.tr_logres;
+	resp->tr_remove.tr_logcount = resp->tr_attrrm.tr_logcount;
+}
+
+void
+xfs_trans_resv_calc(
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
+{
+	/*
+	 * The following transactions are logged in physical format and
+	 * require a permanent reservation on space.
+	 */
+	resp->tr_write.tr_logres = xfs_calc_write_reservation(mp);
+	if (xfs_sb_version_hasreflink(&mp->m_sb))
+		resp->tr_write.tr_logcount = XFS_WRITE_LOG_COUNT_REFLINK;
+	else
+		resp->tr_write.tr_logcount = XFS_WRITE_LOG_COUNT;
+	resp->tr_write.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
+
+	resp->tr_itruncate.tr_logres = xfs_calc_itruncate_reservation(mp);
+	if (xfs_sb_version_hasreflink(&mp->m_sb))
+		resp->tr_itruncate.tr_logcount =
+				XFS_ITRUNCATE_LOG_COUNT_REFLINK;
+	else
+		resp->tr_itruncate.tr_logcount = XFS_ITRUNCATE_LOG_COUNT;
+	resp->tr_itruncate.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
+
 	resp->tr_create_tmpfile.tr_logres =
 			xfs_calc_create_tmpfile_reservation(mp);
 	resp->tr_create_tmpfile.tr_logcount = XFS_CREATE_TMPFILE_LOG_COUNT;
 	resp->tr_create_tmpfile.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
-	resp->tr_mkdir.tr_logres = xfs_calc_mkdir_reservation(mp);
-	resp->tr_mkdir.tr_logcount = XFS_MKDIR_LOG_COUNT;
-	resp->tr_mkdir.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
-
 	resp->tr_ifree.tr_logres = xfs_calc_ifree_reservation(mp);
 	resp->tr_ifree.tr_logcount = XFS_INACTIVE_LOG_COUNT;
 	resp->tr_ifree.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
@@ -886,6 +941,8 @@ xfs_trans_resv_calc(
 		resp->tr_qm_dqalloc.tr_logcount = XFS_WRITE_LOG_COUNT;
 	resp->tr_qm_dqalloc.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
+	xfs_calc_namespace_reservations(mp, resp);
+
 	/*
 	 * The following transactions are logged in logical format with
 	 * a default log count.
-- 
2.7.4

[RFC PATCH 07/13] Add the extra space requirements for parent pointer attributes when calculating the minimum log size during mkfs

[Allison Henderson]

Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 c105979... beec9bf... M	fs/xfs/libxfs/xfs_log_rlimit.c
 fs/xfs/libxfs/xfs_log_rlimit.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_log_rlimit.c b/fs/xfs/libxfs/xfs_log_rlimit.c
index c105979..beec9bf 100644
--- a/fs/xfs/libxfs/xfs_log_rlimit.c
+++ b/fs/xfs/libxfs/xfs_log_rlimit.c
@@ -39,6 +39,40 @@ xfs_log_calc_max_attrsetm_res(
 {
 	int			size;
 	int			nblks;
+	struct xfs_trans_resv   *resp = M_RES(mp);
+
+	/* Calculate extra space needed for parent pointer attributes */
+	if (!xfs_sb_version_hasparent(&mp->m_sb)) {
+
+		/* rename can add/remove/modify 2 parent attributes */
+		resp->tr_rename.tr_logres +=
+			2 * max(resp->tr_attrsetm.tr_logres,
+				resp->tr_attrrm.tr_logres);
+		resp->tr_rename.tr_logcount +=
+			2 * max(resp->tr_attrsetm.tr_logcount,
+				resp->tr_attrrm.tr_logcount);
+
+		/* create will add 1 parent attribute */
+		resp->tr_create.tr_logres += resp->tr_attrsetm.tr_logres;
+		resp->tr_create.tr_logcount += resp->tr_attrsetm.tr_logcount;
+
+		/* mkdir will add 1 parent attribute */
+		resp->tr_mkdir.tr_logres += resp->tr_attrsetm.tr_logres;
+		resp->tr_mkdir.tr_logcount += resp->tr_attrsetm.tr_logcount;
+
+		/* link will add 1 parent attribute */
+		resp->tr_link.tr_logres += resp->tr_attrsetm.tr_logres;
+		resp->tr_link.tr_logcount += resp->tr_attrsetm.tr_logcount;
+
+		/* symlink will add 1 parent attribute */
+		resp->tr_symlink.tr_logres += resp->tr_attrsetm.tr_logres;
+		resp->tr_symlink.tr_logcount += resp->tr_attrsetm.tr_logcount;
+
+		/* remove will remove 1 parent attribute */
+		resp->tr_remove.tr_logres += resp->tr_attrrm.tr_logres;
+		resp->tr_remove.tr_logcount = resp->tr_attrrm.tr_logcount;
+	}
+
 
 	size = xfs_attr_leaf_entsize_local_max(mp->m_attr_geo->blksize) -
 	       MAXNAMELEN - 1;
-- 
2.7.4

[RFC PATCH 08/13] xfs: parent pointer attribute creation

[Allison Henderson]

From: Dave Chinner <dchinner@redhat.com>

[bfoster: rebase, use VFS inode generation]
[achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t,
	   fixed some null pointer bugs]

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 a2a5d04... 90b51ae... M	fs/xfs/Makefile
:100644 100644 646d41b... ed13c67... M	fs/xfs/libxfs/xfs_attr.c
:100644 100644 a2d6466... 91c403e... M	fs/xfs/libxfs/xfs_bmap.c
:100644 100644 851982a... 533f40f... M	fs/xfs/libxfs/xfs_bmap.h
:000000 100644 0000000... 88f7edc... A	fs/xfs/libxfs/xfs_parent.c
:100644 100644 3031a09... fad2fbd... M	fs/xfs/xfs_attr.h
:100644 100644 52c331e... 3557791... M	fs/xfs/xfs_inode.c
 fs/xfs/Makefile            |  1 +
 fs/xfs/libxfs/xfs_attr.c   | 80 ++++++++++++++++++++++++++++++++++---
 fs/xfs/libxfs/xfs_bmap.c   | 51 ++++++++++++++----------
 fs/xfs/libxfs/xfs_bmap.h   |  1 +
 fs/xfs/libxfs/xfs_parent.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_attr.h          | 13 +++++-
 fs/xfs/xfs_inode.c         | 16 +++++++-
 7 files changed, 232 insertions(+), 28 deletions(-)

diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
index a2a5d04..90b51ae 100644
--- a/fs/xfs/Makefile
+++ b/fs/xfs/Makefile
@@ -52,6 +52,7 @@ xfs-y				+= $(addprefix libxfs/, \
 				   xfs_inode_fork.o \
 				   xfs_inode_buf.o \
 				   xfs_log_rlimit.o \
+				   xfs_parent.o \
 				   xfs_ag_resv.o \
 				   xfs_rmap.o \
 				   xfs_rmap_btree.o \
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 646d41b..ed13c67 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -90,12 +90,14 @@ xfs_attr_args_init(
 	args->whichfork = XFS_ATTR_FORK;
 	args->dp = dp;
 	args->flags = flags;
-	args->name = name;
-	args->namelen = strlen((const char *)name);
-	if (args->namelen >= MAXNAMELEN)
-		return -EFAULT;		/* match IRIX behaviour */
+	if (name) {
+		args->name = name;
+		args->namelen = strlen((const char *)name);
+		if (args->namelen >= MAXNAMELEN)
+			return -EFAULT;		/* match IRIX behaviour */
 
-	args->hashval = xfs_da_hashname(args->name, args->namelen);
+		args->hashval = xfs_da_hashname(args->name, args->namelen);
+	}
 	return 0;
 }
 
@@ -203,6 +205,74 @@ xfs_attr_calc_size(
 	return nblks;
 }
 
+/*
+ * Add the initial parent pointer attribute.
+ *
+ * Inode must be locked and completely empty as we are adding the attribute
+ * fork to the inode. This open codes bits of xfs_bmap_add_attrfork() and
+ * xfs_attr_set() because we know the inode is completely empty at this point
+ * and so don't need to handle all the different combinations of fork
+ * configurations here.
+ */
+int
+xfs_attr_set_first_parent(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip,
+	struct xfs_parent_name_rec *rec,
+	int			reclen,
+	const char		*value,
+	int			valuelen,
+	struct xfs_defer_ops	*dfops,
+	xfs_fsblock_t		*firstblock)
+{
+	struct xfs_da_args	args;
+	int			flags = ATTR_PARENT;
+	int			local;
+	int			sf_size;
+	int			error;
+
+	tp->t_flags |= XFS_TRANS_RESERVE;
+
+	error = xfs_attr_args_init(&args, ip, (char *)rec, flags);
+	if (error)
+		return error;
+
+	args.name = (char *)rec;
+	args.namelen = reclen;
+	args.hashval = xfs_da_hashname(args.name, args.namelen);
+	args.value = (char *)value;
+	args.valuelen = valuelen;
+	args.firstblock = firstblock;
+	args.dfops = dfops;
+	args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
+	args.total = xfs_attr_calc_size(&args, &local);
+	args.trans = tp;
+	ASSERT(local);
+
+	/* set the attribute fork appropriately */
+	sf_size = sizeof(struct xfs_attr_sf_hdr) +
+			XFS_ATTR_SF_ENTSIZE_BYNAME(reclen, valuelen);
+	xfs_bmap_set_attrforkoff(ip, sf_size, NULL);
+	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, KM_SLEEP);
+	ip->i_afp->if_flags = XFS_IFEXTENTS;
+
+
+	/* Try to add the attr to the attribute list in the inode. */
+	xfs_attr_shortform_create(&args);
+	error = xfs_attr_shortform_addname(&args);
+	if (error != -ENOSPC)
+		return error;
+
+	/* It won't fit in the shortform, transform to a leaf block. */
+	xfs_defer_init(args.dfops, args.firstblock);
+	error = xfs_attr_shortform_to_leaf(&args);
+	if (error)
+		return error;
+
+	ASSERT(xfs_bmap_one_block(ip, XFS_ATTR_FORK));
+	return xfs_attr_leaf_addname(&args);
+}
+
 int
 xfs_attr_set(
 	struct xfs_inode	*dp,
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index a2d6466..91c403e 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1076,6 +1076,35 @@ xfs_bmap_add_attrfork_local(
 	return -EFSCORRUPTED;
 }
 
+int
+xfs_bmap_set_attrforkoff(
+	struct xfs_inode	*ip,
+	int			size,
+	int			*version)
+{
+	switch (ip->i_d.di_format) {
+	case XFS_DINODE_FMT_DEV:
+		ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
+		break;
+	case XFS_DINODE_FMT_UUID:
+		ip->i_d.di_forkoff = roundup(sizeof(uuid_t), 8) >> 3;
+		break;
+	case XFS_DINODE_FMT_LOCAL:
+	case XFS_DINODE_FMT_EXTENTS:
+	case XFS_DINODE_FMT_BTREE:
+		ip->i_d.di_forkoff = xfs_attr_shortform_bytesfit(ip, size);
+		if (!ip->i_d.di_forkoff)
+			ip->i_d.di_forkoff = xfs_default_attroffset(ip) >> 3;
+		else if ((ip->i_mount->m_flags & XFS_MOUNT_ATTR2) && version)
+			*version = 2;
+		break;
+	default:
+		ASSERT(0);
+		return -EINVAL;
+	}
+	return 0;
+}
+
 /*
  * Convert inode from non-attributed to attributed.
  * Must not be in a transaction, ip must not be locked.
@@ -1130,27 +1159,7 @@ xfs_bmap_add_attrfork(
 	xfs_trans_ijoin(tp, ip, 0);
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 
-	switch (ip->i_d.di_format) {
-	case XFS_DINODE_FMT_DEV:
-		ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
-		break;
-	case XFS_DINODE_FMT_UUID:
-		ip->i_d.di_forkoff = roundup(sizeof(uuid_t), 8) >> 3;
-		break;
-	case XFS_DINODE_FMT_LOCAL:
-	case XFS_DINODE_FMT_EXTENTS:
-	case XFS_DINODE_FMT_BTREE:
-		ip->i_d.di_forkoff = xfs_attr_shortform_bytesfit(ip, size);
-		if (!ip->i_d.di_forkoff)
-			ip->i_d.di_forkoff = xfs_default_attroffset(ip) >> 3;
-		else if (mp->m_flags & XFS_MOUNT_ATTR2)
-			version = 2;
-		break;
-	default:
-		ASSERT(0);
-		error = -EINVAL;
-		goto trans_cancel;
-	}
+	xfs_bmap_set_attrforkoff(ip, size, &version);
 
 	ASSERT(ip->i_afp == NULL);
 	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, KM_SLEEP);
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 851982a..533f40f 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -209,6 +209,7 @@ void	xfs_bmap_trace_exlist(struct xfs_inode *ip, xfs_extnum_t cnt,
 void	xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
 		xfs_filblks_t len);
 int	xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
+int	xfs_bmap_set_attrforkoff(struct xfs_inode *ip, int size, int *version);
 void	xfs_bmap_local_to_extents_empty(struct xfs_inode *ip, int whichfork);
 void	xfs_bmap_add_free(struct xfs_mount *mp, struct xfs_defer_ops *dfops,
 			  xfs_fsblock_t bno, xfs_filblks_t len,
diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c
new file mode 100644
index 0000000..88f7edc
--- /dev/null
+++ b/fs/xfs/libxfs/xfs_parent.c
@@ -0,0 +1,98 @@
+/*
+ * Copyright (c) 2015 Red Hat, Inc.
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation
+ */
+#include "xfs.h"
+#include "xfs_fs.h"
+#include "xfs_format.h"
+#include "xfs_log_format.h"
+#include "xfs_shared.h"
+#include "xfs_trans_resv.h"
+#include "xfs_mount.h"
+#include "xfs_bmap_btree.h"
+#include "xfs_inode.h"
+#include "xfs_error.h"
+#include "xfs_trace.h"
+#include "xfs_trans.h"
+#include "xfs_attr.h"
+
+/*
+ * Parent pointer attribute handling.
+ *
+ * Because the attribute value is a filename component, it will never be longer
+ * than 255 bytes. This means the attribute will always be a local format
+ * attribute as it is xfs_attr_leaf_entsize_local_max() for v5 filesystems will
+ * always be larger than this (max is 75% of block size).
+ *
+ * Creating a new parent attribute will always create a new attribute - there
+ * should never, ever be an existing attribute in the tree for a new inode.
+ * ENOSPC behaviour is problematic - creating the inode without the parent
+ * pointer is effectively a corruption, so we allow parent attribute creation
+ * to dip into the reserve block pool to avoid unexpected ENOSPC errors from
+ * occurring.
+ */
+
+/*
+ * Create the initial parent attribute.
+ *
+ * The initial attribute creation also needs to be atomic w.r.t the parent
+ * directory modification. Hence it needs to run in the same transaction and the
+ * transaction committed by the caller.  Because the attribute created is
+ * guaranteed to be a local attribute and is always going to be the first
+ * attribute in the attribute fork, we can do this safely in the single
+ * transaction context as it is impossible for an overwrite to occur and hence
+ * we'll never have a rolling overwrite transaction occurring here. Hence we
+ * can short-cut a lot of the normal xfs_attr_set() code paths that are needed
+ * to handle the generic cases.
+ */
+static int
+xfs_parent_create_nrec(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*child,
+	struct xfs_parent_name_irec *nrec,
+	struct xfs_defer_ops	*dfops,
+	xfs_fsblock_t		*firstblock)
+{
+	struct xfs_parent_name_rec rec;
+
+	rec.p_ino = cpu_to_be64(nrec->p_ino);
+	rec.p_gen = cpu_to_be32(nrec->p_gen);
+	rec.p_diroffset = cpu_to_be32(nrec->p_diroffset);
+
+	return xfs_attr_set_first_parent(tp, child, &rec, sizeof(rec),
+				   nrec->p_name, nrec->p_namelen,
+				   dfops, firstblock);
+}
+
+int
+xfs_parent_create(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*parent,
+	struct xfs_inode	*child,
+	struct xfs_name		*child_name,
+	xfs_dir2_dataptr_t	diroffset,
+	struct xfs_defer_ops	*dfops,
+	xfs_fsblock_t		*firstblock)
+{
+	struct xfs_parent_name_irec nrec;
+
+	nrec.p_ino = parent->i_ino;
+	nrec.p_gen = VFS_I(parent)->i_generation;
+	nrec.p_diroffset = diroffset;
+	nrec.p_name = child_name->name;
+	nrec.p_namelen = child_name->len;
+
+	return xfs_parent_create_nrec(tp, child, &nrec, dfops, firstblock);
+}
diff --git a/fs/xfs/xfs_attr.h b/fs/xfs/xfs_attr.h
index 3031a09..fad2fbd 100644
--- a/fs/xfs/xfs_attr.h
+++ b/fs/xfs/xfs_attr.h
@@ -155,5 +155,16 @@ int xfs_attr_remove(struct xfs_inode *dp, const unsigned char *name, int flags);
 int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
 		  int flags, struct attrlist_cursor_kern *cursor);
 
-
+/*
+ * Parent pointer attribute prototypes
+ */
+int xfs_parent_create(struct xfs_trans *tp, struct xfs_inode *parent,
+		      struct xfs_inode *child, struct xfs_name *child_name,
+		      xfs_dir2_dataptr_t diroffset, struct xfs_defer_ops *dfops,
+		      xfs_fsblock_t *firstblock);
+int xfs_attr_set_first_parent(struct xfs_trans *tp, struct xfs_inode *ip,
+			      struct xfs_parent_name_rec *rec, int reclen,
+			      const char *value, int valuelen,
+			      struct xfs_defer_ops *dfops,
+			      xfs_fsblock_t *firstblock);
 #endif	/* __XFS_ATTR_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 52c331e..3557791 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1162,6 +1162,7 @@ xfs_create(
 	struct xfs_dquot	*pdqp = NULL;
 	struct xfs_trans_res	*tres;
 	uint			resblks;
+	xfs_dir2_dataptr_t	diroffset;
 
 	trace_xfs_create(dp, name);
 
@@ -1251,7 +1252,7 @@ xfs_create(
 	error = xfs_dir_createname(tp, dp, name, ip->i_ino,
 					&first_block, &dfops, resblks ?
 					resblks - XFS_IALLOC_SPACE_RES(mp) : 0,
-					NULL);
+					&diroffset);
 	if (error) {
 		ASSERT(error != -ENOSPC);
 		goto out_trans_cancel;
@@ -1270,6 +1271,19 @@ xfs_create(
 	}
 
 	/*
+	 * If we have parent pointers, we need to add the attribute containing
+	 * the parent information now. This must be done within the same
+	 * transaction the directory entry is created, while the new inode
+	 * contains nothing in the inode literal area.
+	 */
+	if (xfs_sb_version_hasparent(&mp->m_sb)) {
+		error = xfs_parent_create(tp, dp, ip, name, diroffset,
+					  &dfops, &first_block);
+		if (error)
+			goto out_bmap_cancel;
+	}
+
+	/*
 	 * If this is a synchronous mount, make sure that the
 	 * create transaction goes to disk before returning to
 	 * the user.
-- 
2.7.4

[RFC PATCH 09/13] xfs: add parent attributes to link

[Allison Henderson]

From: Dave Chinner <dchinner@redhat.com>

[bfoster: rebase, use VFS inode fields, fix xfs_bmap_finish() usage]
[achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t,
	   fixed null pointer bugs]

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 ed13c67... 49e1187... M	fs/xfs/libxfs/xfs_attr.c
:100644 100644 88f7edc... 0707336... M	fs/xfs/libxfs/xfs_parent.c
:100644 100644 fad2fbd... 17fdd5f... M	fs/xfs/xfs_attr.h
:100644 100644 3557791... 09caea7... M	fs/xfs/xfs_inode.c
 fs/xfs/libxfs/xfs_attr.c   | 221 +++++++++++++++++++++++++++------------------
 fs/xfs/libxfs/xfs_parent.c |  43 +++++++++
 fs/xfs/xfs_attr.h          |  10 ++
 fs/xfs/xfs_inode.c         |  66 +++++++++++---
 4 files changed, 240 insertions(+), 100 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index ed13c67..49e1187 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -35,6 +35,7 @@
 #include "xfs_bmap_util.h"
 #include "xfs_bmap_btree.h"
 #include "xfs_attr.h"
+#include "xfs_attr_sf.h"
 #include "xfs_attr_leaf.h"
 #include "xfs_attr_remote.h"
 #include "xfs_error.h"
@@ -273,6 +274,129 @@ xfs_attr_set_first_parent(
 	return xfs_attr_leaf_addname(&args);
 }
 
+/*
+ * set the attribute specified in @args. In the case of the parent attribute
+ * being set, we do not want to roll the transaction on shortform-to-leaf
+ * conversion, as the attribute must be added in the same transaction as the
+ * parent directory modifications. Hence @roll_trans needs to be set
+ * appropriately to control whether the transaction is committed during this
+ * function.
+ */
+static int
+xfs_attr_set_args(
+	struct xfs_da_args	*args,
+	int			flags,
+	bool			roll_trans)
+{
+	struct xfs_inode	*dp = args->dp;
+	int			error = 0;
+
+	/*
+	 * If the attribute list is non-existent or a shortform list,
+	 * upgrade it to a single-leaf-block attribute list.
+	 */
+	if (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL ||
+	    (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
+	     dp->i_d.di_anextents == 0)) {
+
+		/*
+		 * Build initial attribute list (if required).
+		 */
+		if (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS)
+			xfs_attr_shortform_create(args);
+
+		/*
+		 * Try to add the attr to the attribute list in the inode.
+		 */
+		error = xfs_attr_shortform_addname(args);
+		if (error != -ENOSPC) {
+			ASSERT(args->trans);
+			if (!error && (flags & ATTR_KERNOTIME) == 0)
+				xfs_trans_ichgtime(args->trans, dp,
+						   XFS_ICHGTIME_CHG);
+			goto out;
+		}
+
+		/*
+		 * It won't fit in the shortform, transform to a leaf block.
+		 * GROT: another possible req'mt for a double-split btree op.
+		 */
+		error = xfs_attr_shortform_to_leaf(args);
+		if (error)
+			goto out;
+		if (roll_trans) {
+			error = xfs_defer_finish(&args->trans, args->dfops, dp);
+			if (error) {
+				args->trans = NULL;
+				goto out;
+			}
+
+			/*
+			 * Commit the leaf transformation.  We'll need another
+			 * (linked) transaction to add the new attribute to the
+			 * leaf.
+			 */
+			error = xfs_trans_roll(&args->trans, dp);
+			if (error)
+				goto out;
+		}
+	}
+
+	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
+		error = xfs_attr_leaf_addname(args);
+	else
+		error = xfs_attr_node_addname(args);
+	if (error)
+		goto out;
+
+	if ((flags & ATTR_KERNOTIME) == 0)
+		xfs_trans_ichgtime(args->trans, dp, XFS_ICHGTIME_CHG);
+
+	xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE);
+out:
+	return error;
+}
+
+int
+xfs_attr_set_parent(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip,
+	struct xfs_parent_name_rec *rec,
+	int			reclen,
+	const char		*value,
+	int			valuelen,
+	struct xfs_defer_ops	*dfops,
+	xfs_fsblock_t		*firstblock)
+{
+	struct xfs_da_args	args;
+	int			flags = ATTR_PARENT;
+	int			local;
+	int			error;
+
+	ASSERT(XFS_IFORK_Q(ip));
+
+	tp->t_flags |= XFS_TRANS_RESERVE;
+
+	error = xfs_attr_args_init(&args, ip, (char *)rec, flags);
+	if (error)
+		return error;
+
+	args.name = (char *)rec;
+	args.namelen = reclen;
+	args.hashval = xfs_da_hashname(args.name, args.namelen);
+	args.value = (char *)value;
+	args.valuelen = valuelen;
+	args.firstblock = firstblock;
+	args.dfops = dfops;
+	args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
+	args.total = xfs_attr_calc_size(&args, &local);
+	args.trans = tp;
+	ASSERT(local);
+	args.trans = tp;
+
+	return xfs_attr_set_args(&args, flags, false);
+}
+
 int
 xfs_attr_set(
 	struct xfs_inode	*dp,
@@ -287,7 +411,7 @@ xfs_attr_set(
 	struct xfs_trans_res	tres;
 	xfs_fsblock_t		firstblock;
 	bool			rsvd = (flags & (ATTR_ROOT | ATTR_PARENT)) != 0;
-	int			error, err2, local;
+	int			error, local;
 
 	XFS_STATS_INC(mp, xs_attr_set);
 
@@ -340,89 +464,15 @@ xfs_attr_set(
 	error = xfs_trans_reserve_quota_nblks(args.trans, dp, args.total, 0,
 				rsvd ? XFS_QMOPT_RES_REGBLKS | XFS_QMOPT_FORCE_RES :
 				       XFS_QMOPT_RES_REGBLKS);
-	if (error) {
-		xfs_iunlock(dp, XFS_ILOCK_EXCL);
-		xfs_trans_cancel(args.trans);
-		return error;
-	}
+	if (error)
+		goto out_cancel_unlock;
 
 	xfs_trans_ijoin(args.trans, dp, 0);
+	xfs_defer_init(args.dfops, args.firstblock);
 
-	/*
-	 * If the attribute list is non-existent or a shortform list,
-	 * upgrade it to a single-leaf-block attribute list.
-	 */
-	if (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL ||
-	    (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
-	     dp->i_d.di_anextents == 0)) {
-
-		/*
-		 * Build initial attribute list (if required).
-		 */
-		if (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS)
-			xfs_attr_shortform_create(&args);
-
-		/*
-		 * Try to add the attr to the attribute list in
-		 * the inode.
-		 */
-		error = xfs_attr_shortform_addname(&args);
-		if (error != -ENOSPC) {
-			/*
-			 * Commit the shortform mods, and we're done.
-			 * NOTE: this is also the error path (EEXIST, etc).
-			 */
-			ASSERT(args.trans != NULL);
-
-			/*
-			 * If this is a synchronous mount, make sure that
-			 * the transaction goes to disk before returning
-			 * to the user.
-			 */
-			if (mp->m_flags & XFS_MOUNT_WSYNC)
-				xfs_trans_set_sync(args.trans);
-
-			if (!error && (flags & ATTR_KERNOTIME) == 0) {
-				xfs_trans_ichgtime(args.trans, dp,
-							XFS_ICHGTIME_CHG);
-			}
-			err2 = xfs_trans_commit(args.trans);
-			xfs_iunlock(dp, XFS_ILOCK_EXCL);
-
-			return error ? error : err2;
-		}
-
-		/*
-		 * It won't fit in the shortform, transform to a leaf block.
-		 * GROT: another possible req'mt for a double-split btree op.
-		 */
-		xfs_defer_init(args.dfops, args.firstblock);
-		error = xfs_attr_shortform_to_leaf(&args);
-		if (!error)
-			error = xfs_defer_finish(&args.trans, args.dfops, dp);
-		if (error) {
-			args.trans = NULL;
-			xfs_defer_cancel(&dfops);
-			goto out;
-		}
-
-		/*
-		 * Commit the leaf transformation.  We'll need another (linked)
-		 * transaction to add the new attribute to the leaf.
-		 */
-
-		error = xfs_trans_roll(&args.trans, dp);
-		if (error)
-			goto out;
-
-	}
-
-	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
-		error = xfs_attr_leaf_addname(&args);
-	else
-		error = xfs_attr_node_addname(&args);
+	error = xfs_attr_set_args(&args, flags, true);
 	if (error)
-		goto out;
+		goto out_cancel_defer;
 
 	/*
 	 * If this is a synchronous mount, make sure that the
@@ -431,22 +481,21 @@ xfs_attr_set(
 	if (mp->m_flags & XFS_MOUNT_WSYNC)
 		xfs_trans_set_sync(args.trans);
 
-	if ((flags & ATTR_KERNOTIME) == 0)
-		xfs_trans_ichgtime(args.trans, dp, XFS_ICHGTIME_CHG);
-
 	/*
 	 * Commit the last in the sequence of transactions.
 	 */
 	xfs_trans_log_inode(args.trans, dp, XFS_ILOG_CORE);
 	error = xfs_trans_commit(args.trans);
-	xfs_iunlock(dp, XFS_ILOCK_EXCL);
 
+	xfs_iunlock(dp, XFS_ILOCK_EXCL);
 	return error;
 
-out:
+out_cancel_defer:
+	xfs_defer_cancel(&dfops);
+out_cancel_unlock:
+	xfs_iunlock(dp, XFS_ILOCK_EXCL);
 	if (args.trans)
 		xfs_trans_cancel(args.trans);
-	xfs_iunlock(dp, XFS_ILOCK_EXCL);
 	return error;
 }
 
diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c
index 88f7edc..0707336 100644
--- a/fs/xfs/libxfs/xfs_parent.c
+++ b/fs/xfs/libxfs/xfs_parent.c
@@ -96,3 +96,46 @@ xfs_parent_create(
 
 	return xfs_parent_create_nrec(tp, child, &nrec, dfops, firstblock);
 }
+
+static int
+xfs_parent_add_nrec(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*child,
+	struct xfs_parent_name_irec *nrec,
+	struct xfs_defer_ops	*dfops,
+	xfs_fsblock_t		*firstblock)
+{
+	struct xfs_parent_name_rec rec;
+
+	rec.p_ino = cpu_to_be64(nrec->p_ino);
+	rec.p_gen = cpu_to_be32(nrec->p_gen);
+	rec.p_diroffset = cpu_to_be32(nrec->p_diroffset);
+
+	return xfs_attr_set_parent(tp, child, &rec, sizeof(rec),
+				   nrec->p_name, nrec->p_namelen,
+				   dfops, firstblock);
+}
+
+/*
+ * Add a parent record to an inode with existing parent records.
+ */
+int
+xfs_parent_add(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*parent,
+	struct xfs_inode	*child,
+	struct xfs_name		*child_name,
+	uint32_t		diroffset,
+	struct xfs_defer_ops	*dfops,
+	xfs_fsblock_t		*firstblock)
+{
+	struct xfs_parent_name_irec nrec;
+
+	nrec.p_ino = parent->i_ino;
+	nrec.p_gen = VFS_I(parent)->i_generation;
+	nrec.p_diroffset = diroffset;
+	nrec.p_name = child_name->name;
+	nrec.p_namelen = child_name->len;
+
+	return xfs_parent_add_nrec(tp, child, &nrec, dfops, firstblock);
+}
diff --git a/fs/xfs/xfs_attr.h b/fs/xfs/xfs_attr.h
index fad2fbd..17fdd5f 100644
--- a/fs/xfs/xfs_attr.h
+++ b/fs/xfs/xfs_attr.h
@@ -167,4 +167,14 @@ int xfs_attr_set_first_parent(struct xfs_trans *tp, struct xfs_inode *ip,
 			      const char *value, int valuelen,
 			      struct xfs_defer_ops *dfops,
 			      xfs_fsblock_t *firstblock);
+
+int xfs_parent_add(struct xfs_trans *tp, struct xfs_inode *parent,
+		   struct xfs_inode *child, struct xfs_name *child_name,
+		   xfs_dir2_dataptr_t diroffset, struct xfs_defer_ops *dfops,
+		   xfs_fsblock_t *firstblock);
+int xfs_attr_set_parent(struct xfs_trans *tp, struct xfs_inode *ip,
+			struct xfs_parent_name_rec *rec, int reclen,
+			const char *value, int valuelen,
+			struct xfs_defer_ops *dfops, xfs_fsblock_t *firstblock);
+
 #endif	/* __XFS_ATTR_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 3557791..09caea7 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1449,6 +1449,8 @@ xfs_link(
 	struct xfs_defer_ops	dfops;
 	xfs_fsblock_t           first_block;
 	int			resblks;
+	uint32_t		diroffset;
+	bool			first_parent = false;
 
 	trace_xfs_link(tdp, target_name);
 
@@ -1465,6 +1467,25 @@ xfs_link(
 	if (error)
 		goto std_return;
 
+	/*
+	 * If we have parent pointers and there is no attribute fork (i.e. we
+	 * are linking in a O_TMPFILE created inode) we need to add the
+	 * attribute fork to the inode. Because we may have an existing data
+	 * fork, we do this before we start the link transaction as adding an
+	 * attribute fork requires it's own transaction.
+	 */
+	if (xfs_sb_version_hasparent(&mp->m_sb) && !xfs_inode_hasattr(sip)) {
+		int sf_size = sizeof(struct xfs_attr_sf_hdr) +
+				XFS_ATTR_SF_ENTSIZE_BYNAME(
+					sizeof(struct xfs_parent_name_rec),
+					target_name->len);
+		ASSERT(VFS_I(sip)->i_nlink == 0);
+		error = xfs_bmap_add_attrfork(sip, sf_size, 0);
+		if (error)
+			goto std_return;
+		first_parent = true;
+	}
+
 	resblks = XFS_LINK_SPACE_RES(mp, target_name->len);
 	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_link, resblks, 0, 0, &tp);
 	if (error == -ENOSPC) {
@@ -1496,8 +1517,6 @@ xfs_link(
 			goto error_return;
 	}
 
-	xfs_defer_init(&dfops, &first_block);
-
 	/*
 	 * Handle initial link state of O_TMPFILE inode
 	 */
@@ -1507,36 +1526,55 @@ xfs_link(
 			goto error_return;
 	}
 
+	xfs_defer_init(&dfops, &first_block);
 	error = xfs_dir_createname(tp, tdp, target_name, sip->i_ino,
-				   &first_block, &dfops, resblks, NULL);
+				   &first_block, &dfops, resblks, &diroffset);
 	if (error)
-		goto error_return;
+		goto out_defer_cancel;
 	xfs_trans_ichgtime(tp, tdp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
 	xfs_trans_log_inode(tp, tdp, XFS_ILOG_CORE);
 
 	error = xfs_bumplink(tp, sip);
 	if (error)
-		goto error_return;
+		goto out_defer_cancel;
 
 	/*
-	 * If this is a synchronous mount, make sure that the
-	 * link transaction goes to disk before returning to
-	 * the user.
+	 * If we have parent pointers, we now need to add the parent record to
+	 * the attribute fork of the inode. If this is the initial parent
+	 * atribute, we need to create it correctly, otherwise we can just add
+	 * the parent to the inode.
+	 */
+	if (xfs_sb_version_hasparent(&mp->m_sb)) {
+		if (first_parent)
+			error = xfs_parent_create(tp, tdp, sip, target_name,
+						  diroffset, &dfops,
+						  &first_block);
+		else
+			error = xfs_parent_add(tp, tdp, sip, target_name,
+					       diroffset, &dfops,
+					       &first_block);
+		if (error)
+			goto out_defer_cancel;
+	}
+
+	/*
+	 * If this is a synchronous mount, make sure that the link transaction
+	 * goes to disk before returning to the user.
 	 */
 	if (mp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
 		xfs_trans_set_sync(tp);
 
 	error = xfs_defer_finish(&tp, &dfops, NULL);
-	if (error) {
-		xfs_defer_cancel(&dfops);
-		goto error_return;
-	}
+	if (error)
+		goto out_defer_cancel;
 
 	return xfs_trans_commit(tp);
 
- error_return:
+out_defer_cancel:
+	xfs_defer_cancel(&dfops);
+error_return:
 	xfs_trans_cancel(tp);
- std_return:
+std_return:
 	return error;
 }
 
-- 
2.7.4

[RFC PATCH 10/13] xfs: remove parent pointers in unlink

[Allison Henderson]

From: Dave Chinner <dchinner@redhat.com>

[bfoster: rebase, use VFS inode generation]
[achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t
	   implemented xfs_attr_remove_parent]

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 49e1187... 80431cb... M	fs/xfs/libxfs/xfs_attr.c
:100644 100644 0707336... ca695c4... M	fs/xfs/libxfs/xfs_parent.c
:100644 100644 17fdd5f... 374e8da... M	fs/xfs/xfs_attr.h
:100644 100644 09caea7... 0654ad8b.. M	fs/xfs/xfs_inode.c
:100644 100644 f013c893.. 6dcd89a... M	fs/xfs/xfs_qm.c
:100644 100644 2975a82... 9976369... M	fs/xfs/xfs_qm.h
 fs/xfs/libxfs/xfs_attr.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_parent.c | 22 ++++++++++++++
 fs/xfs/xfs_attr.h          |  7 +++++
 fs/xfs/xfs_inode.c         | 10 +++++-
 fs/xfs/xfs_qm.c            |  2 +-
 fs/xfs/xfs_qm.h            |  1 +
 6 files changed, 116 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 49e1187..80431cb 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -42,6 +42,7 @@
 #include "xfs_quota.h"
 #include "xfs_trans_space.h"
 #include "xfs_trace.h"
+#include "xfs_qm.h"
 
 /*
  * xfs_attr.c
@@ -499,6 +500,81 @@ xfs_attr_set(
 	return error;
 }
 
+int
+xfs_attr_remove_parent(
+	struct xfs_trans		*tp,
+	struct xfs_inode		*dp,
+	struct xfs_parent_name_rec	*rec,
+	int				reclen,
+	struct xfs_defer_ops		*dfops,
+	xfs_fsblock_t			*firstblock)
+{
+	int flags = ATTR_PARENT;
+	char *name = (char *)rec;
+	struct xfs_mount        *mp = dp->i_mount;
+	struct xfs_da_args      args;
+	int                     error;
+	int                     rsvd = 0;
+
+	XFS_STATS_INC(mp, xs_attr_remove);
+
+	if (XFS_FORCED_SHUTDOWN(dp->i_mount))
+		return -EIO;
+
+	error = xfs_attr_args_init(&args, dp, name, flags);
+	if (error)
+		return error;
+
+	args.firstblock = firstblock;
+	args.dfops = dfops;
+	args.name = (char *)rec;
+	args.namelen = reclen;
+
+	/*
+	 * we have no control over the attribute names that userspace passes us
+	 * to remove, so we have to allow the name lookup prior to attribute
+	 * removal to fail.
+	 */
+	args.op_flags = XFS_DA_OP_OKNOENT;
+
+	if (xfs_qm_need_dqattach(dp)) {
+		error = xfs_qm_dqattach_locked(dp, 0);
+		if (error)
+			return error;
+	}
+
+	/*
+	 * Root fork attributes can use reserved data blocks for this
+	 * operation if necessary
+	 */
+	if (flags & (ATTR_ROOT | ATTR_PARENT))
+		rsvd = XFS_TRANS_RESERVE;
+
+	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_attrrm,
+				XFS_ATTRRM_SPACE_RES(mp), 0, rsvd, &args.trans);
+	if (error)
+		return error;
+
+	if (!xfs_inode_hasattr(dp))
+		error = -ENOATTR;
+	else if (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
+		ASSERT(dp->i_afp->if_flags & XFS_IFINLINE);
+		error = xfs_attr_shortform_remove(&args);
+	} else if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
+		error = xfs_attr_leaf_removename(&args);
+	else
+		error = xfs_attr_node_removename(&args);
+
+	/*
+	 * If this is a synchronous mount, make sure that the
+	 * transaction goes to disk before returning to the user.
+	 */
+	if (!error && mp->m_flags & XFS_MOUNT_WSYNC)
+		xfs_trans_set_sync(args.trans);
+
+	return error;
+}
+
 /*
  * Generic handler routine to remove a name from an attribute list.
  * Transitions attribute list from Btree to shortform as necessary.
diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c
index 0707336..ca695c4 100644
--- a/fs/xfs/libxfs/xfs_parent.c
+++ b/fs/xfs/libxfs/xfs_parent.c
@@ -139,3 +139,25 @@ xfs_parent_add(
 
 	return xfs_parent_add_nrec(tp, child, &nrec, dfops, firstblock);
 }
+
+/*
+ * Remove a parent record from a child inode.
+ */
+int
+xfs_parent_remove(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*parent,
+	struct xfs_inode	*child,
+	xfs_dir2_dataptr_t	diroffset,
+	struct xfs_defer_ops	*dfops,
+	xfs_fsblock_t		*firstblock)
+{
+	struct xfs_parent_name_rec rec;
+
+	rec.p_ino = cpu_to_be64(parent->i_ino);
+	rec.p_gen = cpu_to_be32(VFS_I(parent)->i_generation);
+	rec.p_diroffset = cpu_to_be32(diroffset);
+
+	return xfs_attr_remove_parent(tp, child, &rec, sizeof(rec),
+				      dfops, firstblock);
+}
diff --git a/fs/xfs/xfs_attr.h b/fs/xfs/xfs_attr.h
index 17fdd5f..374e8da 100644
--- a/fs/xfs/xfs_attr.h
+++ b/fs/xfs/xfs_attr.h
@@ -177,4 +177,11 @@ int xfs_attr_set_parent(struct xfs_trans *tp, struct xfs_inode *ip,
 			const char *value, int valuelen,
 			struct xfs_defer_ops *dfops, xfs_fsblock_t *firstblock);
 
+int xfs_parent_remove(struct xfs_trans *tp, struct xfs_inode *parent,
+		      struct xfs_inode *child, xfs_dir2_dataptr_t diroffset,
+		      struct xfs_defer_ops *dfops, xfs_fsblock_t *firstblock);
+int xfs_attr_remove_parent(struct xfs_trans *tp, struct xfs_inode *ip,
+			struct xfs_parent_name_rec *rec, int reclen,
+			struct xfs_defer_ops *dfops, xfs_fsblock_t *firstblock);
+
 #endif	/* __XFS_ATTR_H__ */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 09caea7..0654ad8b 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2594,6 +2594,7 @@ xfs_remove(
 	struct xfs_defer_ops	dfops;
 	xfs_fsblock_t           first_block;
 	uint			resblks;
+	uint32_t		dir_offset;
 
 	trace_xfs_remove(dp, name);
 
@@ -2674,12 +2675,19 @@ xfs_remove(
 
 	xfs_defer_init(&dfops, &first_block);
 	error = xfs_dir_removename(tp, dp, name, ip->i_ino, &first_block,
-				   &dfops, resblks, NULL);
+				   &dfops, resblks, &dir_offset);
 	if (error) {
 		ASSERT(error != -ENOENT);
 		goto out_bmap_cancel;
 	}
 
+	if (xfs_sb_version_hasparent(&mp->m_sb)) {
+		error = xfs_parent_remove(tp, dp, ip, dir_offset, &dfops,
+					  &first_block);
+		if (error)
+			goto out_bmap_cancel;
+	}
+
 	/*
 	 * If this is a synchronous mount, make sure that the
 	 * remove transaction goes to disk before returning to
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index f013c893..6dcd89a 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -305,7 +305,7 @@ xfs_qm_dqattach_one(
 	return 0;
 }
 
-static bool
+bool
 xfs_qm_need_dqattach(
 	struct xfs_inode	*ip)
 {
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index 2975a82..9976369 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -176,6 +176,7 @@ extern int		xfs_qm_scall_setqlim(struct xfs_mount *, xfs_dqid_t, uint,
 					struct qc_dqblk *);
 extern int		xfs_qm_scall_quotaon(struct xfs_mount *, uint);
 extern int		xfs_qm_scall_quotaoff(struct xfs_mount *, uint);
+extern bool		xfs_qm_need_dqattach(struct xfs_inode *ip);
 
 static inline struct xfs_def_quota *
 xfs_get_defquota(struct xfs_dquot *dqp, struct xfs_quotainfo *qi)
-- 
2.7.4

[RFC PATCH 11/13] xfs_bmap_add_attrfork(): re-add error handling from set_attrforkoff() call

[Allison Henderson]

From: Brian Foster <bfoster@redhat.com>

- fix for "xfs: parent pointer attribute creation"

[achender: rebased]

Signed-off-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 91c403e... 0cb6d2e... M	fs/xfs/libxfs/xfs_bmap.c
 fs/xfs/libxfs/xfs_bmap.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 91c403e..0cb6d2e 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1159,7 +1159,9 @@ xfs_bmap_add_attrfork(
 	xfs_trans_ijoin(tp, ip, 0);
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 
-	xfs_bmap_set_attrforkoff(ip, size, &version);
+	error = xfs_bmap_set_attrforkoff(ip, size, &version);
+	if (error)
+		goto trans_cancel;
 
 	ASSERT(ip->i_afp == NULL);
 	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, KM_SLEEP);
-- 
2.7.4

[RFC PATCH 12/13] Add parent pointers to rename

[Allison Henderson]

Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 f5d0d3d... bd99ae2... M	fs/xfs/libxfs/xfs_dir2.c
:100644 100644 0654ad8b.. 2bdade2... M	fs/xfs/xfs_inode.c
 fs/xfs/libxfs/xfs_dir2.c |  6 ++++--
 fs/xfs/xfs_inode.c       | 26 ++++++++++++++++++++------
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index f5d0d3d..bd99ae2 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -324,10 +324,11 @@ xfs_dir_createname(
 	else
 		rval = xfs_dir2_node_addname(args);
 
+out_free:
 	/* return the location that this entry was place in the parent inode */
 	if (offset)
 		*offset = args->offset;
-out_free:
+
 	kmem_free(args);
 	return rval;
 }
@@ -496,9 +497,10 @@ xfs_dir_removename(
 		rval = xfs_dir2_leaf_removename(args);
 	else
 		rval = xfs_dir2_node_removename(args);
+out_free:
 	if (offset)
 		*offset = args->offset;
-out_free:
+
 	kmem_free(args);
 	return rval;
 }
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 0654ad8b..2bdade2 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2971,6 +2971,8 @@ xfs_rename(
 	bool			src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode);
 	int			spaceres;
 	int			error;
+	xfs_dir2_dataptr_t	new_diroffset;
+	xfs_dir2_dataptr_t	old_diroffset;
 
 	trace_xfs_rename(src_dp, target_dp, src_name, target_name);
 
@@ -3073,13 +3075,12 @@ xfs_rename(
 		 */
 		error = xfs_dir_createname(tp, target_dp, target_name,
 					   src_ip->i_ino, &first_block, &dfops,
-					   spaceres, NULL);
+					   spaceres, &new_diroffset);
 		if (error)
 			goto out_bmap_cancel;
 
 		xfs_trans_ichgtime(tp, target_dp,
 					XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-
 		if (new_parent && src_is_directory) {
 			error = xfs_bumplink(tp, target_dp);
 			if (error)
@@ -3113,7 +3114,7 @@ xfs_rename(
 		 */
 		error = xfs_dir_replace(tp, target_dp, target_name,
 					src_ip->i_ino, &first_block, &dfops,
-					spaceres, NULL);
+					spaceres, &new_diroffset);
 		if (error)
 			goto out_bmap_cancel;
 
@@ -3148,7 +3149,7 @@ xfs_rename(
 		 */
 		error = xfs_dir_replace(tp, src_ip, &xfs_name_dotdot,
 					target_dp->i_ino, &first_block, &dfops,
-					spaceres, NULL);
+					spaceres, &new_diroffset);
 		ASSERT(error != -EEXIST);
 		if (error)
 			goto out_bmap_cancel;
@@ -3187,11 +3188,12 @@ xfs_rename(
 	 */
 	if (wip) {
 		error = xfs_dir_replace(tp, src_dp, src_name, wip->i_ino,
-					&first_block, &dfops, spaceres, NULL);
+					&first_block, &dfops, spaceres,
+					&old_diroffset);
 	} else
 		error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino,
 					   &first_block, &dfops, spaceres,
-					   NULL);
+					   &old_diroffset);
 	if (error)
 		goto out_bmap_cancel;
 
@@ -3221,6 +3223,18 @@ xfs_rename(
 		VFS_I(wip)->i_state &= ~I_LINKABLE;
 	}
 
+	if (new_parent && xfs_sb_version_hasparent(&mp->m_sb)) {
+		error = xfs_parent_add(tp, target_dp, src_ip, target_name,
+				       new_diroffset, &dfops, &first_block);
+		if (error)
+			goto out_bmap_cancel;
+
+		error = xfs_parent_remove(tp, src_dp, src_ip,
+					  old_diroffset, &dfops, &first_block);
+		if (error)
+			goto out_bmap_cancel;
+	}
+
 	xfs_trans_ichgtime(tp, src_dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
 	xfs_trans_log_inode(tp, src_dp, XFS_ILOG_CORE);
 	if (new_parent)
-- 
2.7.4

[RFC PATCH 13/13] Add the parent pointer support to the superblock version 5.

[Allison Henderson]

[dchinner: forward ported and cleaned up]
[achender: rebased and added parent pointer attribute to
           compatible attributes mask]

Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
:100644 100644 6cedd75... b00bf1d... M	fs/xfs/libxfs/xfs_format.h
:100644 100644 fba0105... a18dd0c... M	fs/xfs/libxfs/xfs_fs.h
:100644 100644 50fb3a2... d7b9010... M	fs/xfs/xfs_fsops.c
 fs/xfs/libxfs/xfs_format.h | 13 ++++++++-----
 fs/xfs/libxfs/xfs_fs.h     |  1 +
 fs/xfs/xfs_fsops.c         |  4 +++-
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 6cedd75..b00bf1d 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -464,10 +464,12 @@ xfs_sb_has_compat_feature(
 #define XFS_SB_FEAT_RO_COMPAT_FINOBT   (1 << 0)		/* free inode btree */
 #define XFS_SB_FEAT_RO_COMPAT_RMAPBT   (1 << 1)		/* reverse map btree */
 #define XFS_SB_FEAT_RO_COMPAT_REFLINK  (1 << 2)		/* reflinked files */
+#define XFS_SB_FEAT_RO_COMPAT_PARENT	(1 << 3)	/* parent inode ptr */
 #define XFS_SB_FEAT_RO_COMPAT_ALL \
 		(XFS_SB_FEAT_RO_COMPAT_FINOBT | \
 		 XFS_SB_FEAT_RO_COMPAT_RMAPBT | \
-		 XFS_SB_FEAT_RO_COMPAT_REFLINK)
+		 XFS_SB_FEAT_RO_COMPAT_REFLINK| \
+		 XFS_SB_FEAT_RO_COMPAT_PARENT)
 #define XFS_SB_FEAT_RO_COMPAT_UNKNOWN	~XFS_SB_FEAT_RO_COMPAT_ALL
 static inline bool
 xfs_sb_has_ro_compat_feature(
@@ -507,17 +509,17 @@ xfs_sb_has_incompat_log_feature(
 /*
  * V5 superblock specific feature checks
  */
-static inline int xfs_sb_version_hascrc(struct xfs_sb *sbp)
+static inline bool xfs_sb_version_hascrc(struct xfs_sb *sbp)
 {
 	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
 }
 
-static inline int xfs_sb_version_has_pquotino(struct xfs_sb *sbp)
+static inline bool xfs_sb_version_has_pquotino(struct xfs_sb *sbp)
 {
 	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
 }
 
-static inline int xfs_sb_version_hasftype(struct xfs_sb *sbp)
+static inline bool xfs_sb_version_hasftype(struct xfs_sb *sbp)
 {
 	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
 		xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_FTYPE)) ||
@@ -563,7 +565,8 @@ static inline bool xfs_sb_version_hasreflink(struct xfs_sb *sbp)
 
 static inline bool xfs_sb_version_hasparent(struct xfs_sb *sbp)
 {
-	return false; /* We'll enable this at the end of the set */
+	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_PARENT));
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index fba0105..a18dd0c 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -222,6 +222,7 @@ typedef struct xfs_fsop_resblks {
 #define XFS_FSOP_GEOM_FLAGS_SPINODES	0x40000	/* sparse inode chunks	*/
 #define XFS_FSOP_GEOM_FLAGS_RMAPBT	0x80000	/* reverse mapping btree */
 #define XFS_FSOP_GEOM_FLAGS_REFLINK	0x100000 /* files can share blocks */
+#define XFS_FSOP_GEOM_FLAGS_PARENT	0x200000 /* parent pointers */
 
 /*
  * Minimum and maximum sizes need for growth checks.
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 50fb3a2..d7b9010 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -112,7 +112,9 @@ xfs_fs_geometry(
 			(xfs_sb_version_hasrmapbt(&mp->m_sb) ?
 				XFS_FSOP_GEOM_FLAGS_RMAPBT : 0) |
 			(xfs_sb_version_hasreflink(&mp->m_sb) ?
-				XFS_FSOP_GEOM_FLAGS_REFLINK : 0);
+				XFS_FSOP_GEOM_FLAGS_REFLINK : 0) |
+			(xfs_sb_version_hasparent(&mp->m_sb) ?
+				XFS_FSOP_GEOM_FLAGS_PARENT : 0);
 		geo->logsectsize = xfs_sb_version_hassector(&mp->m_sb) ?
 				mp->m_sb.sb_logsectsize : BBSIZE;
 		geo->rtsectsize = mp->m_sb.sb_blocksize;
-- 
2.7.4

Re: [RFC PATCH 02/13] xfs: get directory offset when removing directory name

[Darrick J. Wong]

On Wed, Aug 09, 2017 at 06:41:22PM -0700, Allison Henderson wrote:
> From: Mark Tinguely <tinguely@sgi.com>
> 
> Return the directory offset information when removing an entry to the
> directory.
> 
> This offset will be used as the parent pointer offset in xfs_remove.
> 
> [dchinner: forward ported and cleaned up]
> [achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t]
> 
> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
> ---
> :100644 100644 a1ca460... a34956e... M	fs/xfs/libxfs/xfs_dir2.c
> :100644 100644 e349900... ee98b2a... M	fs/xfs/libxfs/xfs_dir2.h
> :100644 100644 79684d5... 4dbe2fc... M	fs/xfs/libxfs/xfs_dir2_block.c
> :100644 100644 2ac7a7e... 197e627... M	fs/xfs/libxfs/xfs_dir2_leaf.c
> :100644 100644 8bc91f8... 13d5244... M	fs/xfs/libxfs/xfs_dir2_node.c
> :100644 100644 489bdef... 9e90c22... M	fs/xfs/libxfs/xfs_dir2_sf.c
> :100644 100644 cea1c56... f58f62a... M	fs/xfs/xfs_inode.c
>  fs/xfs/libxfs/xfs_dir2.c       | 15 +++++++++------
>  fs/xfs/libxfs/xfs_dir2.h       |  3 ++-
>  fs/xfs/libxfs/xfs_dir2_block.c |  4 ++--
>  fs/xfs/libxfs/xfs_dir2_leaf.c  |  5 +++--
>  fs/xfs/libxfs/xfs_dir2_node.c  |  5 +++--
>  fs/xfs/libxfs/xfs_dir2_sf.c    |  2 ++
>  fs/xfs/xfs_inode.c             |  7 ++++---
>  7 files changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index a1ca460..a34956e 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -443,13 +443,14 @@ xfs_dir_lookup(
>   */
>  int
>  xfs_dir_removename(
> -	xfs_trans_t	*tp,
> -	xfs_inode_t	*dp,
> -	struct xfs_name	*name,
> -	xfs_ino_t	ino,
> -	xfs_fsblock_t	*first,		/* bmap's firstblock */
> +	xfs_trans_t		*tp,

If you change a line containing a struct tyepdef type, please convert it
back to the raw struct type.

	struct xfs_trans	*tp,

etc.  Some day we'll have managed to remove them all! :D

> +	xfs_inode_t		*dp,
> +	struct xfs_name		*name,
> +	xfs_ino_t		ino,
> +	xfs_fsblock_t		*first,		/* bmap's firstblock */
>  	struct xfs_defer_ops	*dfops,		/* bmap's freeblock list */
> -	xfs_extlen_t	total)		/* bmap's total block count */
> +	xfs_extlen_t		total,		/* bmap's total block count */
> +	xfs_dir2_dataptr_t	*offset)	/* OUT: offset in directory */

Hmm.  We /also/ pass a defer_ops into _dir_removename.  I wasn't going
to say this until later in the patchset, but it's apropos now.

The biggest complaint I have about this patch series is that we end up
using separate transactions for each phase of the link/unlink
operations.  There's no guarantee that once we start the process of
expand-dir, add-name-to-dir, log-inode-cores, expand-attr,
add-pptr-to-attrs that we actually get all the way to the end of that
process, particularly if we crash in the middle and have to recover the
log coming back up.

Obviously, back in 2014 when Dave started on these patches there was no
defer_ops, so a lot of effort goes into passing parameters up and down
the stack and fiddling with the transaction reservation type
declarations in order to guarantee that we can shove everything into a
single transaction.  Now that we have a more general mechanism to split
complex updates into a chain of smaller updates (and make log recovery
pick up where we left off) it's time to figure out how to make attr (and
maybe dir) updates a deferrable operation.

This adds complexity to the log because we have to define an "attr add
KEY:VALUE" and a "attr delete KEY" deferred op that hooks into the
existing attr code.  For parent pointers it's not a big deal to log
values since they're not big, but for the general case this idea needs
more thought, or a decision that we'll just keep doing things the same
way.

Anyway, assuming the existence of a "attr delete KEY" log intent item we
no longer have to pass the offsets around everywhere because instead
_dir_removename attaches the defer item to the defer_ops and
_defer_finish takes care of rolling through the updates.

Thoughts?

--D

>  {
>  	struct xfs_da_args *args;
>  	int		rval;
> @@ -495,6 +496,8 @@ xfs_dir_removename(
>  		rval = xfs_dir2_leaf_removename(args);
>  	else
>  		rval = xfs_dir2_node_removename(args);
> +	if (offset)
> +		*offset = args->offset;
>  out_free:
>  	kmem_free(args);
>  	return rval;
> diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
> index e349900..ee98b2a 100644
> --- a/fs/xfs/libxfs/xfs_dir2.h
> +++ b/fs/xfs/libxfs/xfs_dir2.h
> @@ -139,7 +139,8 @@ extern int xfs_dir_lookup(struct xfs_trans *tp, struct xfs_inode *dp,
>  extern int xfs_dir_removename(struct xfs_trans *tp, struct xfs_inode *dp,
>  				struct xfs_name *name, xfs_ino_t ino,
>  				xfs_fsblock_t *first,
> -				struct xfs_defer_ops *dfops, xfs_extlen_t tot);
> +				struct xfs_defer_ops *dfops, xfs_extlen_t tot,
> +				xfs_dir2_dataptr_t *offset);
>  extern int xfs_dir_replace(struct xfs_trans *tp, struct xfs_inode *dp,
>  				struct xfs_name *name, xfs_ino_t inum,
>  				xfs_fsblock_t *first,
> diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
> index 79684d5..4dbe2fc 100644
> --- a/fs/xfs/libxfs/xfs_dir2_block.c
> +++ b/fs/xfs/libxfs/xfs_dir2_block.c
> @@ -791,9 +791,9 @@ xfs_dir2_block_removename(
>  	/*
>  	 * Point to the data entry using the leaf entry.
>  	 */
> +	args->offset = be32_to_cpu(blp[ent].address);
>  	dep = (xfs_dir2_data_entry_t *)((char *)hdr +
> -			xfs_dir2_dataptr_to_off(args->geo,
> -						be32_to_cpu(blp[ent].address)));
> +			xfs_dir2_dataptr_to_off(args->geo, args->offset));
>  	/*
>  	 * Mark the data entry's space free.
>  	 */
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index 2ac7a7e..197e627 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -1383,9 +1383,10 @@ xfs_dir2_leaf_removename(
>  	 * Point to the leaf entry, use that to point to the data entry.
>  	 */
>  	lep = &ents[index];
> -	db = xfs_dir2_dataptr_to_db(args->geo, be32_to_cpu(lep->address));
> +	args->offset = be32_to_cpu(lep->address);
> +	db = xfs_dir2_dataptr_to_db(args->geo, args->offset);
>  	dep = (xfs_dir2_data_entry_t *)((char *)hdr +
> -		xfs_dir2_dataptr_to_off(args->geo, be32_to_cpu(lep->address)));
> +		xfs_dir2_dataptr_to_off(args->geo, args->offset));
>  	needscan = needlog = 0;
>  	oldbest = be16_to_cpu(bf[0].length);
>  	ltp = xfs_dir2_leaf_tail_p(args->geo, leaf);
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index 8bc91f8..13d5244 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -1238,9 +1238,10 @@ xfs_dir2_leafn_remove(
>  	/*
>  	 * Extract the data block and offset from the entry.
>  	 */
> -	db = xfs_dir2_dataptr_to_db(args->geo, be32_to_cpu(lep->address));
> +	args->offset = be32_to_cpu(lep->address);
> +	db = xfs_dir2_dataptr_to_db(args->geo, args->offset);
>  	ASSERT(dblk->blkno == db);
> -	off = xfs_dir2_dataptr_to_off(args->geo, be32_to_cpu(lep->address));
> +	off = xfs_dir2_dataptr_to_off(args->geo, args->offset);
>  	ASSERT(dblk->index == off);
>  
>  	/*
> diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
> index 489bdef..9e90c22 100644
> --- a/fs/xfs/libxfs/xfs_dir2_sf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
> @@ -919,6 +919,8 @@ xfs_dir2_sf_removename(
>  								XFS_CMP_EXACT) {
>  			ASSERT(dp->d_ops->sf_get_ino(sfp, sfep) ==
>  			       args->inumber);
> +			args->offset = xfs_dir2_byte_to_dataptr(
> +						xfs_dir2_sf_get_offset(sfep));
>  			break;
>  		}
>  	}
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index cea1c56..f58f62a 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2621,8 +2621,8 @@ xfs_remove(
>  		goto out_trans_cancel;
>  
>  	xfs_defer_init(&dfops, &first_block);
> -	error = xfs_dir_removename(tp, dp, name, ip->i_ino,
> -					&first_block, &dfops, resblks);
> +	error = xfs_dir_removename(tp, dp, name, ip->i_ino, &first_block,
> +				   &dfops, resblks, NULL);
>  	if (error) {
>  		ASSERT(error != -ENOENT);
>  		goto out_bmap_cancel;
> @@ -3132,7 +3132,8 @@ xfs_rename(
>  					&first_block, &dfops, spaceres);
>  	} else
>  		error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino,
> -					   &first_block, &dfops, spaceres);
> +					   &first_block, &dfops, spaceres,
> +					   NULL);
>  	if (error)
>  		goto out_bmap_cancel;
>  
> -- 
> 2.7.4
> 
> --
> 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: [RFC PATCH 05/13] xfs: define parent pointer xattr format

[Darrick J. Wong]

On Wed, Aug 09, 2017 at 06:41:25PM -0700, Allison Henderson wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> We need to define the parent pointer attribute format before we
> start adding support for it into all the code that needs to use it.
> The EA format we will use encodes the following information:
> 
> 	name={parent inode #, parent inode generation, dirent offset}
> 	value={dirent filename}
> 
> The inode/gen gives all the information we need to reliably identify
> the parent without requiring child->parent lock ordering, and allows
> userspace to do pathname component level reconstruction without the
> kernel ever needing to verify the parent itself as part of ioctl
> calls.
> 
> By using the dirent offset in the EA name, we have a method of
> knowing the exact parent pointer EA we need to modify/remove in
> rename/unlink without an unbound EA name search.
> 
> By keeping the dirent name in the value, we have enough information
> to be able to validate and reconstruct damaged directory trees.
> While the diroffset of a filename alone is not unique enough to
> identify the child, the {diroffset,filename,child_inode} tuple is
> sufficient. That is, if the diroffset gets reused and points to a
> different filename, we can detect that from the contents of EA. If a
> link of the same name is created, then we can check whether it
> points at the same inode as the parent EA we current have.
> 
> [achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t]
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
> ---
> :100644 100644 d4d9bef... 91e2d5a... M	fs/xfs/libxfs/xfs_format.h
>  fs/xfs/libxfs/xfs_format.h | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index d4d9bef..91e2d5a 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -18,6 +18,8 @@
>  #ifndef __XFS_FORMAT_H__
>  #define __XFS_FORMAT_H__
>  
> +#include "xfs_da_format.h"
> +
>  /*
>   * XFS On Disk Format Definitions
>   *
> @@ -1721,4 +1723,29 @@ struct xfs_acl {
>  #define SGI_ACL_FILE_SIZE	(sizeof(SGI_ACL_FILE)-1)
>  #define SGI_ACL_DEFAULT_SIZE	(sizeof(SGI_ACL_DEFAULT)-1)
>  
> +/*
> + * Parent pointer attribute format definition
> + *
> + * EA name encodes the parent inode number, generation and the offset of
> + * the dirent that points to the child inode. The EA value contains the
> + * same name as the dirent in the parent directory.
> + */
> +struct xfs_parent_name_rec {
> +	__be64	p_ino;
> +	__be32	p_gen;
> +	__be32	p_diroffset;
> +};
> +
> +/*
> + * incore version of the above, also contains name pointers so callers
> + * can pass/obtain all the parent pointer information in a single structure
> + */
> +struct xfs_parent_name_irec {
> +	uint64_t		p_ino;

Should use incore xfs types, i.e. xfs_ino_t.

> +	uint32_t		p_gen;
> +	xfs_dir2_dataptr_t	p_diroffset;
> +	const char		*p_name;
> +	uint32_t		p_namelen;

Pathname components are at most 255 bytes long, though I guess that
doesn't save us any space here.

--D

> +};
> +
>  #endif /* __XFS_FORMAT_H__ */
> -- 
> 2.7.4
> 
> --
> 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: [RFC PATCH 08/13] xfs: parent pointer attribute creation

[Darrick J. Wong]

On Wed, Aug 09, 2017 at 06:41:28PM -0700, Allison Henderson wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> [bfoster: rebase, use VFS inode generation]
> [achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t,
> 	   fixed some null pointer bugs]
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
> ---
> :100644 100644 a2a5d04... 90b51ae... M	fs/xfs/Makefile
> :100644 100644 646d41b... ed13c67... M	fs/xfs/libxfs/xfs_attr.c
> :100644 100644 a2d6466... 91c403e... M	fs/xfs/libxfs/xfs_bmap.c
> :100644 100644 851982a... 533f40f... M	fs/xfs/libxfs/xfs_bmap.h
> :000000 100644 0000000... 88f7edc... A	fs/xfs/libxfs/xfs_parent.c
> :100644 100644 3031a09... fad2fbd... M	fs/xfs/xfs_attr.h
> :100644 100644 52c331e... 3557791... M	fs/xfs/xfs_inode.c
>  fs/xfs/Makefile            |  1 +
>  fs/xfs/libxfs/xfs_attr.c   | 80 ++++++++++++++++++++++++++++++++++---
>  fs/xfs/libxfs/xfs_bmap.c   | 51 ++++++++++++++----------
>  fs/xfs/libxfs/xfs_bmap.h   |  1 +
>  fs/xfs/libxfs/xfs_parent.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_attr.h          | 13 +++++-
>  fs/xfs/xfs_inode.c         | 16 +++++++-
>  7 files changed, 232 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> index a2a5d04..90b51ae 100644
> --- a/fs/xfs/Makefile
> +++ b/fs/xfs/Makefile
> @@ -52,6 +52,7 @@ xfs-y				+= $(addprefix libxfs/, \
>  				   xfs_inode_fork.o \
>  				   xfs_inode_buf.o \
>  				   xfs_log_rlimit.o \
> +				   xfs_parent.o \
>  				   xfs_ag_resv.o \
>  				   xfs_rmap.o \
>  				   xfs_rmap_btree.o \
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 646d41b..ed13c67 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -90,12 +90,14 @@ xfs_attr_args_init(
>  	args->whichfork = XFS_ATTR_FORK;
>  	args->dp = dp;
>  	args->flags = flags;
> -	args->name = name;
> -	args->namelen = strlen((const char *)name);
> -	if (args->namelen >= MAXNAMELEN)
> -		return -EFAULT;		/* match IRIX behaviour */
> +	if (name) {
> +		args->name = name;
> +		args->namelen = strlen((const char *)name);
> +		if (args->namelen >= MAXNAMELEN)
> +			return -EFAULT;		/* match IRIX behaviour */
>  
> -	args->hashval = xfs_da_hashname(args->name, args->namelen);
> +		args->hashval = xfs_da_hashname(args->name, args->namelen);
> +	}
>  	return 0;
>  }
>  
> @@ -203,6 +205,74 @@ xfs_attr_calc_size(
>  	return nblks;
>  }
>  
> +/*
> + * Add the initial parent pointer attribute.
> + *
> + * Inode must be locked and completely empty as we are adding the attribute
> + * fork to the inode. This open codes bits of xfs_bmap_add_attrfork() and
> + * xfs_attr_set() because we know the inode is completely empty at this point
> + * and so don't need to handle all the different combinations of fork
> + * configurations here.
> + */
> +int
> +xfs_attr_set_first_parent(
> +	struct xfs_trans	*tp,
> +	struct xfs_inode	*ip,
> +	struct xfs_parent_name_rec *rec,
> +	int			reclen,
> +	const char		*value,
> +	int			valuelen,
> +	struct xfs_defer_ops	*dfops,
> +	xfs_fsblock_t		*firstblock)
> +{
> +	struct xfs_da_args	args;
> +	int			flags = ATTR_PARENT;
> +	int			local;
> +	int			sf_size;
> +	int			error;
> +
> +	tp->t_flags |= XFS_TRANS_RESERVE;
> +
> +	error = xfs_attr_args_init(&args, ip, (char *)rec, flags);
> +	if (error)
> +		return error;
> +
> +	args.name = (char *)rec;
> +	args.namelen = reclen;
> +	args.hashval = xfs_da_hashname(args.name, args.namelen);
> +	args.value = (char *)value;
> +	args.valuelen = valuelen;
> +	args.firstblock = firstblock;
> +	args.dfops = dfops;
> +	args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
> +	args.total = xfs_attr_calc_size(&args, &local);
> +	args.trans = tp;
> +	ASSERT(local);
> +
> +	/* set the attribute fork appropriately */
> +	sf_size = sizeof(struct xfs_attr_sf_hdr) +
> +			XFS_ATTR_SF_ENTSIZE_BYNAME(reclen, valuelen);
> +	xfs_bmap_set_attrforkoff(ip, sf_size, NULL);
> +	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, KM_SLEEP);
> +	ip->i_afp->if_flags = XFS_IFEXTENTS;
> +
> +
> +	/* Try to add the attr to the attribute list in the inode. */
> +	xfs_attr_shortform_create(&args);
> +	error = xfs_attr_shortform_addname(&args);
> +	if (error != -ENOSPC)
> +		return error;

I wonder, what happens if we exceed the maximum number of attr fork
extents by linking a file too many times?  Would be nice if we could
bail out early (instead of shutting down the fs) if we were reasonably
sure it was going to blow up anyway.

Though, I'm wondering how we handle that in general since I don't see
any obvious checking...

Also seems a little fishy that we keep going even if ENOSPC, even though
the comment below says we're allowed to dip into the reserved space to
avoid this condition?

--D

> +
> +	/* It won't fit in the shortform, transform to a leaf block. */
> +	xfs_defer_init(args.dfops, args.firstblock);
> +	error = xfs_attr_shortform_to_leaf(&args);
> +	if (error)
> +		return error;
> +
> +	ASSERT(xfs_bmap_one_block(ip, XFS_ATTR_FORK));
> +	return xfs_attr_leaf_addname(&args);
> +}
> +
>  int
>  xfs_attr_set(
>  	struct xfs_inode	*dp,
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index a2d6466..91c403e 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1076,6 +1076,35 @@ xfs_bmap_add_attrfork_local(
>  	return -EFSCORRUPTED;
>  }
>  
> +int
> +xfs_bmap_set_attrforkoff(
> +	struct xfs_inode	*ip,
> +	int			size,
> +	int			*version)
> +{
> +	switch (ip->i_d.di_format) {
> +	case XFS_DINODE_FMT_DEV:
> +		ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
> +		break;
> +	case XFS_DINODE_FMT_UUID:
> +		ip->i_d.di_forkoff = roundup(sizeof(uuid_t), 8) >> 3;
> +		break;
> +	case XFS_DINODE_FMT_LOCAL:
> +	case XFS_DINODE_FMT_EXTENTS:
> +	case XFS_DINODE_FMT_BTREE:
> +		ip->i_d.di_forkoff = xfs_attr_shortform_bytesfit(ip, size);
> +		if (!ip->i_d.di_forkoff)
> +			ip->i_d.di_forkoff = xfs_default_attroffset(ip) >> 3;
> +		else if ((ip->i_mount->m_flags & XFS_MOUNT_ATTR2) && version)
> +			*version = 2;
> +		break;
> +	default:
> +		ASSERT(0);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
>  /*
>   * Convert inode from non-attributed to attributed.
>   * Must not be in a transaction, ip must not be locked.
> @@ -1130,27 +1159,7 @@ xfs_bmap_add_attrfork(
>  	xfs_trans_ijoin(tp, ip, 0);
>  	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
>  
> -	switch (ip->i_d.di_format) {
> -	case XFS_DINODE_FMT_DEV:
> -		ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
> -		break;
> -	case XFS_DINODE_FMT_UUID:
> -		ip->i_d.di_forkoff = roundup(sizeof(uuid_t), 8) >> 3;
> -		break;
> -	case XFS_DINODE_FMT_LOCAL:
> -	case XFS_DINODE_FMT_EXTENTS:
> -	case XFS_DINODE_FMT_BTREE:
> -		ip->i_d.di_forkoff = xfs_attr_shortform_bytesfit(ip, size);
> -		if (!ip->i_d.di_forkoff)
> -			ip->i_d.di_forkoff = xfs_default_attroffset(ip) >> 3;
> -		else if (mp->m_flags & XFS_MOUNT_ATTR2)
> -			version = 2;
> -		break;
> -	default:
> -		ASSERT(0);
> -		error = -EINVAL;
> -		goto trans_cancel;
> -	}
> +	xfs_bmap_set_attrforkoff(ip, size, &version);
>  
>  	ASSERT(ip->i_afp == NULL);
>  	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, KM_SLEEP);
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 851982a..533f40f 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -209,6 +209,7 @@ void	xfs_bmap_trace_exlist(struct xfs_inode *ip, xfs_extnum_t cnt,
>  void	xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
>  		xfs_filblks_t len);
>  int	xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
> +int	xfs_bmap_set_attrforkoff(struct xfs_inode *ip, int size, int *version);
>  void	xfs_bmap_local_to_extents_empty(struct xfs_inode *ip, int whichfork);
>  void	xfs_bmap_add_free(struct xfs_mount *mp, struct xfs_defer_ops *dfops,
>  			  xfs_fsblock_t bno, xfs_filblks_t len,
> diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c
> new file mode 100644
> index 0000000..88f7edc
> --- /dev/null
> +++ b/fs/xfs/libxfs/xfs_parent.c
> @@ -0,0 +1,98 @@
> +/*
> + * Copyright (c) 2015 Red Hat, Inc.
> + * All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write the Free Software Foundation
> + */
> +#include "xfs.h"
> +#include "xfs_fs.h"
> +#include "xfs_format.h"
> +#include "xfs_log_format.h"
> +#include "xfs_shared.h"
> +#include "xfs_trans_resv.h"
> +#include "xfs_mount.h"
> +#include "xfs_bmap_btree.h"
> +#include "xfs_inode.h"
> +#include "xfs_error.h"
> +#include "xfs_trace.h"
> +#include "xfs_trans.h"
> +#include "xfs_attr.h"
> +
> +/*
> + * Parent pointer attribute handling.
> + *
> + * Because the attribute value is a filename component, it will never be longer
> + * than 255 bytes. This means the attribute will always be a local format
> + * attribute as it is xfs_attr_leaf_entsize_local_max() for v5 filesystems will
> + * always be larger than this (max is 75% of block size).
> + *
> + * Creating a new parent attribute will always create a new attribute - there
> + * should never, ever be an existing attribute in the tree for a new inode.
> + * ENOSPC behaviour is problematic - creating the inode without the parent
> + * pointer is effectively a corruption, so we allow parent attribute creation
> + * to dip into the reserve block pool to avoid unexpected ENOSPC errors from
> + * occurring.
> + */
> +
> +/*
> + * Create the initial parent attribute.
> + *
> + * The initial attribute creation also needs to be atomic w.r.t the parent
> + * directory modification. Hence it needs to run in the same transaction and the
> + * transaction committed by the caller.  Because the attribute created is
> + * guaranteed to be a local attribute and is always going to be the first
> + * attribute in the attribute fork, we can do this safely in the single
> + * transaction context as it is impossible for an overwrite to occur and hence
> + * we'll never have a rolling overwrite transaction occurring here. Hence we
> + * can short-cut a lot of the normal xfs_attr_set() code paths that are needed
> + * to handle the generic cases.
> + */
> +static int
> +xfs_parent_create_nrec(
> +	struct xfs_trans	*tp,
> +	struct xfs_inode	*child,
> +	struct xfs_parent_name_irec *nrec,
> +	struct xfs_defer_ops	*dfops,
> +	xfs_fsblock_t		*firstblock)
> +{
> +	struct xfs_parent_name_rec rec;
> +
> +	rec.p_ino = cpu_to_be64(nrec->p_ino);
> +	rec.p_gen = cpu_to_be32(nrec->p_gen);
> +	rec.p_diroffset = cpu_to_be32(nrec->p_diroffset);
> +
> +	return xfs_attr_set_first_parent(tp, child, &rec, sizeof(rec),
> +				   nrec->p_name, nrec->p_namelen,
> +				   dfops, firstblock);
> +}
> +
> +int
> +xfs_parent_create(
> +	struct xfs_trans	*tp,
> +	struct xfs_inode	*parent,
> +	struct xfs_inode	*child,
> +	struct xfs_name		*child_name,
> +	xfs_dir2_dataptr_t	diroffset,
> +	struct xfs_defer_ops	*dfops,
> +	xfs_fsblock_t		*firstblock)
> +{
> +	struct xfs_parent_name_irec nrec;
> +
> +	nrec.p_ino = parent->i_ino;
> +	nrec.p_gen = VFS_I(parent)->i_generation;
> +	nrec.p_diroffset = diroffset;
> +	nrec.p_name = child_name->name;
> +	nrec.p_namelen = child_name->len;
> +
> +	return xfs_parent_create_nrec(tp, child, &nrec, dfops, firstblock);
> +}
> diff --git a/fs/xfs/xfs_attr.h b/fs/xfs/xfs_attr.h
> index 3031a09..fad2fbd 100644
> --- a/fs/xfs/xfs_attr.h
> +++ b/fs/xfs/xfs_attr.h
> @@ -155,5 +155,16 @@ int xfs_attr_remove(struct xfs_inode *dp, const unsigned char *name, int flags);
>  int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
>  		  int flags, struct attrlist_cursor_kern *cursor);
>  
> -
> +/*
> + * Parent pointer attribute prototypes
> + */
> +int xfs_parent_create(struct xfs_trans *tp, struct xfs_inode *parent,
> +		      struct xfs_inode *child, struct xfs_name *child_name,
> +		      xfs_dir2_dataptr_t diroffset, struct xfs_defer_ops *dfops,
> +		      xfs_fsblock_t *firstblock);
> +int xfs_attr_set_first_parent(struct xfs_trans *tp, struct xfs_inode *ip,
> +			      struct xfs_parent_name_rec *rec, int reclen,
> +			      const char *value, int valuelen,
> +			      struct xfs_defer_ops *dfops,
> +			      xfs_fsblock_t *firstblock);
>  #endif	/* __XFS_ATTR_H__ */
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 52c331e..3557791 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1162,6 +1162,7 @@ xfs_create(
>  	struct xfs_dquot	*pdqp = NULL;
>  	struct xfs_trans_res	*tres;
>  	uint			resblks;
> +	xfs_dir2_dataptr_t	diroffset;
>  
>  	trace_xfs_create(dp, name);
>  
> @@ -1251,7 +1252,7 @@ xfs_create(
>  	error = xfs_dir_createname(tp, dp, name, ip->i_ino,
>  					&first_block, &dfops, resblks ?
>  					resblks - XFS_IALLOC_SPACE_RES(mp) : 0,
> -					NULL);
> +					&diroffset);
>  	if (error) {
>  		ASSERT(error != -ENOSPC);
>  		goto out_trans_cancel;
> @@ -1270,6 +1271,19 @@ xfs_create(
>  	}
>  
>  	/*
> +	 * If we have parent pointers, we need to add the attribute containing
> +	 * the parent information now. This must be done within the same
> +	 * transaction the directory entry is created, while the new inode
> +	 * contains nothing in the inode literal area.
> +	 */
> +	if (xfs_sb_version_hasparent(&mp->m_sb)) {
> +		error = xfs_parent_create(tp, dp, ip, name, diroffset,
> +					  &dfops, &first_block);
> +		if (error)
> +			goto out_bmap_cancel;
> +	}
> +
> +	/*
>  	 * If this is a synchronous mount, make sure that the
>  	 * create transaction goes to disk before returning to
>  	 * the user.
> -- 
> 2.7.4
> 
> --
> 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: [RFC PATCH 13/13] Add the parent pointer support to the superblock version 5.

[Darrick J. Wong]

On Wed, Aug 09, 2017 at 06:41:33PM -0700, Allison Henderson wrote:
> [dchinner: forward ported and cleaned up]
> [achender: rebased and added parent pointer attribute to
>            compatible attributes mask]
> 
> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
> ---
> :100644 100644 6cedd75... b00bf1d... M	fs/xfs/libxfs/xfs_format.h
> :100644 100644 fba0105... a18dd0c... M	fs/xfs/libxfs/xfs_fs.h
> :100644 100644 50fb3a2... d7b9010... M	fs/xfs/xfs_fsops.c
>  fs/xfs/libxfs/xfs_format.h | 13 ++++++++-----
>  fs/xfs/libxfs/xfs_fs.h     |  1 +
>  fs/xfs/xfs_fsops.c         |  4 +++-
>  3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 6cedd75..b00bf1d 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -464,10 +464,12 @@ xfs_sb_has_compat_feature(
>  #define XFS_SB_FEAT_RO_COMPAT_FINOBT   (1 << 0)		/* free inode btree */
>  #define XFS_SB_FEAT_RO_COMPAT_RMAPBT   (1 << 1)		/* reverse map btree */
>  #define XFS_SB_FEAT_RO_COMPAT_REFLINK  (1 << 2)		/* reflinked files */
> +#define XFS_SB_FEAT_RO_COMPAT_PARENT	(1 << 3)	/* parent inode ptr */
>  #define XFS_SB_FEAT_RO_COMPAT_ALL \
>  		(XFS_SB_FEAT_RO_COMPAT_FINOBT | \
>  		 XFS_SB_FEAT_RO_COMPAT_RMAPBT | \
> -		 XFS_SB_FEAT_RO_COMPAT_REFLINK)
> +		 XFS_SB_FEAT_RO_COMPAT_REFLINK| \
> +		 XFS_SB_FEAT_RO_COMPAT_PARENT)
>  #define XFS_SB_FEAT_RO_COMPAT_UNKNOWN	~XFS_SB_FEAT_RO_COMPAT_ALL
>  static inline bool
>  xfs_sb_has_ro_compat_feature(
> @@ -507,17 +509,17 @@ xfs_sb_has_incompat_log_feature(
>  /*
>   * V5 superblock specific feature checks
>   */
> -static inline int xfs_sb_version_hascrc(struct xfs_sb *sbp)
> +static inline bool xfs_sb_version_hascrc(struct xfs_sb *sbp)
>  {
>  	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
>  }
>  
> -static inline int xfs_sb_version_has_pquotino(struct xfs_sb *sbp)
> +static inline bool xfs_sb_version_has_pquotino(struct xfs_sb *sbp)

Not part of parent pointers; if you want to fix this, do it in a separate
patch, please.

--D

>  {
>  	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
>  }
>  
> -static inline int xfs_sb_version_hasftype(struct xfs_sb *sbp)
> +static inline bool xfs_sb_version_hasftype(struct xfs_sb *sbp)
>  {
>  	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
>  		xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_FTYPE)) ||
> @@ -563,7 +565,8 @@ static inline bool xfs_sb_version_hasreflink(struct xfs_sb *sbp)
>  
>  static inline bool xfs_sb_version_hasparent(struct xfs_sb *sbp)
>  {
> -	return false; /* We'll enable this at the end of the set */
> +	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
> +		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_PARENT));
>  }
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index fba0105..a18dd0c 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -222,6 +222,7 @@ typedef struct xfs_fsop_resblks {
>  #define XFS_FSOP_GEOM_FLAGS_SPINODES	0x40000	/* sparse inode chunks	*/
>  #define XFS_FSOP_GEOM_FLAGS_RMAPBT	0x80000	/* reverse mapping btree */
>  #define XFS_FSOP_GEOM_FLAGS_REFLINK	0x100000 /* files can share blocks */
> +#define XFS_FSOP_GEOM_FLAGS_PARENT	0x200000 /* parent pointers */
>  
>  /*
>   * Minimum and maximum sizes need for growth checks.
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 50fb3a2..d7b9010 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -112,7 +112,9 @@ xfs_fs_geometry(
>  			(xfs_sb_version_hasrmapbt(&mp->m_sb) ?
>  				XFS_FSOP_GEOM_FLAGS_RMAPBT : 0) |
>  			(xfs_sb_version_hasreflink(&mp->m_sb) ?
> -				XFS_FSOP_GEOM_FLAGS_REFLINK : 0);
> +				XFS_FSOP_GEOM_FLAGS_REFLINK : 0) |
> +			(xfs_sb_version_hasparent(&mp->m_sb) ?
> +				XFS_FSOP_GEOM_FLAGS_PARENT : 0);
>  		geo->logsectsize = xfs_sb_version_hassector(&mp->m_sb) ?
>  				mp->m_sb.sb_logsectsize : BBSIZE;
>  		geo->rtsectsize = mp->m_sb.sb_blocksize;
> -- 
> 2.7.4
> 
> --
> 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: [RFC PATCH 02/13] xfs: get directory offset when removing directory name

[Dave Chinner]

On Mon, Aug 14, 2017 at 04:56:53PM -0700, Darrick J. Wong wrote:
> The biggest complaint I have about this patch series is that we end up
> using separate transactions for each phase of the link/unlink
> operations.  There's no guarantee that once we start the process of
> expand-dir, add-name-to-dir, log-inode-cores, expand-attr,
> add-pptr-to-attrs that we actually get all the way to the end of that
> process, particularly if we crash in the middle and have to recover the
> log coming back up.

Yes, that's a problem that we had not been solved at the point in
time I wrote that patchset. After making the parent pointer
attributes be added and removed correctly, then next thing to do was
to make them atomic and replayable. And that's the bit I never got
to.

> Obviously, back in 2014 when Dave started on these patches there was no
> defer_ops, so a lot of effort goes into passing parameters up and down
> the stack and fiddling with the transaction reservation type
> declarations in order to guarantee that we can shove everything into a
> single transaction.  Now that we have a more general mechanism to split
> complex updates into a chain of smaller updates (and make log recovery
> pick up where we left off) it's time to figure out how to make attr (and
> maybe dir) updates a deferrable operation.

Right. We've got the infrastructure now, but it's nasty to retrofit
to all the attr code.

> This adds complexity to the log because we have to define an "attr add
> KEY:VALUE" and a "attr delete KEY" deferred op that hooks into the
> existing attr code.  For parent pointers it's not a big deal to log
> values since they're not big, but for the general case this idea needs
> more thought, or a decision that we'll just keep doing things the same
> way.

The problem with this is that it requires us to log the entire
attribute value, which could be 64k in size. That used to be a big
issue because log bandwidth was at a premium. That isn't so much a
problem now, so logging the attr value and getting rid of the
unlogged sync write for remote attr updates is the way I think we
should head.

> Anyway, assuming the existence of a "attr delete KEY" log intent item we
> no longer have to pass the offsets around everywhere because instead
> _dir_removename attaches the defer item to the defer_ops and
> _defer_finish takes care of rolling through the updates.
> 
> Thoughts?

Yup, it will simplify some things.

However, if we're going to start converting complex operations into
smaller, simpler ops chained via intents and deferred ops, lets
let's take a step back here and think about how we'd like them to be
defined, maintained and executed. If we're going to take a big step
to convert the dir/attr code to run from deferred ops, I want to
make sure we can make the most of it in future.

FYI, what I'd really like to do is to be able to statically define
the individual operations needed to make a modification as a set of
deferred ops sorta like the way we build up transaction
reservations. I'm thinking of defer ops being somewhat akin to a
state machine in definition and implementation. i.e. rather than
being opaque things that are built up by dynamic events, they become
mostly static ops with some dynamically driven events.

Hence a directory addition is simply a case of selecting the correct
defer_ops structure and starting it to run on the directory inode
and new dirent name. The high level code would need to check what
directory features are enabled to select the ops that need to run,
but otherwise there's nothing more to do but hand it to the
xfs_deferops_start(). Parent pointers then just becomes a different
set of deferops. If possible, it'd be nice to use the definition of
each op to determine the log/block reservations needed for the op to
run, too.

Expanding this filesystem wide, we'd end up with an engine that runs
defer ops in the most efficient manner possible and a bunch of
smaller, simpler operations linked by intent/done log entries. These
ops could be run sync or async by the engine, which would give us a
path towards async metadata ops and bulk metadata update
optimisations...

Yeah, I'm getting way ahead of myself here, but I think you get the
idea. Does that even remotely sound plausible, or have I just been
hitting the hard stuff too much recently?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [RFC PATCH 08/13] xfs: parent pointer attribute creation

[Dave Chinner]

On Mon, Aug 14, 2017 at 05:06:06PM -0700, Darrick J. Wong wrote:
> On Wed, Aug 09, 2017 at 06:41:28PM -0700, Allison Henderson wrote:
> > +	/* Try to add the attr to the attribute list in the inode. */
> > +	xfs_attr_shortform_create(&args);
> > +	error = xfs_attr_shortform_addname(&args);
> > +	if (error != -ENOSPC)
> > +		return error;
> 
> I wonder, what happens if we exceed the maximum number of attr fork
> extents by linking a file too many times?

We'll hit free space indexing scalability problems with the dir1
format used by the attr tree before we get there. And to solve that
we simply convert the attr code to use the dir3 format tree....

> Would be nice if we could
> bail out early (instead of shutting down the fs) if we were reasonably
> sure it was going to blow up anyway.
> 
> Though, I'm wondering how we handle that in general since I don't see
> any obvious checking...
> 
> Also seems a little fishy that we keep going even if ENOSPC, even though
> the comment below says we're allowed to dip into the reserved space to
> avoid this condition?

When I wrote this, we had a small, fixed reserve pool that could be
depleted by excessive operation at ENOSPC conditions, and hence
ENOSPC could still occur (however unlikely it might be).

Once again, the reserve pool is very different now, so the
assumptions this code makes about ENOSPC behaviour might not be
valid anymore...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [RFC PATCH 02/13] xfs: get directory offset when removing directory name

[Allison Henderson]

On 08/14/2017 04:56 PM, Darrick J. Wong wrote:

> On Wed, Aug 09, 2017 at 06:41:22PM -0700, Allison Henderson wrote:
>> From: Mark Tinguely <tinguely@sgi.com>
>>
>> Return the directory offset information when removing an entry to the
>> directory.
>>
>> This offset will be used as the parent pointer offset in xfs_remove.
>>
>> [dchinner: forward ported and cleaned up]
>> [achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t]
>>
>> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
>> Signed-off-by: Dave Chinner <dchinner@redhat.com>
>> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
>> ---
>> :100644 100644 a1ca460... a34956e... M	fs/xfs/libxfs/xfs_dir2.c
>> :100644 100644 e349900... ee98b2a... M	fs/xfs/libxfs/xfs_dir2.h
>> :100644 100644 79684d5... 4dbe2fc... M	fs/xfs/libxfs/xfs_dir2_block.c
>> :100644 100644 2ac7a7e... 197e627... M	fs/xfs/libxfs/xfs_dir2_leaf.c
>> :100644 100644 8bc91f8... 13d5244... M	fs/xfs/libxfs/xfs_dir2_node.c
>> :100644 100644 489bdef... 9e90c22... M	fs/xfs/libxfs/xfs_dir2_sf.c
>> :100644 100644 cea1c56... f58f62a... M	fs/xfs/xfs_inode.c
>>   fs/xfs/libxfs/xfs_dir2.c       | 15 +++++++++------
>>   fs/xfs/libxfs/xfs_dir2.h       |  3 ++-
>>   fs/xfs/libxfs/xfs_dir2_block.c |  4 ++--
>>   fs/xfs/libxfs/xfs_dir2_leaf.c  |  5 +++--
>>   fs/xfs/libxfs/xfs_dir2_node.c  |  5 +++--
>>   fs/xfs/libxfs/xfs_dir2_sf.c    |  2 ++
>>   fs/xfs/xfs_inode.c             |  7 ++++---
>>   7 files changed, 25 insertions(+), 16 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
>> index a1ca460..a34956e 100644
>> --- a/fs/xfs/libxfs/xfs_dir2.c
>> +++ b/fs/xfs/libxfs/xfs_dir2.c
>> @@ -443,13 +443,14 @@ xfs_dir_lookup(
>>    */
>>   int
>>   xfs_dir_removename(
>> -	xfs_trans_t	*tp,
>> -	xfs_inode_t	*dp,
>> -	struct xfs_name	*name,
>> -	xfs_ino_t	ino,
>> -	xfs_fsblock_t	*first,		/* bmap's firstblock */
>> +	xfs_trans_t		*tp,
> If you change a line containing a struct tyepdef type, please convert it
> back to the raw struct type.
>
> 	struct xfs_trans	*tp,
>
> etc.  Some day we'll have managed to remove them all! :D
Ok, will do
>
>> +	xfs_inode_t		*dp,
>> +	struct xfs_name		*name,
>> +	xfs_ino_t		ino,
>> +	xfs_fsblock_t		*first,		/* bmap's firstblock */
>>   	struct xfs_defer_ops	*dfops,		/* bmap's freeblock list */
>> -	xfs_extlen_t	total)		/* bmap's total block count */
>> +	xfs_extlen_t		total,		/* bmap's total block count */
>> +	xfs_dir2_dataptr_t	*offset)	/* OUT: offset in directory */
> Hmm.  We /also/ pass a defer_ops into _dir_removename.  I wasn't going
> to say this until later in the patchset, but it's apropos now.
>
> The biggest complaint I have about this patch series is that we end up
> using separate transactions for each phase of the link/unlink
> operations.  There's no guarantee that once we start the process of
> expand-dir, add-name-to-dir, log-inode-cores, expand-attr,
> add-pptr-to-attrs that we actually get all the way to the end of that
> process, particularly if we crash in the middle and have to recover the
> log coming back up.
>
> Obviously, back in 2014 when Dave started on these patches there was no
> defer_ops, so a lot of effort goes into passing parameters up and down
> the stack and fiddling with the transaction reservation type
> declarations in order to guarantee that we can shove everything into a
> single transaction.  Now that we have a more general mechanism to split
> complex updates into a chain of smaller updates (and make log recovery
> pick up where we left off) it's time to figure out how to make attr (and
> maybe dir) updates a deferrable operation.
>
> This adds complexity to the log because we have to define an "attr add
> KEY:VALUE" and a "attr delete KEY" deferred op that hooks into the
> existing attr code.  For parent pointers it's not a big deal to log
> values since they're not big, but for the general case this idea needs
> more thought, or a decision that we'll just keep doing things the same
> way.
>
> Anyway, assuming the existence of a "attr delete KEY" log intent item we
> no longer have to pass the offsets around everywhere because instead
> _dir_removename attaches the defer item to the defer_ops and
> _defer_finish takes care of rolling through the updates.
>
> Thoughts?
>
> --D
It sounds like what you're describing would make sense in the greater 
scheme of things. If folks agree, I could dig in the log code and see 
how much surgery it would need.  I'm generally on board with the idea of 
restructuring things sooner rather than later when there's more code to 
patch up.

Allison
>>   {
>>   	struct xfs_da_args *args;
>>   	int		rval;
>> @@ -495,6 +496,8 @@ xfs_dir_removename(
>>   		rval = xfs_dir2_leaf_removename(args);
>>   	else
>>   		rval = xfs_dir2_node_removename(args);
>> +	if (offset)
>> +		*offset = args->offset;
>>   out_free:
>>   	kmem_free(args);
>>   	return rval;
>> diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h
>> index e349900..ee98b2a 100644
>> --- a/fs/xfs/libxfs/xfs_dir2.h
>> +++ b/fs/xfs/libxfs/xfs_dir2.h
>> @@ -139,7 +139,8 @@ extern int xfs_dir_lookup(struct xfs_trans *tp, struct xfs_inode *dp,
>>   extern int xfs_dir_removename(struct xfs_trans *tp, struct xfs_inode *dp,
>>   				struct xfs_name *name, xfs_ino_t ino,
>>   				xfs_fsblock_t *first,
>> -				struct xfs_defer_ops *dfops, xfs_extlen_t tot);
>> +				struct xfs_defer_ops *dfops, xfs_extlen_t tot,
>> +				xfs_dir2_dataptr_t *offset);
>>   extern int xfs_dir_replace(struct xfs_trans *tp, struct xfs_inode *dp,
>>   				struct xfs_name *name, xfs_ino_t inum,
>>   				xfs_fsblock_t *first,
>> diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
>> index 79684d5..4dbe2fc 100644
>> --- a/fs/xfs/libxfs/xfs_dir2_block.c
>> +++ b/fs/xfs/libxfs/xfs_dir2_block.c
>> @@ -791,9 +791,9 @@ xfs_dir2_block_removename(
>>   	/*
>>   	 * Point to the data entry using the leaf entry.
>>   	 */
>> +	args->offset = be32_to_cpu(blp[ent].address);
>>   	dep = (xfs_dir2_data_entry_t *)((char *)hdr +
>> -			xfs_dir2_dataptr_to_off(args->geo,
>> -						be32_to_cpu(blp[ent].address)));
>> +			xfs_dir2_dataptr_to_off(args->geo, args->offset));
>>   	/*
>>   	 * Mark the data entry's space free.
>>   	 */
>> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
>> index 2ac7a7e..197e627 100644
>> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
>> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
>> @@ -1383,9 +1383,10 @@ xfs_dir2_leaf_removename(
>>   	 * Point to the leaf entry, use that to point to the data entry.
>>   	 */
>>   	lep = &ents[index];
>> -	db = xfs_dir2_dataptr_to_db(args->geo, be32_to_cpu(lep->address));
>> +	args->offset = be32_to_cpu(lep->address);
>> +	db = xfs_dir2_dataptr_to_db(args->geo, args->offset);
>>   	dep = (xfs_dir2_data_entry_t *)((char *)hdr +
>> -		xfs_dir2_dataptr_to_off(args->geo, be32_to_cpu(lep->address)));
>> +		xfs_dir2_dataptr_to_off(args->geo, args->offset));
>>   	needscan = needlog = 0;
>>   	oldbest = be16_to_cpu(bf[0].length);
>>   	ltp = xfs_dir2_leaf_tail_p(args->geo, leaf);
>> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
>> index 8bc91f8..13d5244 100644
>> --- a/fs/xfs/libxfs/xfs_dir2_node.c
>> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
>> @@ -1238,9 +1238,10 @@ xfs_dir2_leafn_remove(
>>   	/*
>>   	 * Extract the data block and offset from the entry.
>>   	 */
>> -	db = xfs_dir2_dataptr_to_db(args->geo, be32_to_cpu(lep->address));
>> +	args->offset = be32_to_cpu(lep->address);
>> +	db = xfs_dir2_dataptr_to_db(args->geo, args->offset);
>>   	ASSERT(dblk->blkno == db);
>> -	off = xfs_dir2_dataptr_to_off(args->geo, be32_to_cpu(lep->address));
>> +	off = xfs_dir2_dataptr_to_off(args->geo, args->offset);
>>   	ASSERT(dblk->index == off);
>>   
>>   	/*
>> diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
>> index 489bdef..9e90c22 100644
>> --- a/fs/xfs/libxfs/xfs_dir2_sf.c
>> +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
>> @@ -919,6 +919,8 @@ xfs_dir2_sf_removename(
>>   								XFS_CMP_EXACT) {
>>   			ASSERT(dp->d_ops->sf_get_ino(sfp, sfep) ==
>>   			       args->inumber);
>> +			args->offset = xfs_dir2_byte_to_dataptr(
>> +						xfs_dir2_sf_get_offset(sfep));
>>   			break;
>>   		}
>>   	}
>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
>> index cea1c56..f58f62a 100644
>> --- a/fs/xfs/xfs_inode.c
>> +++ b/fs/xfs/xfs_inode.c
>> @@ -2621,8 +2621,8 @@ xfs_remove(
>>   		goto out_trans_cancel;
>>   
>>   	xfs_defer_init(&dfops, &first_block);
>> -	error = xfs_dir_removename(tp, dp, name, ip->i_ino,
>> -					&first_block, &dfops, resblks);
>> +	error = xfs_dir_removename(tp, dp, name, ip->i_ino, &first_block,
>> +				   &dfops, resblks, NULL);
>>   	if (error) {
>>   		ASSERT(error != -ENOENT);
>>   		goto out_bmap_cancel;
>> @@ -3132,7 +3132,8 @@ xfs_rename(
>>   					&first_block, &dfops, spaceres);
>>   	} else
>>   		error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino,
>> -					   &first_block, &dfops, spaceres);
>> +					   &first_block, &dfops, spaceres,
>> +					   NULL);
>>   	if (error)
>>   		goto out_bmap_cancel;
>>   
>> -- 
>> 2.7.4
>>
>> --
>> 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: [RFC PATCH 05/13] xfs: define parent pointer xattr format

[Allison Henderson]

On 08/14/2017 04:59 PM, Darrick J. Wong wrote:

> On Wed, Aug 09, 2017 at 06:41:25PM -0700, Allison Henderson wrote:
>> From: Dave Chinner <dchinner@redhat.com>
>>
>> We need to define the parent pointer attribute format before we
>> start adding support for it into all the code that needs to use it.
>> The EA format we will use encodes the following information:
>>
>> 	name={parent inode #, parent inode generation, dirent offset}
>> 	value={dirent filename}
>>
>> The inode/gen gives all the information we need to reliably identify
>> the parent without requiring child->parent lock ordering, and allows
>> userspace to do pathname component level reconstruction without the
>> kernel ever needing to verify the parent itself as part of ioctl
>> calls.
>>
>> By using the dirent offset in the EA name, we have a method of
>> knowing the exact parent pointer EA we need to modify/remove in
>> rename/unlink without an unbound EA name search.
>>
>> By keeping the dirent name in the value, we have enough information
>> to be able to validate and reconstruct damaged directory trees.
>> While the diroffset of a filename alone is not unique enough to
>> identify the child, the {diroffset,filename,child_inode} tuple is
>> sufficient. That is, if the diroffset gets reused and points to a
>> different filename, we can detect that from the contents of EA. If a
>> link of the same name is created, then we can check whether it
>> points at the same inode as the parent EA we current have.
>>
>> [achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t]
>>
>> Signed-off-by: Dave Chinner <dchinner@redhat.com>
>> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
>> ---
>> :100644 100644 d4d9bef... 91e2d5a... M	fs/xfs/libxfs/xfs_format.h
>>   fs/xfs/libxfs/xfs_format.h | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
>> index d4d9bef..91e2d5a 100644
>> --- a/fs/xfs/libxfs/xfs_format.h
>> +++ b/fs/xfs/libxfs/xfs_format.h
>> @@ -18,6 +18,8 @@
>>   #ifndef __XFS_FORMAT_H__
>>   #define __XFS_FORMAT_H__
>>   
>> +#include "xfs_da_format.h"
>> +
>>   /*
>>    * XFS On Disk Format Definitions
>>    *
>> @@ -1721,4 +1723,29 @@ struct xfs_acl {
>>   #define SGI_ACL_FILE_SIZE	(sizeof(SGI_ACL_FILE)-1)
>>   #define SGI_ACL_DEFAULT_SIZE	(sizeof(SGI_ACL_DEFAULT)-1)
>>   
>> +/*
>> + * Parent pointer attribute format definition
>> + *
>> + * EA name encodes the parent inode number, generation and the offset of
>> + * the dirent that points to the child inode. The EA value contains the
>> + * same name as the dirent in the parent directory.
>> + */
>> +struct xfs_parent_name_rec {
>> +	__be64	p_ino;
>> +	__be32	p_gen;
>> +	__be32	p_diroffset;
>> +};
>> +
>> +/*
>> + * incore version of the above, also contains name pointers so callers
>> + * can pass/obtain all the parent pointer information in a single structure
>> + */
>> +struct xfs_parent_name_irec {
>> +	uint64_t		p_ino;
> Should use incore xfs types, i.e. xfs_ino_t.
Ok, will fix
>> +	uint32_t		p_gen;
>> +	xfs_dir2_dataptr_t	p_diroffset;
>> +	const char		*p_name;
>> +	uint32_t		p_namelen;
> Pathname components are at most 255 bytes long, though I guess that
> doesn't save us any space here.
Ok, I'll update it to use a uint8_t, parameters get shifted around all 
the time.

Allison
> --D
>
>> +};
>> +
>>   #endif /* __XFS_FORMAT_H__ */
>> -- 
>> 2.7.4
>>
>> --
>> 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: [RFC PATCH 08/13] xfs: parent pointer attribute creation

[Allison Henderson]

On 08/14/2017 05:06 PM, Darrick J. Wong wrote:

> On Wed, Aug 09, 2017 at 06:41:28PM -0700, Allison Henderson wrote:
>> From: Dave Chinner <dchinner@redhat.com>
>>
>> [bfoster: rebase, use VFS inode generation]
>> [achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t,
>> 	   fixed some null pointer bugs]
>>
>> Signed-off-by: Dave Chinner <dchinner@redhat.com>
>> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
>> ---
>> :100644 100644 a2a5d04... 90b51ae... M	fs/xfs/Makefile
>> :100644 100644 646d41b... ed13c67... M	fs/xfs/libxfs/xfs_attr.c
>> :100644 100644 a2d6466... 91c403e... M	fs/xfs/libxfs/xfs_bmap.c
>> :100644 100644 851982a... 533f40f... M	fs/xfs/libxfs/xfs_bmap.h
>> :000000 100644 0000000... 88f7edc... A	fs/xfs/libxfs/xfs_parent.c
>> :100644 100644 3031a09... fad2fbd... M	fs/xfs/xfs_attr.h
>> :100644 100644 52c331e... 3557791... M	fs/xfs/xfs_inode.c
>>   fs/xfs/Makefile            |  1 +
>>   fs/xfs/libxfs/xfs_attr.c   | 80 ++++++++++++++++++++++++++++++++++---
>>   fs/xfs/libxfs/xfs_bmap.c   | 51 ++++++++++++++----------
>>   fs/xfs/libxfs/xfs_bmap.h   |  1 +
>>   fs/xfs/libxfs/xfs_parent.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++
>>   fs/xfs/xfs_attr.h          | 13 +++++-
>>   fs/xfs/xfs_inode.c         | 16 +++++++-
>>   7 files changed, 232 insertions(+), 28 deletions(-)
>>
>> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
>> index a2a5d04..90b51ae 100644
>> --- a/fs/xfs/Makefile
>> +++ b/fs/xfs/Makefile
>> @@ -52,6 +52,7 @@ xfs-y				+= $(addprefix libxfs/, \
>>   				   xfs_inode_fork.o \
>>   				   xfs_inode_buf.o \
>>   				   xfs_log_rlimit.o \
>> +				   xfs_parent.o \
>>   				   xfs_ag_resv.o \
>>   				   xfs_rmap.o \
>>   				   xfs_rmap_btree.o \
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index 646d41b..ed13c67 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -90,12 +90,14 @@ xfs_attr_args_init(
>>   	args->whichfork = XFS_ATTR_FORK;
>>   	args->dp = dp;
>>   	args->flags = flags;
>> -	args->name = name;
>> -	args->namelen = strlen((const char *)name);
>> -	if (args->namelen >= MAXNAMELEN)
>> -		return -EFAULT;		/* match IRIX behaviour */
>> +	if (name) {
>> +		args->name = name;
>> +		args->namelen = strlen((const char *)name);
>> +		if (args->namelen >= MAXNAMELEN)
>> +			return -EFAULT;		/* match IRIX behaviour */
>>   
>> -	args->hashval = xfs_da_hashname(args->name, args->namelen);
>> +		args->hashval = xfs_da_hashname(args->name, args->namelen);
>> +	}
>>   	return 0;
>>   }
>>   
>> @@ -203,6 +205,74 @@ xfs_attr_calc_size(
>>   	return nblks;
>>   }
>>   
>> +/*
>> + * Add the initial parent pointer attribute.
>> + *
>> + * Inode must be locked and completely empty as we are adding the attribute
>> + * fork to the inode. This open codes bits of xfs_bmap_add_attrfork() and
>> + * xfs_attr_set() because we know the inode is completely empty at this point
>> + * and so don't need to handle all the different combinations of fork
>> + * configurations here.
>> + */
>> +int
>> +xfs_attr_set_first_parent(
>> +	struct xfs_trans	*tp,
>> +	struct xfs_inode	*ip,
>> +	struct xfs_parent_name_rec *rec,
>> +	int			reclen,
>> +	const char		*value,
>> +	int			valuelen,
>> +	struct xfs_defer_ops	*dfops,
>> +	xfs_fsblock_t		*firstblock)
>> +{
>> +	struct xfs_da_args	args;
>> +	int			flags = ATTR_PARENT;
>> +	int			local;
>> +	int			sf_size;
>> +	int			error;
>> +
>> +	tp->t_flags |= XFS_TRANS_RESERVE;
>> +
>> +	error = xfs_attr_args_init(&args, ip, (char *)rec, flags);
>> +	if (error)
>> +		return error;
>> +
>> +	args.name = (char *)rec;
>> +	args.namelen = reclen;
>> +	args.hashval = xfs_da_hashname(args.name, args.namelen);
>> +	args.value = (char *)value;
>> +	args.valuelen = valuelen;
>> +	args.firstblock = firstblock;
>> +	args.dfops = dfops;
>> +	args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
>> +	args.total = xfs_attr_calc_size(&args, &local);
>> +	args.trans = tp;
>> +	ASSERT(local);
>> +
>> +	/* set the attribute fork appropriately */
>> +	sf_size = sizeof(struct xfs_attr_sf_hdr) +
>> +			XFS_ATTR_SF_ENTSIZE_BYNAME(reclen, valuelen);
>> +	xfs_bmap_set_attrforkoff(ip, sf_size, NULL);
>> +	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, KM_SLEEP);
>> +	ip->i_afp->if_flags = XFS_IFEXTENTS;
>> +
>> +
>> +	/* Try to add the attr to the attribute list in the inode. */
>> +	xfs_attr_shortform_create(&args);
>> +	error = xfs_attr_shortform_addname(&args);
>> +	if (error != -ENOSPC)
>> +		return error;
> I wonder, what happens if we exceed the maximum number of attr fork
> extents by linking a file too many times?  Would be nice if we could
> bail out early (instead of shutting down the fs) if we were reasonably
> sure it was going to blow up anyway.
>
> Though, I'm wondering how we handle that in general since I don't see
> any obvious checking...
>
> Also seems a little fishy that we keep going even if ENOSPC, even though
> the comment below says we're allowed to dip into the reserved space to
> avoid this condition?
>
> --D

Unless someone feels strongly one way or another, the early bail out 
sounds reasonable to me.  It seems like a rather rare condition, and the 
need to squeeze in just a few more links before getting an error seems 
somewhat nominal.

Allison

>
>> +
>> +	/* It won't fit in the shortform, transform to a leaf block. */
>> +	xfs_defer_init(args.dfops, args.firstblock);
>> +	error = xfs_attr_shortform_to_leaf(&args);
>> +	if (error)
>> +		return error;
>> +
>> +	ASSERT(xfs_bmap_one_block(ip, XFS_ATTR_FORK));
>> +	return xfs_attr_leaf_addname(&args);
>> +}
>> +
>>   int
>>   xfs_attr_set(
>>   	struct xfs_inode	*dp,
>> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
>> index a2d6466..91c403e 100644
>> --- a/fs/xfs/libxfs/xfs_bmap.c
>> +++ b/fs/xfs/libxfs/xfs_bmap.c
>> @@ -1076,6 +1076,35 @@ xfs_bmap_add_attrfork_local(
>>   	return -EFSCORRUPTED;
>>   }
>>   
>> +int
>> +xfs_bmap_set_attrforkoff(
>> +	struct xfs_inode	*ip,
>> +	int			size,
>> +	int			*version)
>> +{
>> +	switch (ip->i_d.di_format) {
>> +	case XFS_DINODE_FMT_DEV:
>> +		ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
>> +		break;
>> +	case XFS_DINODE_FMT_UUID:
>> +		ip->i_d.di_forkoff = roundup(sizeof(uuid_t), 8) >> 3;
>> +		break;
>> +	case XFS_DINODE_FMT_LOCAL:
>> +	case XFS_DINODE_FMT_EXTENTS:
>> +	case XFS_DINODE_FMT_BTREE:
>> +		ip->i_d.di_forkoff = xfs_attr_shortform_bytesfit(ip, size);
>> +		if (!ip->i_d.di_forkoff)
>> +			ip->i_d.di_forkoff = xfs_default_attroffset(ip) >> 3;
>> +		else if ((ip->i_mount->m_flags & XFS_MOUNT_ATTR2) && version)
>> +			*version = 2;
>> +		break;
>> +	default:
>> +		ASSERT(0);
>> +		return -EINVAL;
>> +	}
>> +	return 0;
>> +}
>> +
>>   /*
>>    * Convert inode from non-attributed to attributed.
>>    * Must not be in a transaction, ip must not be locked.
>> @@ -1130,27 +1159,7 @@ xfs_bmap_add_attrfork(
>>   	xfs_trans_ijoin(tp, ip, 0);
>>   	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
>>   
>> -	switch (ip->i_d.di_format) {
>> -	case XFS_DINODE_FMT_DEV:
>> -		ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
>> -		break;
>> -	case XFS_DINODE_FMT_UUID:
>> -		ip->i_d.di_forkoff = roundup(sizeof(uuid_t), 8) >> 3;
>> -		break;
>> -	case XFS_DINODE_FMT_LOCAL:
>> -	case XFS_DINODE_FMT_EXTENTS:
>> -	case XFS_DINODE_FMT_BTREE:
>> -		ip->i_d.di_forkoff = xfs_attr_shortform_bytesfit(ip, size);
>> -		if (!ip->i_d.di_forkoff)
>> -			ip->i_d.di_forkoff = xfs_default_attroffset(ip) >> 3;
>> -		else if (mp->m_flags & XFS_MOUNT_ATTR2)
>> -			version = 2;
>> -		break;
>> -	default:
>> -		ASSERT(0);
>> -		error = -EINVAL;
>> -		goto trans_cancel;
>> -	}
>> +	xfs_bmap_set_attrforkoff(ip, size, &version);
>>   
>>   	ASSERT(ip->i_afp == NULL);
>>   	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, KM_SLEEP);
>> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
>> index 851982a..533f40f 100644
>> --- a/fs/xfs/libxfs/xfs_bmap.h
>> +++ b/fs/xfs/libxfs/xfs_bmap.h
>> @@ -209,6 +209,7 @@ void	xfs_bmap_trace_exlist(struct xfs_inode *ip, xfs_extnum_t cnt,
>>   void	xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
>>   		xfs_filblks_t len);
>>   int	xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
>> +int	xfs_bmap_set_attrforkoff(struct xfs_inode *ip, int size, int *version);
>>   void	xfs_bmap_local_to_extents_empty(struct xfs_inode *ip, int whichfork);
>>   void	xfs_bmap_add_free(struct xfs_mount *mp, struct xfs_defer_ops *dfops,
>>   			  xfs_fsblock_t bno, xfs_filblks_t len,
>> diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c
>> new file mode 100644
>> index 0000000..88f7edc
>> --- /dev/null
>> +++ b/fs/xfs/libxfs/xfs_parent.c
>> @@ -0,0 +1,98 @@
>> +/*
>> + * Copyright (c) 2015 Red Hat, Inc.
>> + * All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it would be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write the Free Software Foundation
>> + */
>> +#include "xfs.h"
>> +#include "xfs_fs.h"
>> +#include "xfs_format.h"
>> +#include "xfs_log_format.h"
>> +#include "xfs_shared.h"
>> +#include "xfs_trans_resv.h"
>> +#include "xfs_mount.h"
>> +#include "xfs_bmap_btree.h"
>> +#include "xfs_inode.h"
>> +#include "xfs_error.h"
>> +#include "xfs_trace.h"
>> +#include "xfs_trans.h"
>> +#include "xfs_attr.h"
>> +
>> +/*
>> + * Parent pointer attribute handling.
>> + *
>> + * Because the attribute value is a filename component, it will never be longer
>> + * than 255 bytes. This means the attribute will always be a local format
>> + * attribute as it is xfs_attr_leaf_entsize_local_max() for v5 filesystems will
>> + * always be larger than this (max is 75% of block size).
>> + *
>> + * Creating a new parent attribute will always create a new attribute - there
>> + * should never, ever be an existing attribute in the tree for a new inode.
>> + * ENOSPC behaviour is problematic - creating the inode without the parent
>> + * pointer is effectively a corruption, so we allow parent attribute creation
>> + * to dip into the reserve block pool to avoid unexpected ENOSPC errors from
>> + * occurring.
>> + */
>> +
>> +/*
>> + * Create the initial parent attribute.
>> + *
>> + * The initial attribute creation also needs to be atomic w.r.t the parent
>> + * directory modification. Hence it needs to run in the same transaction and the
>> + * transaction committed by the caller.  Because the attribute created is
>> + * guaranteed to be a local attribute and is always going to be the first
>> + * attribute in the attribute fork, we can do this safely in the single
>> + * transaction context as it is impossible for an overwrite to occur and hence
>> + * we'll never have a rolling overwrite transaction occurring here. Hence we
>> + * can short-cut a lot of the normal xfs_attr_set() code paths that are needed
>> + * to handle the generic cases.
>> + */
>> +static int
>> +xfs_parent_create_nrec(
>> +	struct xfs_trans	*tp,
>> +	struct xfs_inode	*child,
>> +	struct xfs_parent_name_irec *nrec,
>> +	struct xfs_defer_ops	*dfops,
>> +	xfs_fsblock_t		*firstblock)
>> +{
>> +	struct xfs_parent_name_rec rec;
>> +
>> +	rec.p_ino = cpu_to_be64(nrec->p_ino);
>> +	rec.p_gen = cpu_to_be32(nrec->p_gen);
>> +	rec.p_diroffset = cpu_to_be32(nrec->p_diroffset);
>> +
>> +	return xfs_attr_set_first_parent(tp, child, &rec, sizeof(rec),
>> +				   nrec->p_name, nrec->p_namelen,
>> +				   dfops, firstblock);
>> +}
>> +
>> +int
>> +xfs_parent_create(
>> +	struct xfs_trans	*tp,
>> +	struct xfs_inode	*parent,
>> +	struct xfs_inode	*child,
>> +	struct xfs_name		*child_name,
>> +	xfs_dir2_dataptr_t	diroffset,
>> +	struct xfs_defer_ops	*dfops,
>> +	xfs_fsblock_t		*firstblock)
>> +{
>> +	struct xfs_parent_name_irec nrec;
>> +
>> +	nrec.p_ino = parent->i_ino;
>> +	nrec.p_gen = VFS_I(parent)->i_generation;
>> +	nrec.p_diroffset = diroffset;
>> +	nrec.p_name = child_name->name;
>> +	nrec.p_namelen = child_name->len;
>> +
>> +	return xfs_parent_create_nrec(tp, child, &nrec, dfops, firstblock);
>> +}
>> diff --git a/fs/xfs/xfs_attr.h b/fs/xfs/xfs_attr.h
>> index 3031a09..fad2fbd 100644
>> --- a/fs/xfs/xfs_attr.h
>> +++ b/fs/xfs/xfs_attr.h
>> @@ -155,5 +155,16 @@ int xfs_attr_remove(struct xfs_inode *dp, const unsigned char *name, int flags);
>>   int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
>>   		  int flags, struct attrlist_cursor_kern *cursor);
>>   
>> -
>> +/*
>> + * Parent pointer attribute prototypes
>> + */
>> +int xfs_parent_create(struct xfs_trans *tp, struct xfs_inode *parent,
>> +		      struct xfs_inode *child, struct xfs_name *child_name,
>> +		      xfs_dir2_dataptr_t diroffset, struct xfs_defer_ops *dfops,
>> +		      xfs_fsblock_t *firstblock);
>> +int xfs_attr_set_first_parent(struct xfs_trans *tp, struct xfs_inode *ip,
>> +			      struct xfs_parent_name_rec *rec, int reclen,
>> +			      const char *value, int valuelen,
>> +			      struct xfs_defer_ops *dfops,
>> +			      xfs_fsblock_t *firstblock);
>>   #endif	/* __XFS_ATTR_H__ */
>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
>> index 52c331e..3557791 100644
>> --- a/fs/xfs/xfs_inode.c
>> +++ b/fs/xfs/xfs_inode.c
>> @@ -1162,6 +1162,7 @@ xfs_create(
>>   	struct xfs_dquot	*pdqp = NULL;
>>   	struct xfs_trans_res	*tres;
>>   	uint			resblks;
>> +	xfs_dir2_dataptr_t	diroffset;
>>   
>>   	trace_xfs_create(dp, name);
>>   
>> @@ -1251,7 +1252,7 @@ xfs_create(
>>   	error = xfs_dir_createname(tp, dp, name, ip->i_ino,
>>   					&first_block, &dfops, resblks ?
>>   					resblks - XFS_IALLOC_SPACE_RES(mp) : 0,
>> -					NULL);
>> +					&diroffset);
>>   	if (error) {
>>   		ASSERT(error != -ENOSPC);
>>   		goto out_trans_cancel;
>> @@ -1270,6 +1271,19 @@ xfs_create(
>>   	}
>>   
>>   	/*
>> +	 * If we have parent pointers, we need to add the attribute containing
>> +	 * the parent information now. This must be done within the same
>> +	 * transaction the directory entry is created, while the new inode
>> +	 * contains nothing in the inode literal area.
>> +	 */
>> +	if (xfs_sb_version_hasparent(&mp->m_sb)) {
>> +		error = xfs_parent_create(tp, dp, ip, name, diroffset,
>> +					  &dfops, &first_block);
>> +		if (error)
>> +			goto out_bmap_cancel;
>> +	}
>> +
>> +	/*
>>   	 * If this is a synchronous mount, make sure that the
>>   	 * create transaction goes to disk before returning to
>>   	 * the user.
>> -- 
>> 2.7.4
>>
>> --
>> 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: [RFC PATCH 13/13] Add the parent pointer support to the superblock version 5.

[Allison Henderson]

On 08/14/2017 05:07 PM, Darrick J. Wong wrote:

> On Wed, Aug 09, 2017 at 06:41:33PM -0700, Allison Henderson wrote:
>> [dchinner: forward ported and cleaned up]
>> [achender: rebased and added parent pointer attribute to
>>             compatible attributes mask]
>>
>> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
>> Signed-off-by: Dave Chinner <dchinner@redhat.com>
>> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
>> ---
>> :100644 100644 6cedd75... b00bf1d... M	fs/xfs/libxfs/xfs_format.h
>> :100644 100644 fba0105... a18dd0c... M	fs/xfs/libxfs/xfs_fs.h
>> :100644 100644 50fb3a2... d7b9010... M	fs/xfs/xfs_fsops.c
>>   fs/xfs/libxfs/xfs_format.h | 13 ++++++++-----
>>   fs/xfs/libxfs/xfs_fs.h     |  1 +
>>   fs/xfs/xfs_fsops.c         |  4 +++-
>>   3 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
>> index 6cedd75..b00bf1d 100644
>> --- a/fs/xfs/libxfs/xfs_format.h
>> +++ b/fs/xfs/libxfs/xfs_format.h
>> @@ -464,10 +464,12 @@ xfs_sb_has_compat_feature(
>>   #define XFS_SB_FEAT_RO_COMPAT_FINOBT   (1 << 0)		/* free inode btree */
>>   #define XFS_SB_FEAT_RO_COMPAT_RMAPBT   (1 << 1)		/* reverse map btree */
>>   #define XFS_SB_FEAT_RO_COMPAT_REFLINK  (1 << 2)		/* reflinked files */
>> +#define XFS_SB_FEAT_RO_COMPAT_PARENT	(1 << 3)	/* parent inode ptr */
>>   #define XFS_SB_FEAT_RO_COMPAT_ALL \
>>   		(XFS_SB_FEAT_RO_COMPAT_FINOBT | \
>>   		 XFS_SB_FEAT_RO_COMPAT_RMAPBT | \
>> -		 XFS_SB_FEAT_RO_COMPAT_REFLINK)
>> +		 XFS_SB_FEAT_RO_COMPAT_REFLINK| \
>> +		 XFS_SB_FEAT_RO_COMPAT_PARENT)
>>   #define XFS_SB_FEAT_RO_COMPAT_UNKNOWN	~XFS_SB_FEAT_RO_COMPAT_ALL
>>   static inline bool
>>   xfs_sb_has_ro_compat_feature(
>> @@ -507,17 +509,17 @@ xfs_sb_has_incompat_log_feature(
>>   /*
>>    * V5 superblock specific feature checks
>>    */
>> -static inline int xfs_sb_version_hascrc(struct xfs_sb *sbp)
>> +static inline bool xfs_sb_version_hascrc(struct xfs_sb *sbp)
>>   {
>>   	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
>>   }
>>   
>> -static inline int xfs_sb_version_has_pquotino(struct xfs_sb *sbp)
>> +static inline bool xfs_sb_version_has_pquotino(struct xfs_sb *sbp)
> Not part of parent pointers; if you want to fix this, do it in a separate
> patch, please.
>
> --D

Ok, will drop for now.  I may come back and get them later.  Thanks for 
the review!

Allison
>
>>   {
>>   	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
>>   }
>>   
>> -static inline int xfs_sb_version_hasftype(struct xfs_sb *sbp)
>> +static inline bool xfs_sb_version_hasftype(struct xfs_sb *sbp)
>>   {
>>   	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
>>   		xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_FTYPE)) ||
>> @@ -563,7 +565,8 @@ static inline bool xfs_sb_version_hasreflink(struct xfs_sb *sbp)
>>   
>>   static inline bool xfs_sb_version_hasparent(struct xfs_sb *sbp)
>>   {
>> -	return false; /* We'll enable this at the end of the set */
>> +	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
>> +		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_PARENT));
>>   }
>>   
>>   /*
>> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
>> index fba0105..a18dd0c 100644
>> --- a/fs/xfs/libxfs/xfs_fs.h
>> +++ b/fs/xfs/libxfs/xfs_fs.h
>> @@ -222,6 +222,7 @@ typedef struct xfs_fsop_resblks {
>>   #define XFS_FSOP_GEOM_FLAGS_SPINODES	0x40000	/* sparse inode chunks	*/
>>   #define XFS_FSOP_GEOM_FLAGS_RMAPBT	0x80000	/* reverse mapping btree */
>>   #define XFS_FSOP_GEOM_FLAGS_REFLINK	0x100000 /* files can share blocks */
>> +#define XFS_FSOP_GEOM_FLAGS_PARENT	0x200000 /* parent pointers */
>>   
>>   /*
>>    * Minimum and maximum sizes need for growth checks.
>> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
>> index 50fb3a2..d7b9010 100644
>> --- a/fs/xfs/xfs_fsops.c
>> +++ b/fs/xfs/xfs_fsops.c
>> @@ -112,7 +112,9 @@ xfs_fs_geometry(
>>   			(xfs_sb_version_hasrmapbt(&mp->m_sb) ?
>>   				XFS_FSOP_GEOM_FLAGS_RMAPBT : 0) |
>>   			(xfs_sb_version_hasreflink(&mp->m_sb) ?
>> -				XFS_FSOP_GEOM_FLAGS_REFLINK : 0);
>> +				XFS_FSOP_GEOM_FLAGS_REFLINK : 0) |
>> +			(xfs_sb_version_hasparent(&mp->m_sb) ?
>> +				XFS_FSOP_GEOM_FLAGS_PARENT : 0);
>>   		geo->logsectsize = xfs_sb_version_hassector(&mp->m_sb) ?
>>   				mp->m_sb.sb_logsectsize : BBSIZE;
>>   		geo->rtsectsize = mp->m_sb.sb_blocksize;
>> -- 
>> 2.7.4
>>
>> --
>> 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: [RFC PATCH 02/13] xfs: get directory offset when removing directory name

[Dave Chinner]

On Tue, Aug 15, 2017 at 01:23:07PM +1000, Dave Chinner wrote:
> On Mon, Aug 14, 2017 at 04:56:53PM -0700, Darrick J. Wong wrote:
> > The biggest complaint I have about this patch series is that we end up
> > using separate transactions for each phase of the link/unlink
> > operations.  There's no guarantee that once we start the process of
> > expand-dir, add-name-to-dir, log-inode-cores, expand-attr,
> > add-pptr-to-attrs that we actually get all the way to the end of that
> > process, particularly if we crash in the middle and have to recover the
> > log coming back up.
> 
> Yes, that's a problem that we had not been solved at the point in
> time I wrote that patchset. After making the parent pointer
> attributes be added and removed correctly, then next thing to do was
> to make them atomic and replayable. And that's the bit I never got
> to.
> 
> > Obviously, back in 2014 when Dave started on these patches there was no
> > defer_ops, so a lot of effort goes into passing parameters up and down
> > the stack and fiddling with the transaction reservation type
> > declarations in order to guarantee that we can shove everything into a
> > single transaction.  Now that we have a more general mechanism to split
> > complex updates into a chain of smaller updates (and make log recovery
> > pick up where we left off) it's time to figure out how to make attr (and
> > maybe dir) updates a deferrable operation.
> 
> Right. We've got the infrastructure now, but it's nasty to retrofit
> to all the attr code.
> 
> > This adds complexity to the log because we have to define an "attr add
> > KEY:VALUE" and a "attr delete KEY" deferred op that hooks into the
> > existing attr code.  For parent pointers it's not a big deal to log
> > values since they're not big, but for the general case this idea needs
> > more thought, or a decision that we'll just keep doing things the same
> > way.
> 
> The problem with this is that it requires us to log the entire
> attribute value, which could be 64k in size. That used to be a big
> issue because log bandwidth was at a premium. That isn't so much a
> problem now, so logging the attr value and getting rid of the
> unlogged sync write for remote attr updates is the way I think we
> should head.

Just had another thought about this - if we rewrite any code to use
a different set of transaction structures, we're going to need to
use log incompat flags to identify each of the changed transaction
sets that require redo item support to replay. That's not a huge
deal - it just means that people taking filesystems back to old
kernels will need to make sure the log is clean. i.e. mount sets,
unmount/remount-ro clears the relevant log incompat flags.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com