xfs: parent pointers
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
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
@@ -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
@@ -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__ */
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
userspacexfs_scrubbits and is probably too ugly to live. Wait for the real implementation.Signed-off-by Darrick J. Wong <[email protected]>---
[...]
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.
PATCH 2/2 xfsprogs: implement the upper half of parent pointers
In reply to Darrick J. WongFrom: 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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. */