Skip to contentSkip to conversation

xfs: parent pointers

PATCH In review Darrick J. Wong 2 participants 4 messages Dec 8, 2017 XFS filesystem

lore
Share & tools
lore
#0 lore raw

cover xfs: parent pointers

Hi all,

So I've been working in my spare time (ha ha) to build a prototype parent pointer ioctl backend so that I can play around with the userspace side of things -- namely (cough) having xfs_scrub be able to resolve an (ino, gen) tuple back to a user-friendly file path.

The first ugly patch attacks the dentry cache to use d_parent to provide one of the parents of some file. Obviously, this is totally deficient as we can only report one parent and the file has to have been opened via path at some previous point in time, and I bet the locking is totally wrong. It's too ugly to live but it /does/ function as a stub.

The second patch implements two iterators atop the parent pointer ioctl we've been tossing around on the mailing list -- one to return parents of an open file/file handle, and a second one to return all the paths of an open file/file handle, and plunges it into (part of) xfs_scrub and the xfs_io parent command. Note that I'm expecting xfs_scrub/xfs_repair to take over parent pointer checking and fixing, so I removed the checker function from xfs_io.

This is _not_ a substitute for Allison's parent pointer patches, as they provide the meat of the real feature... but seeing how much bikeshedding ioctls invite, we might as well start the reconciliation process now.

--D

#1 lore raw

PATCH 1/2 xfs: sketchy implementation of parent pointers

From: Darrick J. Wong <[email protected]>

This is a, um, "sample" implementation of parent pointers that abuses struct dentry to return one parent pointer of a file ... if the dentry cache has been connected. This exists only to enable testing of the userspace xfs_scrub bits and is probably too ugly to live. Wait for the real implementation.

---
 fs/xfs/Makefile        |    1 
 fs/xfs/libxfs/xfs_fs.h |   57 +++++++++++++
 fs/xfs/xfs_ioctl.c     |   49 +++++++++++
 fs/xfs/xfs_parent.c    |  214 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_parent.h    |   29 +++++++
 5 files changed, 350 insertions(+)
 create mode 100644 fs/xfs/xfs_parent.c
 create mode 100644 fs/xfs/xfs_parent.h
fs/xfs/Makefile +1 -0
index e1768e7..9f4de14 100644
@@ -99,6 +99,7 @@ xfs-y				+= xfs_aops.o \
 				   xfs_message.o \
 				   xfs_mount.o \
 				   xfs_mru_cache.o \
+				   xfs_parent.o \
 				   xfs_reflink.o \
 				   xfs_stats.o \
 				   xfs_super.o \
fs/xfs/libxfs/xfs_fs.h +57 -0

69 lines not on this page · raw message

fs/xfs/xfs_ioctl.c +49 -0

67 lines not on this page · raw message

fs/xfs/xfs_parent.c new file +214 -0

214 lines not on this page · raw message

fs/xfs/xfs_parent.h new file +29 -0
new file mode 100644
index 0000000..23ee505
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2017 Oracle.  All Rights Reserved.
+ *
+ * Author: Darrick J. Wong <[email protected]>
+ *
+ * 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; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+#ifndef __XFS_PARENT_H__
+#define __XFS_PARENT_H__
+
+/* pptr to userspace formatter - copy to user & advance pointer */
+typedef int (*xfs_parent_format_t)(struct xfs_pptr *, void *);
+
+int xfs_parent_get_pointers(struct file *filp, struct xfs_pptr_info *pi,
+		xfs_parent_format_t formatter, void *priv);
+
+#endif /* __XFS_PARENT_H__ */
#2 lore raw

Re: [RFCRAP DONOTMERGE PATCH 1/2] xfs: sketchy implementation of parent pointers

Quoted 5 lines · Darrick J. Wong

From: Darrick J. Wong <[email protected]>

This is a, um, "sample" implementation of parent pointers that abuses struct dentry to return one parent pointer of a file ... if the dentry cache has been connected.

Note that your sample implementation will return parent pointer of a directory even if it wasn't already in dcache.

Quoted 6 lines

This exists only to enable testing of the
userspace xfs_scrub bits and is probably too ugly to live. Wait for the real implementation.

---

[...]

fs/xfs/xfs_parent.c line 77

Quoted 10 lines
fs/xfs/xfs_parent.c new file +10 -0
+/*
+ * No need to do permission checks on the various pathname components
+ * as the handle operations are privileged.
+ */
+STATIC int
+xfs_parent_handle_acceptable(
+       void                    *context,
+       struct dentry           *dentry)
+{
+       return 1;

For the sake of fun, assuming that you want to gear up your sample to return all parents, the acceptable callback is an iterator on all aliases of inode in dcache.
So if you pass in your parent iterator context to this iterator, you can emit all parents
from this callback. Just return 0 to keep iterating. You won't get a dentry back from exportfs_decode_fh(), but you won't need it.

Quoted 43 lines
fs/xfs/xfs_parent.c new file +43 -0
+}
+
+/* Get parent info for an inode by walking the parent dentry. */
+static int
+xfs_parent_get_dparent(
+       struct file             *filp,
+       struct xfs_pptr_info    *pi,
+       struct xfs_pptr_buf     *pb)
+{
+       struct xfs_inode        *ip;
+       struct inode            *dir_inode;
+       struct dentry           *dentry;
+       struct dentry           *dparent;
+       unsigned int            ilock;
+       int                     error = 0;
+
+       pi->pi_oflags |= XFS_PPTR_OFLAG_PARTIAL;
+
+       /* Any nonzero byte in the cursor means we've already retrieved one. */
+       if (memchr_inv(&pi->pi_cursor, 0, sizeof(pi->pi_cursor)))
+               return 0;
+       memset(&pi->pi_cursor, 0xFF, sizeof(pi->pi_cursor));
+
+       if (pi->pi_iflags & XFS_PPTR_IFLAG_HANDLE) {
+               struct xfs_handle       *handle = &pi->pi_handle;
+               struct xfs_fid64        fid = { 0 };
+
+               /* Extract the desired file from the handle info. */
+               if (sizeof(handle->ha_fid) - sizeof(handle->ha_fid.fid_len) !=
+                               handle->ha_fid.fid_len)
+                       return -EINVAL;
+
+               fid.ino = handle->ha_fid.fid_ino;
+               fid.gen = handle->ha_fid.fid_gen;
+
+               dentry = exportfs_decode_fh(filp->f_path.mnt,
+                               (struct fid *)&fid, 3,
+                               FILEID_INO32_GEN | XFS_FILEID_TYPE_64FLAG,
+                               xfs_parent_handle_acceptable, NULL);
+               if (IS_ERR(dentry))
+                       return PTR_ERR(dentry);
+       } else {
+               /* Or just use the dentry of the open file. */

fs/xfs/xfs_parent.c line 130

Quoted 10 lines
fs/xfs/xfs_parent.c new file +10 -0
+               dentry = dget(file_dentry(filp));
+       }
+
+       /* Lock XFS inode... */
+       ip = XFS_I(d_inode(dentry));
+       ilock = XFS_IOLOCK_SHARED | XFS_MMAPLOCK_SHARED | XFS_ILOCK_SHARED;
+       xfs_ilock(ip, ilock);
+
+       /* Bail out early for the root dir. */
+       if (ip->i_ino == ip->i_mount->m_sb.sb_rootino) {

Is it interesting for the API to filter path that are under the root of the mount where the ioctl was issued from (i.e. bind mount case)? Probably not. This API should probably be bind mount blind, but userspace should know to expect that it may can get a real fs path, that is not really resolvable in its mount namespace.

fs/xfs/xfs_parent.c line 139

Quoted 9 lines
fs/xfs/xfs_parent.c new file +9 -0
+               pi->pi_oflags |= XFS_PPTR_OFLAG_ROOT;
+               goto out_dentry;
+       }
+
+       /* Filter out the unconnected inodes. */
+       if (d_unlinked(dentry) || (dentry->d_flags & DCACHE_DISCONNECTED))
+               goto out_dentry;
+       if (IS_ROOT(dentry)) {
+               pi->pi_oflags |= XFS_PPTR_OFLAG_ROOT;

This is a bit confusing... IS_ROOT(dentry) does not mean that you found the fs root, it really means disconnected of first order (parent == self), while DCACHE_DISCONNECTED means not connected up to fs root. You want to check if dentry == dentry->d_sb->s_root.

Quoted 9 lines
fs/xfs/xfs_parent.c new file +9 -0
+               goto out_dentry;
+       }
+
+       /* Otherwise look up the parent... */
+       dparent = dget_parent(dentry);
+       if (IS_ERR(dparent)) {
+               error = PTR_ERR(dparent);
+               goto out_dentry;
+       }

fs/xfs/xfs_parent.c line 158

Quoted 10 lines
fs/xfs/xfs_parent.c new file +10 -0
+
+       dir_inode = d_inode(dparent);
+       if (!dir_inode)
+               goto out_dparent;
+
+       /* ...and emit a record. */
+       error = xfs_parent_emit(pi, pb, dir_inode->i_ino,
+                       dir_inode->i_generation,
+                       ACCESS_ONCE(dentry->d_name.len),
+                       ACCESS_ONCE(dentry->d_name.name));

Just in case some bits of this patch are going to be used in final implementation, you need to copy d_name either under dentry->d_lock or use take_dentry_name_snapshot().

Cheers,
Amir.

#3 lore raw

PATCH 2/2 xfsprogs: implement the upper half of parent pointers

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Add ioctl definitions to libxfs, build the necessary helpers into libfrog and libhandle to iterate parents (and parent paths), then wire up xfs_scrub to be able to query parent pointers from userspace. The goal of this patch is to exercise userspace, and is nowhere near a complete solution. A basic xfs_io parent command implementation replaces ... whatever that is that's there now.

Totally missing: actual support in libxfs for working with parent ptrs straight off the disk (mkfs, xfs_db, xfs_repair).

---
 fsr/Makefile       |    5 -
 include/handle.h   |    1 
 include/parent.h   |   20 ++
 include/path.h     |   19 ++
 io/parent.c        |  459 +++++++++++++---------------------------------------
 libfrog/paths.c    |  135 +++++++++++++++
 libhandle/Makefile |    2 
 libhandle/handle.c |    7 -
 libhandle/parent.c |  324 +++++++++++++++++++++++++++++++++++++
 libxfs/xfs_fs.h    |   57 ++++++
 scrub/inodes.c     |   25 +++
 scrub/inodes.h     |    2 
 scrub/phase5.c     |    9 -
 13 files changed, 708 insertions(+), 357 deletions(-)
 create mode 100644 libhandle/parent.c
fsr/Makefile +4 -1
index d3521b2..e5baf02 100644
@@ -7,7 +7,10 @@ include $(TOPDIR)/include/builddefs
 
 LTCOMMAND = xfs_fsr
 CFILES = xfs_fsr.c
-LLDLIBS = $(LIBHANDLE)
+
+LLDLIBS += $(LIBHANDLE) $(LIBFROG) $(LIBPTHREAD)
+LTDEPENDENCIES += $(LIBHANDLE) $(LIBFROG)
+LLDFLAGS = -static
 
 ifeq ($(HAVE_GETMNTENT),yes)
 LCFLAGS += -DHAVE_GETMNTENT
include/handle.h +1 -0
index 49f1441..cae0e99 100644
@@ -51,6 +51,7 @@ extern int  fssetdm_by_handle (void *__hanp, size_t __hlen,
 			       struct fsdmidata *__fsdmi);
 
 void fshandle_destroy(void);
+int handle_to_fsfd(void *hanp, char **path);
 
 #ifdef __cplusplus
 }
include/parent.h +19 -1
index f338f96..9757fb6 100644
@@ -16,7 +16,25 @@
  * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 #ifndef __PARENT_H__
-#define	__PARENT_H__
+#define __PARENT_H__
+
+struct path_list;
+
+typedef int (*walk_pptr_fn)(struct xfs_pptr_info *pi, struct xfs_pptr *pptr,
+		void *arg);
+typedef int (*walk_ppath_fn)(const char *mntpt, struct path_list *path,
+		void *arg);
+
+#define WALK_PPTRS_ABORT	1
+int fd_walk_pptrs(int fd, walk_pptr_fn fn, void *arg);
+int handle_walk_pptrs(void *hanp, size_t hanlen, walk_pptr_fn fn, void *arg);
+
+#define WALK_PPATHS_ABORT	1
+int fd_walk_ppaths(int fd, walk_ppath_fn fn, void *arg);
+int handle_walk_ppaths(void *hanp, size_t hanlen, walk_ppath_fn fn, void *arg);
+
+int fd_to_path(int fd, char *path, size_t pathlen);
+int handle_to_path(void *hanp, size_t hlen, char *path, size_t pathlen);
 
 typedef struct parent {
 	__u64	p_ino;
include/path.h +19 -0
index 1d3a902..9b0c8a3 100644
@@ -69,4 +69,23 @@ typedef struct fs_cursor {
 extern void fs_cursor_initialise(char *__dir, uint __flags, fs_cursor_t *__cp);
 extern fs_path_t *fs_cursor_next_entry(fs_cursor_t *__cp);
 
+/* Path information. */
+
+struct path_list;
+struct path_component;
+
+struct path_component *path_component_init(const char *name);
+void path_component_free(struct path_component *pc);
+int path_component_change(struct path_component *pc, void *name,
+		size_t namelen);
+
+struct path_list *path_list_init(void);
+void path_list_free(struct path_list *path);
+void path_list_add_parent_component(struct path_list *path,
+		struct path_component *pc);
+void path_list_add_component(struct path_list *path, struct path_component *pc);
+void path_list_del_component(struct path_list *path, struct path_component *pc);
+
+ssize_t path_list_to_string(struct path_list *path, char *buf, size_t buflen);
+
 #endif	/* __PATH_H__ */
io/parent.c +115 -344

529 lines not on this page · raw message

libfrog/paths.c +135 -0

144 lines not on this page · raw message

libhandle/Makefile +1 -1
index fe1a2af..d3cea41 100644
@@ -16,7 +16,7 @@ else
 LTLDFLAGS += -Wl,--version-script,libhandle.sym
 endif
 
-CFILES = handle.c jdm.c
+CFILES = handle.c jdm.c parent.c
 LSRCFILES = libhandle.sym
 
 default: ltdepend $(LTLIBRARY)
libhandle/handle.c +4 -3
index 878d14d..a70fa32 100644
@@ -41,7 +41,6 @@ typedef union {
 } comarg_t;
 
 static int obj_to_handle(char *, int, unsigned int, comarg_t, void**, size_t*);
-static int handle_to_fsfd(void *, char **);
 static char *path_to_fspath(char *path);
 
 
@@ -214,8 +213,10 @@ handle_to_fshandle(
 	return 0;
 }
 
-static int
-handle_to_fsfd(void *hanp, char **path)
+int
+handle_to_fsfd(
+	void		*hanp,
+	char		**path)
 {
 	struct fdhash	*fdhp;
 
libhandle/parent.c new file +324 -0

324 lines not on this page · raw message

libxfs/xfs_fs.h +57 -0

69 lines not on this page · raw message

scrub/inodes.c +25 -0
index c880c36..076ea6f 100644
@@ -33,6 +33,7 @@
 #include "xfs_scrub.h"
 #include "common.h"
 #include "inodes.h"
+#include "parent.h"
 
 /*
  * Iterate a range of inodes.
@@ -282,3 +283,27 @@ xfs_open_handle(
 	return open_by_fshandle(handle, sizeof(*handle),
 			O_RDONLY | O_NOATIME | O_NOFOLLOW | O_NOCTTY);
 }
+
+/* Construct a description for an inode. */
+void
+xfs_scrub_ino_descr(
+	struct scrub_ctx	*ctx,
+	struct xfs_handle	*handle,
+	char			*buf,
+	size_t			buflen)
+{
+	uint64_t		ino;
+	xfs_agnumber_t		agno;
+	xfs_agino_t		agino;
+	int			ret;
+
+	ret = handle_to_path(handle, sizeof(struct xfs_handle), buf, buflen);
+	if (ret >= 0)
+		return;
+
+	ino = handle->ha_fid.fid_ino;
+	agno = ino / (1ULL << (ctx->inopblog + ctx->agblklog));
+	agino = ino % (1ULL << (ctx->inopblog + ctx->agblklog));
+	snprintf(buf, buflen, _("inode %"PRIu64" (%u/%u)"), ino, agno,
+			agino);
+}
scrub/inodes.h +2 -0
index c398677..912985e 100644
@@ -28,5 +28,7 @@ bool xfs_scan_all_inodes(struct scrub_ctx *ctx, xfs_inode_iter_fn fn,
 		void *arg);
 
 int xfs_open_handle(struct xfs_handle *handle);
+void xfs_scrub_ino_descr(struct scrub_ctx *ctx, struct xfs_handle *handle,
+		char *buf, size_t buflen);
 
 #endif /* XFS_SCRUB_INODES_H_ */
scrub/phase5.c +2 -7
index 72b7cda..ba2e412 100644
@@ -253,16 +253,11 @@ xfs_scrub_connections(
 	void			*arg)
 {
 	bool			*pmoveon = arg;
-	char			descr[DESCR_BUFSZ];
+	char			descr[PATH_MAX];
 	bool			moveon;
-	xfs_agnumber_t		agno;
-	xfs_agino_t		agino;
 	int			fd = -1;
 
-	agno = bstat->bs_ino / (1ULL << (ctx->inopblog + ctx->agblklog));
-	agino = bstat->bs_ino % (1ULL << (ctx->inopblog + ctx->agblklog));
-	snprintf(descr, DESCR_BUFSZ, _("inode %"PRIu64" (%u/%u)"),
-			(uint64_t)bstat->bs_ino, agno, agino);
+	xfs_scrub_ino_descr(ctx, handle, descr, PATH_MAX);
 	background_sleep();
 
         /* Warn about naming problems in xattrs. */