All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] xfsprogs: 4.15 rollup
@ 2017-11-17 19:56 Darrick J. Wong
  2017-11-17 19:56 ` [PATCH 1/5] xfs_db: print structure padding fields consistently Darrick J. Wong
                   ` (4 more replies)
  0 siblings, 5 replies; 26+ messages in thread
From: Darrick J. Wong @ 2017-11-17 19:56 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

Hi all,

Here are some patches fixing up random odds and ends in 4.15.  xfs_db is
changed to print all padding fields, xfs_io starts pulling error tag
definitions from libxfs and talk to the scrub ioctl, and manpage updates
for the new scrub ioctl.

--D

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/5] xfs_db: print structure padding fields consistently
  2017-11-17 19:56 [PATCH 0/5] xfsprogs: 4.15 rollup Darrick J. Wong
@ 2017-11-17 19:56 ` Darrick J. Wong
  2017-11-30 21:37   ` Eric Sandeen
  2017-12-04 23:54   ` [PATCH 1/5 V2] " Eric Sandeen
  2017-11-17 19:56 ` [PATCH 2/5] xfs_db: add missing padding fields Darrick J. Wong
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 26+ messages in thread
From: Darrick J. Wong @ 2017-11-17 19:56 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

We are very inconsistent about how we print padding fields in on-disk
structures -- sometimes we hide it from printall, sometimes we deviate
from unsigned hex values, etc.  Make this all consistent -- never hide
padding values, always print them as unsigned hex integers.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/attr.c  |   10 +++++-----
 db/dir2.c  |    4 ++--
 db/dquot.c |    4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)


diff --git a/db/attr.c b/db/attr.c
index 75fe239..ba8313c 100644
--- a/db/attr.c
+++ b/db/attr.c
@@ -73,7 +73,7 @@ const field_t	attr_blkinfo_flds[] = {
 	{ "forw", FLDT_ATTRBLOCK, OI(BOFF(forw)), C1, 0, TYP_ATTR },
 	{ "back", FLDT_ATTRBLOCK, OI(BOFF(back)), C1, 0, TYP_ATTR },
 	{ "magic", FLDT_UINT16X, OI(BOFF(magic)), C1, 0, TYP_NONE },
-	{ "pad", FLDT_UINT16X, OI(BOFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
+	{ "pad", FLDT_UINT16X, OI(BOFF(pad)), C1, 0, TYP_NONE },
 	{ NULL }
 };
 
@@ -94,7 +94,7 @@ const field_t	attr_leaf_entry_flds[] = {
 	{ "local", FLDT_UINT1,
 	  OI(LEOFF(flags) + bitsz(uint8_t) - XFS_ATTR_LOCAL_BIT - 1), C1, 0,
 	  TYP_NONE },
-	{ "pad2", FLDT_UINT8X, OI(LEOFF(pad2)), C1, FLD_SKIPALL, TYP_NONE },
+	{ "pad2", FLDT_UINT8X, OI(LEOFF(pad2)), C1, 0, TYP_NONE },
 	{ NULL }
 };
 
@@ -105,7 +105,7 @@ const field_t	attr_leaf_hdr_flds[] = {
 	{ "usedbytes", FLDT_UINT16D, OI(LHOFF(usedbytes)), C1, 0, TYP_NONE },
 	{ "firstused", FLDT_UINT16D, OI(LHOFF(firstused)), C1, 0, TYP_NONE },
 	{ "holes", FLDT_UINT8D, OI(LHOFF(holes)), C1, 0, TYP_NONE },
-	{ "pad1", FLDT_UINT8X, OI(LHOFF(pad1)), C1, FLD_SKIPALL, TYP_NONE },
+	{ "pad1", FLDT_UINT8X, OI(LHOFF(pad1)), C1, 0, TYP_NONE },
 	{ "freemap", FLDT_ATTR_LEAF_MAP, OI(LHOFF(freemap)),
 	  CI(XFS_ATTR_LEAF_MAPSIZE), FLD_ARRAY, TYP_NONE },
 	{ NULL }
@@ -567,7 +567,7 @@ const field_t	attr3_leaf_hdr_flds[] = {
 	{ "usedbytes", FLDT_UINT16D, OI(LH3OFF(usedbytes)), C1, 0, TYP_NONE },
 	{ "firstused", FLDT_UINT16D, OI(LH3OFF(firstused)), C1, 0, TYP_NONE },
 	{ "holes", FLDT_UINT8D, OI(LH3OFF(holes)), C1, 0, TYP_NONE },
-	{ "pad1", FLDT_UINT8X, OI(LH3OFF(pad1)), C1, FLD_SKIPALL, TYP_NONE },
+	{ "pad1", FLDT_UINT8X, OI(LH3OFF(pad1)), C1, 0, TYP_NONE },
 	{ "freemap", FLDT_ATTR_LEAF_MAP, OI(LH3OFF(freemap)),
 	  CI(XFS_ATTR_LEAF_MAPSIZE), FLD_ARRAY, TYP_NONE },
 	{ NULL }
@@ -589,7 +589,7 @@ const field_t	attr3_node_hdr_flds[] = {
 	{ "info", FLDT_ATTR3_BLKINFO, OI(H3OFF(info)), C1, 0, TYP_NONE },
 	{ "count", FLDT_UINT16D, OI(H3OFF(__count)), C1, 0, TYP_NONE },
 	{ "level", FLDT_UINT16D, OI(H3OFF(__level)), C1, 0, TYP_NONE },
-	{ "pad", FLDT_UINT32D, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
+	{ "pad", FLDT_UINT32X, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
 	{ NULL }
 };
 
diff --git a/db/dir2.c b/db/dir2.c
index 3e21a7b..bd9c6aa 100644
--- a/db/dir2.c
+++ b/db/dir2.c
@@ -171,7 +171,7 @@ const field_t	da_blkinfo_flds[] = {
 	{ "forw", FLDT_DIRBLOCK, OI(DBOFF(forw)), C1, 0, TYP_INODATA },
 	{ "back", FLDT_DIRBLOCK, OI(DBOFF(back)), C1, 0, TYP_INODATA },
 	{ "magic", FLDT_UINT16X, OI(DBOFF(magic)), C1, 0, TYP_NONE },
-	{ "pad", FLDT_UINT16X, OI(DBOFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
+	{ "pad", FLDT_UINT16X, OI(DBOFF(pad)), C1, 0, TYP_NONE },
 	{ NULL }
 };
 
@@ -977,7 +977,7 @@ const field_t	da3_node_hdr_flds[] = {
 	{ "info", FLDT_DA3_BLKINFO, OI(H3OFF(info)), C1, 0, TYP_NONE },
 	{ "count", FLDT_UINT16D, OI(H3OFF(__count)), C1, 0, TYP_NONE },
 	{ "level", FLDT_UINT16D, OI(H3OFF(__level)), C1, 0, TYP_NONE },
-	{ "pad", FLDT_UINT32D, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
+	{ "pad", FLDT_UINT32X, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
 	{ NULL }
 };
 
diff --git a/db/dquot.c b/db/dquot.c
index 4e35df4..222c082 100644
--- a/db/dquot.c
+++ b/db/dquot.c
@@ -76,7 +76,7 @@ const field_t	disk_dquot_flds[] = {
 	{ "btimer", FLDT_INT32D, OI(DOFF(btimer)), C1, 0, TYP_NONE },
 	{ "iwarns", FLDT_QWARNCNT, OI(DOFF(iwarns)), C1, 0, TYP_NONE },
 	{ "bwarns", FLDT_QWARNCNT, OI(DOFF(bwarns)), C1, 0, TYP_NONE },
-	{ "pad0", FLDT_INT32D, OI(DOFF(pad0)), C1, FLD_SKIPALL, TYP_NONE },
+	{ "pad0", FLDT_UINT32X, OI(DOFF(pad0)), C1, 0, TYP_NONE },
 	{ "rtb_hardlimit", FLDT_QCNT, OI(DOFF(rtb_hardlimit)), C1, 0,
 	  TYP_NONE },
 	{ "rtb_softlimit", FLDT_QCNT, OI(DOFF(rtb_softlimit)), C1, 0,
@@ -84,7 +84,7 @@ const field_t	disk_dquot_flds[] = {
 	{ "rtbcount", FLDT_QCNT, OI(DOFF(rtbcount)), C1, 0, TYP_NONE },
 	{ "rtbtimer", FLDT_INT32D, OI(DOFF(rtbtimer)), C1, 0, TYP_NONE },
 	{ "rtbwarns", FLDT_QWARNCNT, OI(DOFF(rtbwarns)), C1, 0, TYP_NONE },
-	{ "pad", FLDT_UINT16X, OI(DOFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
+	{ "pad", FLDT_UINT16X, OI(DOFF(pad)), C1, 0, TYP_NONE },
 	{ NULL }
 };
 


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/5] xfs_db: add missing padding fields
  2017-11-17 19:56 [PATCH 0/5] xfsprogs: 4.15 rollup Darrick J. Wong
  2017-11-17 19:56 ` [PATCH 1/5] xfs_db: print structure padding fields consistently Darrick J. Wong
@ 2017-11-17 19:56 ` Darrick J. Wong
  2017-12-04 23:54   ` [PATCH 2/5 V2] " Eric Sandeen
  2017-11-17 19:56 ` [PATCH 3/5] xfs_io: pull xfs errortag definitions from libxfs Darrick J. Wong
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 26+ messages in thread
From: Darrick J. Wong @ 2017-11-17 19:56 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Several data structures are missing padding fields from their field
definitions.  Add them so that they will be printed out.

Fix the AGI field order to be consistent with the structure
definition while we're at it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/agi.c   |    3 ++-
 db/dir2.c  |    3 +++
 db/inode.c |    2 ++
 3 files changed, 7 insertions(+), 1 deletion(-)


diff --git a/db/agi.c b/db/agi.c
index fb69210..29d7ab5 100644
--- a/db/agi.c
+++ b/db/agi.c
@@ -55,8 +55,9 @@ const field_t	agi_flds[] = {
 	{ "unlinked", FLDT_AGINONN, OI(OFF(unlinked)),
 	  CI(XFS_AGI_UNLINKED_BUCKETS), FLD_ARRAY, TYP_NONE },
 	{ "uuid", FLDT_UUID, OI(OFF(uuid)), C1, 0, TYP_NONE },
-	{ "lsn", FLDT_UINT64X, OI(OFF(lsn)), C1, 0, TYP_NONE },
 	{ "crc", FLDT_CRC, OI(OFF(crc)), C1, 0, TYP_NONE },
+	{ "pad32", FLDT_UINT32X, OI(OFF(pad32)), C1, 0, TYP_NONE },
+	{ "lsn", FLDT_UINT64X, OI(OFF(lsn)), C1, 0, TYP_NONE },
 	{ "free_root", FLDT_AGBLOCK, OI(OFF(free_root)), C1, 0, TYP_FINOBT },
 	{ "free_level", FLDT_UINT32D, OI(OFF(free_level)), C1, 0, TYP_NONE },
 	{ NULL }
diff --git a/db/dir2.c b/db/dir2.c
index bd9c6aa..a866c3f 100644
--- a/db/dir2.c
+++ b/db/dir2.c
@@ -940,6 +940,7 @@ const field_t	dir3_data_hdr_flds[] = {
 	{ "hdr", FLDT_DIR3_BLKHDR, OI(DH3OFF(hdr)), C1, 0, TYP_NONE },
 	{ "bestfree", FLDT_DIR2_DATA_FREE, OI(DH3OFF(best_free)),
 	  CI(XFS_DIR2_DATA_FD_COUNT), FLD_ARRAY, TYP_NONE },
+	{ "pad", FLDT_UINT32X, OI(DH3OFF(pad)), C1, 0, TYP_NONE },
 	{ NULL }
 };
 
@@ -948,6 +949,7 @@ const field_t	dir3_leaf_hdr_flds[] = {
 	{ "info", FLDT_DA3_BLKINFO, OI(LH3OFF(info)), C1, 0, TYP_NONE },
 	{ "count", FLDT_UINT16D, OI(LH3OFF(count)), C1, 0, TYP_NONE },
 	{ "stale", FLDT_UINT16D, OI(LH3OFF(stale)), C1, 0, TYP_NONE },
+	{ "pad", FLDT_UINT32X, OI(LH3OFF(pad)), C1, 0, TYP_NONE },
 	{ NULL }
 };
 
@@ -957,6 +959,7 @@ const field_t	dir3_free_hdr_flds[] = {
 	{ "firstdb", FLDT_INT32D, OI(FH3OFF(firstdb)), C1, 0, TYP_NONE },
 	{ "nvalid", FLDT_INT32D, OI(FH3OFF(nvalid)), C1, 0, TYP_NONE },
 	{ "nused", FLDT_INT32D, OI(FH3OFF(nused)), C1, 0, TYP_NONE },
+	{ "pad", FLDT_UINT32X, OI(FH3OFF(pad)), C1, 0, TYP_NONE },
 	{ NULL }
 };
 
diff --git a/db/inode.c b/db/inode.c
index 6f971c6..0f5fb10 100644
--- a/db/inode.c
+++ b/db/inode.c
@@ -102,6 +102,7 @@ const field_t	inode_core_flds[] = {
 	  inode_core_projid_count, FLD_COUNT, TYP_NONE },
 	{ "projid_hi", FLDT_UINT16D, OI(COFF(projid_hi)),
 	  inode_core_projid_count, FLD_COUNT, TYP_NONE },
+	{ "pad", FLDT_UINT8X, OI(OFF(pad)), CI(6), FLD_ARRAY, TYP_NONE },
 	{ "uid", FLDT_UINT32D, OI(COFF(uid)), C1, 0, TYP_NONE },
 	{ "gid", FLDT_UINT32D, OI(COFF(gid)), C1, 0, TYP_NONE },
 	{ "flushiter", FLDT_UINT16D, OI(COFF(flushiter)), C1, 0, TYP_NONE },
@@ -173,6 +174,7 @@ const field_t	inode_v3_flds[] = {
 	{ "lsn", FLDT_UINT64X, OI(COFF(lsn)), C1, 0, TYP_NONE },
 	{ "flags2", FLDT_UINT64X, OI(COFF(flags2)), C1, 0, TYP_NONE },
 	{ "cowextsize", FLDT_EXTLEN, OI(COFF(cowextsize)), C1, 0, TYP_NONE },
+	{ "pad2", FLDT_UINT8X, OI(OFF(pad2)), CI(12), FLD_ARRAY, TYP_NONE },
 	{ "crtime", FLDT_TIMESTAMP, OI(COFF(crtime)), C1, 0, TYP_NONE },
 	{ "inumber", FLDT_INO, OI(COFF(ino)), C1, 0, TYP_NONE },
 	{ "uuid", FLDT_UUID, OI(COFF(uuid)), C1, 0, TYP_NONE },


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/5] xfs_io: pull xfs errortag definitions from libxfs
  2017-11-17 19:56 [PATCH 0/5] xfsprogs: 4.15 rollup Darrick J. Wong
  2017-11-17 19:56 ` [PATCH 1/5] xfs_db: print structure padding fields consistently Darrick J. Wong
  2017-11-17 19:56 ` [PATCH 2/5] xfs_db: add missing padding fields Darrick J. Wong
@ 2017-11-17 19:56 ` Darrick J. Wong
  2017-11-30 21:40   ` Eric Sandeen
  2017-11-17 19:57 ` [PATCH 4/5] xfs_io: provide an interface to the scrub ioctls Darrick J. Wong
  2017-11-17 19:57 ` [PATCH 5/5] man: describe the metadata scrubbing ioctl Darrick J. Wong
  4 siblings, 1 reply; 26+ messages in thread
From: Darrick J. Wong @ 2017-11-17 19:56 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Use the libxfs definitions, don't provide our own.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 include/libxfs.h |    1 +
 io/inject.c      |   35 ++---------------------------------
 2 files changed, 3 insertions(+), 33 deletions(-)


diff --git a/include/libxfs.h b/include/libxfs.h
index 6d08da4..abb01cb 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -61,6 +61,7 @@ extern uint32_t crc32c_le(uint32_t crc, unsigned char const *p, size_t len);
 #include "xfs_sb.h"
 #include "xfs_mount.h"
 #include "xfs_defer.h"
+#include "xfs_errortag.h"
 #include "xfs_da_format.h"
 #include "xfs_da_btree.h"
 #include "xfs_dir2.h"
diff --git a/io/inject.c b/io/inject.c
index 964ebfe..9d0cf62 100644
--- a/io/inject.c
+++ b/io/inject.c
@@ -20,6 +20,7 @@
 #include "input.h"
 #include "init.h"
 #include "io.h"
+#include "xfs_errortag.h"
 
 static cmdinfo_t inject_cmd;
 
@@ -29,70 +30,38 @@ error_tag(char *name)
 	static struct {
 		int	tag;
 		char	*name;
-	} *e, eflags[] = {
-#define XFS_ERRTAG_NOERROR                              0
+	} *e, eflags[XFS_ERRTAG_MAX + 1] = {
 		{ XFS_ERRTAG_NOERROR,			"noerror" },
-#define XFS_ERRTAG_IFLUSH_1                             1
 		{ XFS_ERRTAG_IFLUSH_1,			"iflush1" },
-#define XFS_ERRTAG_IFLUSH_2                             2
 		{ XFS_ERRTAG_IFLUSH_2,			"iflush2" },
-#define XFS_ERRTAG_IFLUSH_3                             3
 		{ XFS_ERRTAG_IFLUSH_3,			"iflush3" },
-#define XFS_ERRTAG_IFLUSH_4                             4
 		{ XFS_ERRTAG_IFLUSH_4,			"iflush4" },
-#define XFS_ERRTAG_IFLUSH_5                             5
 		{ XFS_ERRTAG_IFLUSH_5,			"iflush5" },
-#define XFS_ERRTAG_IFLUSH_6                             6
 		{ XFS_ERRTAG_IFLUSH_6,			"iflush6" },
-#define XFS_ERRTAG_DA_READ_BUF                          7
 		{ XFS_ERRTAG_DA_READ_BUF,		"dareadbuf" },
-#define XFS_ERRTAG_BTREE_CHECK_LBLOCK                   8
 		{ XFS_ERRTAG_BTREE_CHECK_LBLOCK,	"btree_chk_lblk" },
-#define XFS_ERRTAG_BTREE_CHECK_SBLOCK                   9
 		{ XFS_ERRTAG_BTREE_CHECK_SBLOCK,	"btree_chk_sblk" },
-#define XFS_ERRTAG_ALLOC_READ_AGF                       10
 		{ XFS_ERRTAG_ALLOC_READ_AGF,		"readagf" },
-#define XFS_ERRTAG_IALLOC_READ_AGI                      11
 		{ XFS_ERRTAG_IALLOC_READ_AGI,		"readagi" },
-#define XFS_ERRTAG_ITOBP_INOTOBP                        12
 		{ XFS_ERRTAG_ITOBP_INOTOBP,		"itobp" },
-#define XFS_ERRTAG_IUNLINK                              13
 		{ XFS_ERRTAG_IUNLINK,			"iunlink" },
-#define XFS_ERRTAG_IUNLINK_REMOVE                       14
 		{ XFS_ERRTAG_IUNLINK_REMOVE,		"iunlinkrm" },
-#define XFS_ERRTAG_DIR_INO_VALIDATE                     15
 		{ XFS_ERRTAG_DIR_INO_VALIDATE,		"dirinovalid" },
-#define XFS_ERRTAG_BULKSTAT_READ_CHUNK                  16
 		{ XFS_ERRTAG_BULKSTAT_READ_CHUNK,	"bulkstat" },
-#define XFS_ERRTAG_IODONE_IOERR                         17
 		{ XFS_ERRTAG_IODONE_IOERR,		"logiodone" },
-#define XFS_ERRTAG_STRATREAD_IOERR                      18
 		{ XFS_ERRTAG_STRATREAD_IOERR,		"stratread" },
-#define XFS_ERRTAG_STRATCMPL_IOERR                      19
 		{ XFS_ERRTAG_STRATCMPL_IOERR,		"stratcmpl" },
-#define XFS_ERRTAG_DIOWRITE_IOERR                       20
 		{ XFS_ERRTAG_DIOWRITE_IOERR,		"diowrite" },
-#define XFS_ERRTAG_BMAPIFORMAT                          21
 		{ XFS_ERRTAG_BMAPIFORMAT,		"bmapifmt" },
-#define XFS_ERRTAG_FREE_EXTENT				22
 		{ XFS_ERRTAG_FREE_EXTENT,		"free_extent" },
-#define XFS_ERRTAG_RMAP_FINISH_ONE			23
 		{ XFS_ERRTAG_RMAP_FINISH_ONE,		"rmap_finish_one" },
-#define XFS_ERRTAG_REFCOUNT_CONTINUE_UPDATE		24
 		{ XFS_ERRTAG_REFCOUNT_CONTINUE_UPDATE,	"refcount_continue_update" },
-#define XFS_ERRTAG_REFCOUNT_FINISH_ONE			25
 		{ XFS_ERRTAG_REFCOUNT_FINISH_ONE,	"refcount_finish_one" },
-#define XFS_ERRTAG_BMAP_FINISH_ONE			26
 		{ XFS_ERRTAG_BMAP_FINISH_ONE,		"bmap_finish_one" },
-#define XFS_ERRTAG_AG_RESV_CRITICAL			27
 		{ XFS_ERRTAG_AG_RESV_CRITICAL,		"ag_resv_critical" },
-#define XFS_ERRTAG_DROP_WRITES				28
 		{ XFS_ERRTAG_DROP_WRITES,		"drop_writes" },
-#define XFS_ERRTAG_LOG_BAD_CRC				29
 		{ XFS_ERRTAG_LOG_BAD_CRC,		"log_bad_crc" },
-#define XFS_ERRTAG_LOG_ITEM_PIN				30
 		{ XFS_ERRTAG_LOG_ITEM_PIN,		"log_item_pin" },
-#define XFS_ERRTAG_MAX                                  31
 		{ XFS_ERRTAG_MAX,			NULL }
 	};
 	int	count;


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 4/5] xfs_io: provide an interface to the scrub ioctls
  2017-11-17 19:56 [PATCH 0/5] xfsprogs: 4.15 rollup Darrick J. Wong
                   ` (2 preceding siblings ...)
  2017-11-17 19:56 ` [PATCH 3/5] xfs_io: pull xfs errortag definitions from libxfs Darrick J. Wong
@ 2017-11-17 19:57 ` Darrick J. Wong
  2017-12-01  3:46   ` Eric Sandeen
                     ` (2 more replies)
  2017-11-17 19:57 ` [PATCH 5/5] man: describe the metadata scrubbing ioctl Darrick J. Wong
  4 siblings, 3 replies; 26+ messages in thread
From: Darrick J. Wong @ 2017-11-17 19:57 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a new xfs_io command to call the new XFS metadata scrub ioctl.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 io/Makefile       |    3 -
 io/init.c         |    1 
 io/io.h           |    2 
 io/scrub.c        |  244 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 man/man8/xfs_io.8 |   10 ++
 5 files changed, 259 insertions(+), 1 deletion(-)
 create mode 100644 io/scrub.c


diff --git a/io/Makefile b/io/Makefile
index 050d6bd..b983bcc 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -11,7 +11,8 @@ HFILES = init.h io.h
 CFILES = init.c \
 	attr.c bmap.c cowextsize.c encrypt.c file.c freeze.c fsync.c \
 	getrusage.c imap.c link.c mmap.c open.c parent.c pread.c prealloc.c \
-	pwrite.c reflink.c seek.c shutdown.c stat.c sync.c truncate.c utimes.c
+	pwrite.c reflink.c scrub.c seek.c shutdown.c stat.c sync.c truncate.c \
+	utimes.c
 
 LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD)
 LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE)
diff --git a/io/init.c b/io/init.c
index 20d5f80..9e576fe 100644
--- a/io/init.c
+++ b/io/init.c
@@ -84,6 +84,7 @@ init_commands(void)
 	readdir_init();
 	reflink_init();
 	resblks_init();
+	scrub_init();
 	seek_init();
 	sendfile_init();
 	shutdown_init();
diff --git a/io/io.h b/io/io.h
index 3862985..82e142f 100644
--- a/io/io.h
+++ b/io/io.h
@@ -185,3 +185,5 @@ extern void		fsmap_init(void);
 #else
 # define fsmap_init()	do { } while (0)
 #endif
+
+extern void		scrub_init(void);
diff --git a/io/scrub.c b/io/scrub.c
new file mode 100644
index 0000000..cd2a1ee
--- /dev/null
+++ b/io/scrub.c
@@ -0,0 +1,244 @@
+/*
+ * Copyright (C) 2017 Oracle.  All Rights Reserved.
+ *
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ *
+ * 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,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include <sys/uio.h>
+#include <xfs/xfs.h>
+#include "command.h"
+#include "input.h"
+#include "init.h"
+#include "path.h"
+#include "io.h"
+
+static struct cmdinfo scrub_cmd;
+
+/* Type info and names for the scrub types. */
+enum scrub_type {
+	ST_NONE,	/* disabled */
+	ST_PERAG,	/* per-AG metadata */
+	ST_FS,		/* per-FS metadata */
+	ST_INODE,	/* per-inode metadata */
+};
+
+struct scrub_descr {
+	const char	*name;
+	enum scrub_type	type;
+};
+
+static const struct scrub_descr scrubbers[XFS_SCRUB_TYPE_NR] = {
+	[XFS_SCRUB_TYPE_PROBE]		= {"probe",		ST_NONE},
+	[XFS_SCRUB_TYPE_SB]		= {"sb",		ST_PERAG},
+	[XFS_SCRUB_TYPE_AGF]		= {"agf",		ST_PERAG},
+	[XFS_SCRUB_TYPE_AGFL]		= {"agfl",		ST_PERAG},
+	[XFS_SCRUB_TYPE_AGI]		= {"agi",		ST_PERAG},
+	[XFS_SCRUB_TYPE_BNOBT]		= {"bnobt",		ST_PERAG},
+	[XFS_SCRUB_TYPE_CNTBT]		= {"cntbt",		ST_PERAG},
+	[XFS_SCRUB_TYPE_INOBT]		= {"inobt",		ST_PERAG},
+	[XFS_SCRUB_TYPE_FINOBT]		= {"finobt",		ST_PERAG},
+	[XFS_SCRUB_TYPE_RMAPBT]		= {"rmapbt",		ST_PERAG},
+	[XFS_SCRUB_TYPE_REFCNTBT]	= {"refcountbt",	ST_PERAG},
+	[XFS_SCRUB_TYPE_INODE]		= {"inode",		ST_INODE},
+	[XFS_SCRUB_TYPE_BMBTD]		= {"bmapbtd",		ST_INODE},
+	[XFS_SCRUB_TYPE_BMBTA]		= {"bmapbta",		ST_INODE},
+	[XFS_SCRUB_TYPE_BMBTC]		= {"bmapbtc",		ST_INODE},
+	[XFS_SCRUB_TYPE_DIR]		= {"directory",		ST_INODE},
+	[XFS_SCRUB_TYPE_XATTR]		= {"xattr",		ST_INODE},
+	[XFS_SCRUB_TYPE_SYMLINK]	= {"symlink",		ST_INODE},
+	[XFS_SCRUB_TYPE_PARENT]		= {"parent",		ST_INODE},
+	[XFS_SCRUB_TYPE_RTBITMAP]	= {"rtbitmap",		ST_FS},
+	[XFS_SCRUB_TYPE_RTSUM]		= {"rtsummary",		ST_FS},
+	[XFS_SCRUB_TYPE_UQUOTA]		= {"usrquota",		ST_FS},
+	[XFS_SCRUB_TYPE_GQUOTA]		= {"grpquota",		ST_FS},
+	[XFS_SCRUB_TYPE_PQUOTA]		= {"prjquota",		ST_FS},
+};
+
+static void
+scrub_help(void)
+{
+	const struct scrub_descr	*d;
+	int				i;
+
+	printf(_("\n\
+ Scrubs a piece of XFS filesystem metadata.  The first argument is the type\n\
+ of metadata to examine.  Allocation group number(s) can be specified to\n\
+ restrict the scrub operation to a subset of allocation groups.\n\
+ Certain metadata types do not take AG numbers.\n\
+\n\
+ Example:\n\
+ 'scrub inobt 3' - scrub the inode btree in AG 3.\n\
+ 'scrub bmapbtd 128 13525' - scrubs the extent map of inode 128 gen 13525.\n\
+\n\
+ Known metadata scrub types are:"));
+	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++)
+		printf(" %s", d->name);
+	printf("\n");
+}
+
+static void
+scrub_ioctl(
+	int				fd,
+	int				type,
+	uint64_t			control,
+	uint32_t			control2)
+{
+	struct xfs_scrub_metadata	meta;
+	const struct scrub_descr	*sc;
+	int				error;
+
+	sc = &scrubbers[type];
+	memset(&meta, 0, sizeof(meta));
+	meta.sm_type = type;
+	switch (sc->type) {
+	case ST_PERAG:
+		meta.sm_agno = control;
+		break;
+	case ST_INODE:
+		meta.sm_ino = control;
+		meta.sm_gen = control2;
+		break;
+	case ST_NONE:
+	case ST_FS:
+		/* no control parameters */
+		break;
+	}
+	meta.sm_flags = 0;
+
+	error = ioctl(fd, XFS_IOC_SCRUB_METADATA, &meta);
+	if (error)
+		perror("scrub");
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
+		printf(_("Corruption detected.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_PREEN)
+		printf(_("Optimization possible.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_XFAIL)
+		printf(_("Cross-referencing failed.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_XCORRUPT)
+		printf(_("Corruption detected during cross-referencing.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_INCOMPLETE)
+		printf(_("Scan was not complete.\n"));
+}
+
+static int
+parse_args(
+	int				argc,
+	char				**argv,
+	struct cmdinfo			*cmdinfo,
+	void				(*fn)(int, int, uint64_t, uint32_t))
+{
+	char				*p;
+	int				type = -1;
+	int				i, c;
+	uint64_t			control = 0;
+	uint32_t			control2 = 0;
+	const struct scrub_descr	*d = NULL;
+
+	while ((c = getopt(argc, argv, "")) != EOF) {
+		switch (c) {
+		default:
+			return command_usage(cmdinfo);
+		}
+	}
+	if (optind > argc - 1)
+		return command_usage(cmdinfo);
+
+	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++) {
+		if (strcmp(d->name, argv[optind]) == 0) {
+			type = i;
+			break;
+		}
+	}
+	optind++;
+
+	if (type < 0)
+		return command_usage(cmdinfo);
+
+	switch (d->type) {
+	case ST_INODE:
+		if (optind == argc) {
+			control = 0;
+			control2 = 0;
+		} else if (optind == argc - 2) {
+			control = strtoull(argv[optind], &p, 0);
+			if (*p != '\0') {
+				fprintf(stderr,
+					_("Bad inode number %s.\n"), argv[i]);
+				return 0;
+			}
+			control2 = strtoul(argv[optind + 1], &p, 0);
+			if (*p != '\0') {
+				fprintf(stderr,
+					_("Bad generation number %s.\n"), argv[i]);
+				return 0;
+			}
+		} else {
+			fprintf(stderr,
+				_("Must specify inode number and generation.\n"));
+			return 0;
+		}
+		break;
+	case ST_PERAG:
+	case ST_NONE:
+		if (optind != argc - 1) {
+			fprintf(stderr,
+				_("Must specify AG number.\n"));
+			return 0;
+		}
+		control = strtoul(argv[optind], &p, 0);
+		if (*p != '\0') {
+			fprintf(stderr,
+				_("Bad AG number %s.\n"), argv[i]);
+			return 0;
+		}
+		break;
+	default:
+		if (optind != argc) {
+			fprintf(stderr,
+				_("No parameters allowed.\n"));
+			return 0;
+		}
+	}
+	fn(file->fd, type, control, control2);
+
+	return 0;
+}
+
+static int
+scrub_f(
+	int				argc,
+	char				**argv)
+{
+	return parse_args(argc, argv, &scrub_cmd, scrub_ioctl);
+}
+
+void
+scrub_init(void)
+{
+	scrub_cmd.name = "scrub";
+	scrub_cmd.altname = "sc";
+	scrub_cmd.cfunc = scrub_f;
+	scrub_cmd.argmin = 1;
+	scrub_cmd.argmax = -1;
+	scrub_cmd.flags = CMD_NOMAP_OK;
+	scrub_cmd.args =
+_("type [agno...]");
+	scrub_cmd.oneline =
+		_("scrubs filesystem metadata");
+	scrub_cmd.help = scrub_help;
+
+	add_command(&scrub_cmd);
+}
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 9bf1a47..ed10ba6 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1119,6 +1119,16 @@ version of policy structure (numeric)
 .BR get_encpolicy
 On filesystems that support encryption, display the encryption policy of the
 current file.
+.TP
+.BI "scrub " type " [ " agnumber... " | " "ino" " " "gen" " ]"
+Scrub internal XFS filesystem metadata.  The
+.BI type
+parameter specifies which type of metadata to scrub.
+For AG metadata, AG numbers can optionally be specified to restrict the
+scrub operation to a particular set of allocation groups.
+By default, all allocation groups are scrubbed.
+For file metadata, the scrub is applied to the open file unless the
+inode number and generation number are specified.
 
 .SH SEE ALSO
 .BR mkfs.xfs (8),


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 5/5] man: describe the metadata scrubbing ioctl
  2017-11-17 19:56 [PATCH 0/5] xfsprogs: 4.15 rollup Darrick J. Wong
                   ` (3 preceding siblings ...)
  2017-11-17 19:57 ` [PATCH 4/5] xfs_io: provide an interface to the scrub ioctls Darrick J. Wong
@ 2017-11-17 19:57 ` Darrick J. Wong
  2017-12-01  4:34   ` Eric Sandeen
  2017-12-01 19:03   ` [PATCH v2 " Darrick J. Wong
  4 siblings, 2 replies; 26+ messages in thread
From: Darrick J. Wong @ 2017-11-17 19:57 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Document the XFS-specific metadata scrub/repair ioctl's behavior,
arguments, and side effects.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 man/Makefile                        |    2 
 man/man2/Makefile                   |   22 +++
 man/man2/ioctl_xfs_scrub_metadata.2 |  298 +++++++++++++++++++++++++++++++++++
 3 files changed, 321 insertions(+), 1 deletion(-)
 create mode 100644 man/man2/Makefile
 create mode 100644 man/man2/ioctl_xfs_scrub_metadata.2


diff --git a/man/Makefile b/man/Makefile
index 863284c..cae891f 100644
--- a/man/Makefile
+++ b/man/Makefile
@@ -5,7 +5,7 @@
 TOPDIR = ..
 include $(TOPDIR)/include/builddefs
 
-SUBDIRS = man3 man5 man8
+SUBDIRS = man2 man3 man5 man8
 
 default : $(SUBDIRS)
 
diff --git a/man/man2/Makefile b/man/man2/Makefile
new file mode 100644
index 0000000..8aecde3
--- /dev/null
+++ b/man/man2/Makefile
@@ -0,0 +1,22 @@
+#
+# Copyright (c) 2000-2001 Silicon Graphics, Inc.  All Rights Reserved.
+#
+
+TOPDIR = ../..
+include $(TOPDIR)/include/builddefs
+
+MAN_SECTION	= 2
+
+MAN_PAGES	= $(shell echo *.$(MAN_SECTION))
+MAN_DEST	= $(PKG_MAN_DIR)/man$(MAN_SECTION)
+LSRCFILES	= $(MAN_PAGES)
+
+default : $(MAN_PAGES)
+
+include $(BUILDRULES)
+
+install :
+
+install-dev : default
+	$(INSTALL) -m 755 -d $(MAN_DEST)
+	$(INSTALL_MAN)
diff --git a/man/man2/ioctl_xfs_scrub_metadata.2 b/man/man2/ioctl_xfs_scrub_metadata.2
new file mode 100644
index 0000000..fa1c56d
--- /dev/null
+++ b/man/man2/ioctl_xfs_scrub_metadata.2
@@ -0,0 +1,298 @@
+.\" Copyright (c) 2017, Oracle.  All rights reserved.
+.\"
+.\" %%%LICENSE_START(GPLv2+_DOC_FULL)
+.\" This is free documentation; 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.
+.\"
+.\" The GNU General Public License's references to "object code"
+.\" and "executables" are to be interpreted as the output of any
+.\" document formatting or typesetting system, including
+.\" intermediate and printed output.
+.\"
+.\" This manual is distributed in the hope that it will 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 manual; if not, see
+.\" <http://www.gnu.org/licenses/>.
+.\" %%%LICENSE_END
+.TH IOCTL-XFS-SCRUB-METADATA 2 2017-09-21 "Linux" "Linux Programmer's Manual"
+.SH NAME
+ioctl_xfs_scrub_metadata \- check XFS filesystem metadata
+.SH SYNOPSIS
+.br
+.B #include <xfs/xfs_fs.h>
+.PP
+.BI "int ioctl(int " dest_fd ", XFS_IOC_SCRUB_METADATA, struct xfs_scrub_metadata *" arg );
+.SH DESCRIPTION
+This XFS ioctl asks the kernel driver to examine a piece of metadata for
+errors or suboptimal metadata.
+Examination includes running the metadata verifiers, checking records
+for obviously incorrect or impossible values, and cross-referencing each
+record with any other available metadata in the filesystem.
+This ioctl can also try to repair or optimize metadata, though this may
+tie up the filesystem for a long period of time.
+The type and location of the metadata is conveyed in a structure of the
+following form:
+.PP
+.in +4n
+.EX
+struct xfs_scrub_metadata {
+	__u32 sm_type;
+	__u32 sm_flags;
+	__u64 sm_ino;
+	__u32 sm_gen;
+	__u32 sm_agno;
+	__u64 sm_reserved[5];
+};
+.EE
+.in
+.PP
+The field
+.I sm_reserved
+must be zero.
+.PP
+The field
+.I sm_type
+indicates the type of metadata to check:
+.RS 0.4i
+.TP
+.B XFS_SCRUB_TYPE_PROBE
+Probe the kernel to see if it is willing to try to check or repair this
+filesystem.
+If any
+.B sm_flags
+output flags are set in
+.BR sm_gen ", "
+they will be copied to
+.B sm_flags
+before the call returns.
+
+.PD 0
+.PP
+.nf
+.B XFS_SCRUB_TYPE_SB
+.B XFS_SCRUB_TYPE_AGF
+.B XFS_SCRUB_TYPE_AGFL
+.fi
+.TP
+.B XFS_SCRUB_TYPE_AGI
+Examine a given allocation group's superblock, free space header, free
+block list, or inode header, respectively.
+Headers are checked for obviously incorrect values and cross-referenced
+against the allocation group's metadata btrees, if possible.
+The allocation group number must be given in
+.BR sm_agno "."
+
+.PP
+.nf
+.B XFS_SCRUB_TYPE_BNOBT
+.B XFS_SCRUB_TYPE_CNTBT
+.B XFS_SCRUB_TYPE_INOBT
+.B XFS_SCRUB_TYPE_FINOBT
+.B XFS_SCRUB_TYPE_RMAPBT
+.fi
+.TP
+.B XFS_SCRUB_TYPE_REFCNTBT
+Examine a given allocation group's free space btrees, inode btress, reverse
+mapping btrees, or reference count btrees, respectively.
+against the allocation group's metadata btrees, if possible.
+Space extent records are checked for obviously incorrect values and
+cross-referenced with the other space extent metadata to ensure that
+there are no conflicts.
+The allocation group number must be given in
+.BR sm_agno "."
+
+.TP
+.B XFS_SCRUB_TYPE_INODE
+Examine a given inode's inode record for obviously incorrect values and
+discrepancies with the rest of filesystem metadata.
+Parent pointers are checked for impossible inode values and are then
+followed up to the parent directory to ensure that the linkage makes
+sense.
+The inode to examine can be specified either through
+.B sm_ino
+and
+.BR sm_gen "; "
+if not specified, then the file described by
+.B dest_fd
+will be examined.
+
+.PP
+.nf
+.B XFS_SCRUB_TYPE_BMBTD
+.B XFS_SCRUB_TYPE_BMBTA
+.fi
+.TP
+.B XFS_SCRUB_TYPE_BMBTC
+Examine a given inode's data block map, extended attribute block map,
+copy on write block map, or parent inode pointer.
+Inode records are examined for obviously incorrect values and
+discrepancies with the three block map types.
+The block maps are checked for obviously wrong values and
+cross-referenced with the allocation group space extent metadata for
+discrepancies.
+The inode to examine can be specified in the same manner as
+.BR XFS_SCRUB_TYPE_INODE "."
+
+.TP
+.B XFS_SCRUB_TYPE_XATTR
+Examine the extended attribute records and indices of a given inode for
+incorrect pointers and other signs of damage.
+The inode to examine can be specified in the same manner as
+.BR XFS_SCRUB_TYPE_INODE "."
+
+.TP
+.B XFS_SCRUB_TYPE_DIR
+Examine the entries in a given directory for invalid data or dangling pointers.
+The directory to examine can be specified in the same manner as
+.BR XFS_SCRUB_TYPE_INODE "."
+
+.TP
+.B XFS_SCRUB_TYPE_SYMLINK
+Examine the target of a symbolic link for obvious pathname problems.
+The link to examine can be specified in the same manner as
+.BR XFS_SCRUB_TYPE_INODE "."
+
+.PP
+.nf
+.B XFS_SCRUB_TYPE_RTBITMAP
+.fi
+.TP
+.B XFS_SCRUB_TYPE_RTSUM
+Examine the realtime block bitmap and realtime summary inodes for
+corruption.
+
+.PP
+.nf
+.B XFS_SCRUB_TYPE_UQUOTA
+.B XFS_SCRUB_TYPE_GQUOTA
+.fi
+.TP
+.B XFS_SCRUB_TYPE_PQUOTA
+Examine all user, group, or project quota records for corruption.
+.RE
+
+.PD 1
+.PP
+The field
+.I sm_flags
+control the behavior of the scrub operation and provide more information
+about the outcome of the operation.
+If none of the
+.B XFS_SCRUB_OFLAG_*
+flags are set upon return, the metadata is clean.
+.RS 0.4i
+.TP
+.B XFS_SCRUB_IFLAG_REPAIR
+If the caller sets this flag, the checker will examine the metadata and
+try to fix any problems or suboptimal metadata that it finds.
+If no errors occur during the repair operation, the check is performed a
+second time to determine if the repair succeeded.
+If errors do occur, the call returns an error status immediately.
+.TP
+.B XFS_SCRUB_OFLAG_CORRUPT
+The metadata was corrupt when the call returned.
+If
+.B XFS_SCRUB_IFLAG_REPAIR
+was specified, then an attempted repair failed to fix the problem.
+Unmount the filesystem and run
+.B xfs_repair
+to fix the filesystem.
+.TP
+.B XFS_SCRUB_OFLAG_PREEN
+The metadata is ok, but some aspect of the metadata could be optimized
+to increase performance.
+.TP
+.B XFS_SCRUB_OFLAG_XFAIL
+Filesystem errors were encountered when accessing other metadata to
+cross-reference the records attached to this metadata object.
+.TP
+.B XFS_SCRUB_OFLAG_XCORRUPT
+Discrepancies were found when cross-referencing the records attached to
+this metadata object against all other available metadata in the system.
+.TP
+.B XFS_SCRUB_OFLAG_INCOMPLETE
+The checker was unable to complete its check of all records.
+.TP
+.B XFS_SCRUB_OFLAG_WARNING
+The checker encountered a metadata object with potentially problematic
+records.
+However, the records were not obviously corrupt.
+.RE
+.PP
+For metadata checkers that operate on inodes or inode metadata, the fields
+.IR sm_ino " and " sm_gen
+are the inode number and generation number of the inode to check.
+If the inode number is zero, the inode represented by
+.I dest_fd
+is used instead.
+.PP
+For metadata checkers that operate on allocation group metadata, the field
+.I sm_agno
+indicates the allocation group in which to find the metadata.
+.PP
+For metadata checkers that operate on filesystem-wide metadata, no
+further arguments are required.
+.SH RETURN VALUE
+On error, \-1 is returned, and
+.I errno
+is set to indicate the error.
+.PP
+.SH ERRORS
+Error codes can be one of, but are not limited to, the following:
+.TP
+.B EBUSY
+The filesystem object is busy; the repair will have to be tried again.
+.TP
+.B EFSCORRUPTED
+Severe filesystem corruption was detected and could not be repaired.
+Unmount the filesystem and run
+.B xfs_repair
+to fix the filesystem.
+.TP
+.B EINVAL
+One or more of the arguments specified is invalid.
+.TP
+.B ENOENT
+The specified metadata object does not exist.
+For example, this error code is returned for a
+.B XFS_SCRUB_TYPE_REFCNTBT
+request on a filesystem that does not support reflink.
+.TP
+.B ENOMEM
+There was not sufficient memory to perform the scrub or repair operation.
+Some operations (most notably reference count checking) require a lot of
+memory.
+.TP
+.B ENOSPC
+There is not enough free disk space to attempt a repair.
+.TP
+.B ENOTRECOVERABLE
+Filesystem was mounted in
+.B norecovery
+mode and therefore has an unclean log.
+.TP
+.B ENOTTY
+Online scrubbing or repair were not enabled.
+.TP
+.B EOPNOTSUPP
+Repairs of the requested metadata object are not supported.
+.TP
+.B EROFS
+Filesystem is read-only and a repair was requested.
+.TP
+.B ESHUTDOWN;
+Filesystem is shut down due to previous errors.
+.SH CONFORMING TO
+This API is specific to XFS on Linux.
+.SH NOTES
+These operations may tie up the filesystem for a long time.
+A calling process can be stop the operation by being sent a fatal
+signal, but non-fatal signals are blocked.
+.SH SEE ALSO
+.BR ioctl (2)


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/5] xfs_db: print structure padding fields consistently
  2017-11-17 19:56 ` [PATCH 1/5] xfs_db: print structure padding fields consistently Darrick J. Wong
@ 2017-11-30 21:37   ` Eric Sandeen
  2017-11-30 21:51     ` Darrick J. Wong
  2017-12-04 23:54   ` [PATCH 1/5 V2] " Eric Sandeen
  1 sibling, 1 reply; 26+ messages in thread
From: Eric Sandeen @ 2017-11-30 21:37 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs

On 11/17/17 1:56 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> We are very inconsistent about how we print padding fields in on-disk
> structures -- sometimes we hide it from printall, sometimes we deviate
> from unsigned hex values, etc.  Make this all consistent -- never hide
> padding values, always print them as unsigned hex integers.

I'm not sure I see the point to cluttering up structure printing
with padding, and would prefer to hide them /all/ by default (and
set them all to hex printing when explicitly asked for).

Consistency is good but I'm not sure we need to consistently print
values that are never even used...?

-Eric

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  db/attr.c  |   10 +++++-----
>  db/dir2.c  |    4 ++--
>  db/dquot.c |    4 ++--
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/db/attr.c b/db/attr.c
> index 75fe239..ba8313c 100644
> --- a/db/attr.c
> +++ b/db/attr.c
> @@ -73,7 +73,7 @@ const field_t	attr_blkinfo_flds[] = {
>  	{ "forw", FLDT_ATTRBLOCK, OI(BOFF(forw)), C1, 0, TYP_ATTR },
>  	{ "back", FLDT_ATTRBLOCK, OI(BOFF(back)), C1, 0, TYP_ATTR },
>  	{ "magic", FLDT_UINT16X, OI(BOFF(magic)), C1, 0, TYP_NONE },
> -	{ "pad", FLDT_UINT16X, OI(BOFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
> +	{ "pad", FLDT_UINT16X, OI(BOFF(pad)), C1, 0, TYP_NONE },
>  	{ NULL }
>  };
>  
> @@ -94,7 +94,7 @@ const field_t	attr_leaf_entry_flds[] = {
>  	{ "local", FLDT_UINT1,
>  	  OI(LEOFF(flags) + bitsz(uint8_t) - XFS_ATTR_LOCAL_BIT - 1), C1, 0,
>  	  TYP_NONE },
> -	{ "pad2", FLDT_UINT8X, OI(LEOFF(pad2)), C1, FLD_SKIPALL, TYP_NONE },
> +	{ "pad2", FLDT_UINT8X, OI(LEOFF(pad2)), C1, 0, TYP_NONE },
>  	{ NULL }
>  };
>  
> @@ -105,7 +105,7 @@ const field_t	attr_leaf_hdr_flds[] = {
>  	{ "usedbytes", FLDT_UINT16D, OI(LHOFF(usedbytes)), C1, 0, TYP_NONE },
>  	{ "firstused", FLDT_UINT16D, OI(LHOFF(firstused)), C1, 0, TYP_NONE },
>  	{ "holes", FLDT_UINT8D, OI(LHOFF(holes)), C1, 0, TYP_NONE },
> -	{ "pad1", FLDT_UINT8X, OI(LHOFF(pad1)), C1, FLD_SKIPALL, TYP_NONE },
> +	{ "pad1", FLDT_UINT8X, OI(LHOFF(pad1)), C1, 0, TYP_NONE },
>  	{ "freemap", FLDT_ATTR_LEAF_MAP, OI(LHOFF(freemap)),
>  	  CI(XFS_ATTR_LEAF_MAPSIZE), FLD_ARRAY, TYP_NONE },
>  	{ NULL }
> @@ -567,7 +567,7 @@ const field_t	attr3_leaf_hdr_flds[] = {
>  	{ "usedbytes", FLDT_UINT16D, OI(LH3OFF(usedbytes)), C1, 0, TYP_NONE },
>  	{ "firstused", FLDT_UINT16D, OI(LH3OFF(firstused)), C1, 0, TYP_NONE },
>  	{ "holes", FLDT_UINT8D, OI(LH3OFF(holes)), C1, 0, TYP_NONE },
> -	{ "pad1", FLDT_UINT8X, OI(LH3OFF(pad1)), C1, FLD_SKIPALL, TYP_NONE },
> +	{ "pad1", FLDT_UINT8X, OI(LH3OFF(pad1)), C1, 0, TYP_NONE },
>  	{ "freemap", FLDT_ATTR_LEAF_MAP, OI(LH3OFF(freemap)),
>  	  CI(XFS_ATTR_LEAF_MAPSIZE), FLD_ARRAY, TYP_NONE },
>  	{ NULL }
> @@ -589,7 +589,7 @@ const field_t	attr3_node_hdr_flds[] = {
>  	{ "info", FLDT_ATTR3_BLKINFO, OI(H3OFF(info)), C1, 0, TYP_NONE },
>  	{ "count", FLDT_UINT16D, OI(H3OFF(__count)), C1, 0, TYP_NONE },
>  	{ "level", FLDT_UINT16D, OI(H3OFF(__level)), C1, 0, TYP_NONE },
> -	{ "pad", FLDT_UINT32D, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
> +	{ "pad", FLDT_UINT32X, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
>  	{ NULL }
>  };
>  
> diff --git a/db/dir2.c b/db/dir2.c
> index 3e21a7b..bd9c6aa 100644
> --- a/db/dir2.c
> +++ b/db/dir2.c
> @@ -171,7 +171,7 @@ const field_t	da_blkinfo_flds[] = {
>  	{ "forw", FLDT_DIRBLOCK, OI(DBOFF(forw)), C1, 0, TYP_INODATA },
>  	{ "back", FLDT_DIRBLOCK, OI(DBOFF(back)), C1, 0, TYP_INODATA },
>  	{ "magic", FLDT_UINT16X, OI(DBOFF(magic)), C1, 0, TYP_NONE },
> -	{ "pad", FLDT_UINT16X, OI(DBOFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
> +	{ "pad", FLDT_UINT16X, OI(DBOFF(pad)), C1, 0, TYP_NONE },
>  	{ NULL }
>  };
>  
> @@ -977,7 +977,7 @@ const field_t	da3_node_hdr_flds[] = {
>  	{ "info", FLDT_DA3_BLKINFO, OI(H3OFF(info)), C1, 0, TYP_NONE },
>  	{ "count", FLDT_UINT16D, OI(H3OFF(__count)), C1, 0, TYP_NONE },
>  	{ "level", FLDT_UINT16D, OI(H3OFF(__level)), C1, 0, TYP_NONE },
> -	{ "pad", FLDT_UINT32D, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
> +	{ "pad", FLDT_UINT32X, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
>  	{ NULL }
>  };
>  
> diff --git a/db/dquot.c b/db/dquot.c
> index 4e35df4..222c082 100644
> --- a/db/dquot.c
> +++ b/db/dquot.c
> @@ -76,7 +76,7 @@ const field_t	disk_dquot_flds[] = {
>  	{ "btimer", FLDT_INT32D, OI(DOFF(btimer)), C1, 0, TYP_NONE },
>  	{ "iwarns", FLDT_QWARNCNT, OI(DOFF(iwarns)), C1, 0, TYP_NONE },
>  	{ "bwarns", FLDT_QWARNCNT, OI(DOFF(bwarns)), C1, 0, TYP_NONE },
> -	{ "pad0", FLDT_INT32D, OI(DOFF(pad0)), C1, FLD_SKIPALL, TYP_NONE },
> +	{ "pad0", FLDT_UINT32X, OI(DOFF(pad0)), C1, 0, TYP_NONE },
>  	{ "rtb_hardlimit", FLDT_QCNT, OI(DOFF(rtb_hardlimit)), C1, 0,
>  	  TYP_NONE },
>  	{ "rtb_softlimit", FLDT_QCNT, OI(DOFF(rtb_softlimit)), C1, 0,
> @@ -84,7 +84,7 @@ const field_t	disk_dquot_flds[] = {
>  	{ "rtbcount", FLDT_QCNT, OI(DOFF(rtbcount)), C1, 0, TYP_NONE },
>  	{ "rtbtimer", FLDT_INT32D, OI(DOFF(rtbtimer)), C1, 0, TYP_NONE },
>  	{ "rtbwarns", FLDT_QWARNCNT, OI(DOFF(rtbwarns)), C1, 0, TYP_NONE },
> -	{ "pad", FLDT_UINT16X, OI(DOFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
> +	{ "pad", FLDT_UINT16X, OI(DOFF(pad)), C1, 0, TYP_NONE },
>  	{ NULL }
>  };
>  
> 
> --
> 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
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/5] xfs_io: pull xfs errortag definitions from libxfs
  2017-11-17 19:56 ` [PATCH 3/5] xfs_io: pull xfs errortag definitions from libxfs Darrick J. Wong
@ 2017-11-30 21:40   ` Eric Sandeen
  0 siblings, 0 replies; 26+ messages in thread
From: Eric Sandeen @ 2017-11-30 21:40 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs

On 11/17/17 1:56 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use the libxfs definitions, don't provide our own.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Yay

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/5] xfs_db: print structure padding fields consistently
  2017-11-30 21:37   ` Eric Sandeen
@ 2017-11-30 21:51     ` Darrick J. Wong
  2017-11-30 21:57       ` Eric Sandeen
  0 siblings, 1 reply; 26+ messages in thread
From: Darrick J. Wong @ 2017-11-30 21:51 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: sandeen, linux-xfs

On Thu, Nov 30, 2017 at 03:37:54PM -0600, Eric Sandeen wrote:
> On 11/17/17 1:56 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > We are very inconsistent about how we print padding fields in on-disk
> > structures -- sometimes we hide it from printall, sometimes we deviate
> > from unsigned hex values, etc.  Make this all consistent -- never hide
> > padding values, always print them as unsigned hex integers.
> 
> I'm not sure I see the point to cluttering up structure printing
> with padding, and would prefer to hide them /all/ by default (and
> set them all to hex printing when explicitly asked for).
> 
> Consistency is good but I'm not sure we need to consistently print
> values that are never even used...?

Ok.  Will you take a patch that changes them all to FLD_SKIPALL?

--D

> -Eric
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  db/attr.c  |   10 +++++-----
> >  db/dir2.c  |    4 ++--
> >  db/dquot.c |    4 ++--
> >  3 files changed, 9 insertions(+), 9 deletions(-)
> > 
> > 
> > diff --git a/db/attr.c b/db/attr.c
> > index 75fe239..ba8313c 100644
> > --- a/db/attr.c
> > +++ b/db/attr.c
> > @@ -73,7 +73,7 @@ const field_t	attr_blkinfo_flds[] = {
> >  	{ "forw", FLDT_ATTRBLOCK, OI(BOFF(forw)), C1, 0, TYP_ATTR },
> >  	{ "back", FLDT_ATTRBLOCK, OI(BOFF(back)), C1, 0, TYP_ATTR },
> >  	{ "magic", FLDT_UINT16X, OI(BOFF(magic)), C1, 0, TYP_NONE },
> > -	{ "pad", FLDT_UINT16X, OI(BOFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
> > +	{ "pad", FLDT_UINT16X, OI(BOFF(pad)), C1, 0, TYP_NONE },
> >  	{ NULL }
> >  };
> >  
> > @@ -94,7 +94,7 @@ const field_t	attr_leaf_entry_flds[] = {
> >  	{ "local", FLDT_UINT1,
> >  	  OI(LEOFF(flags) + bitsz(uint8_t) - XFS_ATTR_LOCAL_BIT - 1), C1, 0,
> >  	  TYP_NONE },
> > -	{ "pad2", FLDT_UINT8X, OI(LEOFF(pad2)), C1, FLD_SKIPALL, TYP_NONE },
> > +	{ "pad2", FLDT_UINT8X, OI(LEOFF(pad2)), C1, 0, TYP_NONE },
> >  	{ NULL }
> >  };
> >  
> > @@ -105,7 +105,7 @@ const field_t	attr_leaf_hdr_flds[] = {
> >  	{ "usedbytes", FLDT_UINT16D, OI(LHOFF(usedbytes)), C1, 0, TYP_NONE },
> >  	{ "firstused", FLDT_UINT16D, OI(LHOFF(firstused)), C1, 0, TYP_NONE },
> >  	{ "holes", FLDT_UINT8D, OI(LHOFF(holes)), C1, 0, TYP_NONE },
> > -	{ "pad1", FLDT_UINT8X, OI(LHOFF(pad1)), C1, FLD_SKIPALL, TYP_NONE },
> > +	{ "pad1", FLDT_UINT8X, OI(LHOFF(pad1)), C1, 0, TYP_NONE },
> >  	{ "freemap", FLDT_ATTR_LEAF_MAP, OI(LHOFF(freemap)),
> >  	  CI(XFS_ATTR_LEAF_MAPSIZE), FLD_ARRAY, TYP_NONE },
> >  	{ NULL }
> > @@ -567,7 +567,7 @@ const field_t	attr3_leaf_hdr_flds[] = {
> >  	{ "usedbytes", FLDT_UINT16D, OI(LH3OFF(usedbytes)), C1, 0, TYP_NONE },
> >  	{ "firstused", FLDT_UINT16D, OI(LH3OFF(firstused)), C1, 0, TYP_NONE },
> >  	{ "holes", FLDT_UINT8D, OI(LH3OFF(holes)), C1, 0, TYP_NONE },
> > -	{ "pad1", FLDT_UINT8X, OI(LH3OFF(pad1)), C1, FLD_SKIPALL, TYP_NONE },
> > +	{ "pad1", FLDT_UINT8X, OI(LH3OFF(pad1)), C1, 0, TYP_NONE },
> >  	{ "freemap", FLDT_ATTR_LEAF_MAP, OI(LH3OFF(freemap)),
> >  	  CI(XFS_ATTR_LEAF_MAPSIZE), FLD_ARRAY, TYP_NONE },
> >  	{ NULL }
> > @@ -589,7 +589,7 @@ const field_t	attr3_node_hdr_flds[] = {
> >  	{ "info", FLDT_ATTR3_BLKINFO, OI(H3OFF(info)), C1, 0, TYP_NONE },
> >  	{ "count", FLDT_UINT16D, OI(H3OFF(__count)), C1, 0, TYP_NONE },
> >  	{ "level", FLDT_UINT16D, OI(H3OFF(__level)), C1, 0, TYP_NONE },
> > -	{ "pad", FLDT_UINT32D, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
> > +	{ "pad", FLDT_UINT32X, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
> >  	{ NULL }
> >  };
> >  
> > diff --git a/db/dir2.c b/db/dir2.c
> > index 3e21a7b..bd9c6aa 100644
> > --- a/db/dir2.c
> > +++ b/db/dir2.c
> > @@ -171,7 +171,7 @@ const field_t	da_blkinfo_flds[] = {
> >  	{ "forw", FLDT_DIRBLOCK, OI(DBOFF(forw)), C1, 0, TYP_INODATA },
> >  	{ "back", FLDT_DIRBLOCK, OI(DBOFF(back)), C1, 0, TYP_INODATA },
> >  	{ "magic", FLDT_UINT16X, OI(DBOFF(magic)), C1, 0, TYP_NONE },
> > -	{ "pad", FLDT_UINT16X, OI(DBOFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
> > +	{ "pad", FLDT_UINT16X, OI(DBOFF(pad)), C1, 0, TYP_NONE },
> >  	{ NULL }
> >  };
> >  
> > @@ -977,7 +977,7 @@ const field_t	da3_node_hdr_flds[] = {
> >  	{ "info", FLDT_DA3_BLKINFO, OI(H3OFF(info)), C1, 0, TYP_NONE },
> >  	{ "count", FLDT_UINT16D, OI(H3OFF(__count)), C1, 0, TYP_NONE },
> >  	{ "level", FLDT_UINT16D, OI(H3OFF(__level)), C1, 0, TYP_NONE },
> > -	{ "pad", FLDT_UINT32D, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
> > +	{ "pad", FLDT_UINT32X, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
> >  	{ NULL }
> >  };
> >  
> > diff --git a/db/dquot.c b/db/dquot.c
> > index 4e35df4..222c082 100644
> > --- a/db/dquot.c
> > +++ b/db/dquot.c
> > @@ -76,7 +76,7 @@ const field_t	disk_dquot_flds[] = {
> >  	{ "btimer", FLDT_INT32D, OI(DOFF(btimer)), C1, 0, TYP_NONE },
> >  	{ "iwarns", FLDT_QWARNCNT, OI(DOFF(iwarns)), C1, 0, TYP_NONE },
> >  	{ "bwarns", FLDT_QWARNCNT, OI(DOFF(bwarns)), C1, 0, TYP_NONE },
> > -	{ "pad0", FLDT_INT32D, OI(DOFF(pad0)), C1, FLD_SKIPALL, TYP_NONE },
> > +	{ "pad0", FLDT_UINT32X, OI(DOFF(pad0)), C1, 0, TYP_NONE },
> >  	{ "rtb_hardlimit", FLDT_QCNT, OI(DOFF(rtb_hardlimit)), C1, 0,
> >  	  TYP_NONE },
> >  	{ "rtb_softlimit", FLDT_QCNT, OI(DOFF(rtb_softlimit)), C1, 0,
> > @@ -84,7 +84,7 @@ const field_t	disk_dquot_flds[] = {
> >  	{ "rtbcount", FLDT_QCNT, OI(DOFF(rtbcount)), C1, 0, TYP_NONE },
> >  	{ "rtbtimer", FLDT_INT32D, OI(DOFF(rtbtimer)), C1, 0, TYP_NONE },
> >  	{ "rtbwarns", FLDT_QWARNCNT, OI(DOFF(rtbwarns)), C1, 0, TYP_NONE },
> > -	{ "pad", FLDT_UINT16X, OI(DOFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
> > +	{ "pad", FLDT_UINT16X, OI(DOFF(pad)), C1, 0, TYP_NONE },
> >  	{ NULL }
> >  };
> >  
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/5] xfs_db: print structure padding fields consistently
  2017-11-30 21:51     ` Darrick J. Wong
@ 2017-11-30 21:57       ` Eric Sandeen
  2017-11-30 22:01         ` Darrick J. Wong
  0 siblings, 1 reply; 26+ messages in thread
From: Eric Sandeen @ 2017-11-30 21:57 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: sandeen, linux-xfs



On 11/30/17 3:51 PM, Darrick J. Wong wrote:
> On Thu, Nov 30, 2017 at 03:37:54PM -0600, Eric Sandeen wrote:
>> On 11/17/17 1:56 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> We are very inconsistent about how we print padding fields in on-disk
>>> structures -- sometimes we hide it from printall, sometimes we deviate
>>> from unsigned hex values, etc.  Make this all consistent -- never hide
>>> padding values, always print them as unsigned hex integers.
>>
>> I'm not sure I see the point to cluttering up structure printing
>> with padding, and would prefer to hide them /all/ by default (and
>> set them all to hex printing when explicitly asked for).
>>
>> Consistency is good but I'm not sure we need to consistently print
>> values that are never even used...?
> 
> Ok.  Will you take a patch that changes them all to FLD_SKIPALL?

and sets them all to hex type, and adds missing ones?  Sure.

If you love the idea of printing padding or need it, that's ok,
I'd just like a little justification for the extra clutter,
if you need it. :)

Thanks,
-Eric

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/5] xfs_db: print structure padding fields consistently
  2017-11-30 21:57       ` Eric Sandeen
@ 2017-11-30 22:01         ` Darrick J. Wong
  0 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2017-11-30 22:01 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: sandeen, linux-xfs

On Thu, Nov 30, 2017 at 03:57:16PM -0600, Eric Sandeen wrote:
> 
> 
> On 11/30/17 3:51 PM, Darrick J. Wong wrote:
> > On Thu, Nov 30, 2017 at 03:37:54PM -0600, Eric Sandeen wrote:
> >> On 11/17/17 1:56 PM, Darrick J. Wong wrote:
> >>> From: Darrick J. Wong <darrick.wong@oracle.com>
> >>>
> >>> We are very inconsistent about how we print padding fields in on-disk
> >>> structures -- sometimes we hide it from printall, sometimes we deviate
> >>> from unsigned hex values, etc.  Make this all consistent -- never hide
> >>> padding values, always print them as unsigned hex integers.
> >>
> >> I'm not sure I see the point to cluttering up structure printing
> >> with padding, and would prefer to hide them /all/ by default (and
> >> set them all to hex printing when explicitly asked for).
> >>
> >> Consistency is good but I'm not sure we need to consistently print
> >> values that are never even used...?
> > 
> > Ok.  Will you take a patch that changes them all to FLD_SKIPALL?
> 
> and sets them all to hex type, and adds missing ones?  Sure.
> 
> If you love the idea of printing padding or need it, that's ok,
> I'd just like a little justification for the extra clutter,
> if you need it. :)

Originally it was so that the fuzz tests could test whether or not
fsck/scrub notice garbage in the padding fields but then I observed that
we don't consistently write zeroes to padd fields, which means that we
can't check them in any meaningful manner.

--D

> Thanks,
> -Eric
> --
> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/5] xfs_io: provide an interface to the scrub ioctls
  2017-11-17 19:57 ` [PATCH 4/5] xfs_io: provide an interface to the scrub ioctls Darrick J. Wong
@ 2017-12-01  3:46   ` Eric Sandeen
  2017-12-01 17:02     ` Darrick J. Wong
  2017-12-01 16:06   ` Eric Sandeen
  2017-12-01 19:02   ` [PATCH v2 " Darrick J. Wong
  2 siblings, 1 reply; 26+ messages in thread
From: Eric Sandeen @ 2017-12-01  3:46 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs

On 11/17/17 1:57 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Create a new xfs_io command to call the new XFS metadata scrub ioctl.

Looks pretty reasonable, a few things to prove I looked ;)

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  io/Makefile       |    3 -
>  io/init.c         |    1 
>  io/io.h           |    2 
>  io/scrub.c        |  244 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  man/man8/xfs_io.8 |   10 ++
>  5 files changed, 259 insertions(+), 1 deletion(-)
>  create mode 100644 io/scrub.c
> 
> 
> diff --git a/io/Makefile b/io/Makefile
> index 050d6bd..b983bcc 100644
> --- a/io/Makefile
> +++ b/io/Makefile
> @@ -11,7 +11,8 @@ HFILES = init.h io.h
>  CFILES = init.c \
>  	attr.c bmap.c cowextsize.c encrypt.c file.c freeze.c fsync.c \
>  	getrusage.c imap.c link.c mmap.c open.c parent.c pread.c prealloc.c \
> -	pwrite.c reflink.c seek.c shutdown.c stat.c sync.c truncate.c utimes.c
> +	pwrite.c reflink.c scrub.c seek.c shutdown.c stat.c sync.c truncate.c \
> +	utimes.c
>  
>  LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD)
>  LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE)
> diff --git a/io/init.c b/io/init.c
> index 20d5f80..9e576fe 100644
> --- a/io/init.c
> +++ b/io/init.c
> @@ -84,6 +84,7 @@ init_commands(void)
>  	readdir_init();
>  	reflink_init();
>  	resblks_init();
> +	scrub_init();
>  	seek_init();
>  	sendfile_init();
>  	shutdown_init();
> diff --git a/io/io.h b/io/io.h
> index 3862985..82e142f 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -185,3 +185,5 @@ extern void		fsmap_init(void);
>  #else
>  # define fsmap_init()	do { } while (0)
>  #endif
> +
> +extern void		scrub_init(void);
> diff --git a/io/scrub.c b/io/scrub.c
> new file mode 100644
> index 0000000..cd2a1ee
> --- /dev/null
> +++ b/io/scrub.c
> @@ -0,0 +1,244 @@
> +/*
> + * Copyright (C) 2017 Oracle.  All Rights Reserved.
> + *
> + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> + *
> + * 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,
> + * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + */
> +
> +#include <sys/uio.h>
> +#include <xfs/xfs.h>
> +#include "command.h"
> +#include "input.h"
> +#include "init.h"
> +#include "path.h"
> +#include "io.h"
> +
> +static struct cmdinfo scrub_cmd;
> +
> +/* Type info and names for the scrub types. */
> +enum scrub_type {
> +	ST_NONE,	/* disabled */
> +	ST_PERAG,	/* per-AG metadata */
> +	ST_FS,		/* per-FS metadata */
> +	ST_INODE,	/* per-inode metadata */
> +};
> +
> +struct scrub_descr {
> +	const char	*name;
> +	enum scrub_type	type;
> +};
> +
> +static const struct scrub_descr scrubbers[XFS_SCRUB_TYPE_NR] = {
> +	[XFS_SCRUB_TYPE_PROBE]		= {"probe",		ST_NONE},
> +	[XFS_SCRUB_TYPE_SB]		= {"sb",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_AGF]		= {"agf",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_AGFL]		= {"agfl",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_AGI]		= {"agi",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_BNOBT]		= {"bnobt",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_CNTBT]		= {"cntbt",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_INOBT]		= {"inobt",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_FINOBT]		= {"finobt",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_RMAPBT]		= {"rmapbt",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_REFCNTBT]	= {"refcountbt",	ST_PERAG},
> +	[XFS_SCRUB_TYPE_INODE]		= {"inode",		ST_INODE},
> +	[XFS_SCRUB_TYPE_BMBTD]		= {"bmapbtd",		ST_INODE},
> +	[XFS_SCRUB_TYPE_BMBTA]		= {"bmapbta",		ST_INODE},
> +	[XFS_SCRUB_TYPE_BMBTC]		= {"bmapbtc",		ST_INODE},
> +	[XFS_SCRUB_TYPE_DIR]		= {"directory",		ST_INODE},
> +	[XFS_SCRUB_TYPE_XATTR]		= {"xattr",		ST_INODE},
> +	[XFS_SCRUB_TYPE_SYMLINK]	= {"symlink",		ST_INODE},
> +	[XFS_SCRUB_TYPE_PARENT]		= {"parent",		ST_INODE},
> +	[XFS_SCRUB_TYPE_RTBITMAP]	= {"rtbitmap",		ST_FS},
> +	[XFS_SCRUB_TYPE_RTSUM]		= {"rtsummary",		ST_FS},
> +	[XFS_SCRUB_TYPE_UQUOTA]		= {"usrquota",		ST_FS},
> +	[XFS_SCRUB_TYPE_GQUOTA]		= {"grpquota",		ST_FS},
> +	[XFS_SCRUB_TYPE_PQUOTA]		= {"prjquota",		ST_FS},
> +};
> +
> +static void
> +scrub_help(void)
> +{
> +	const struct scrub_descr	*d;
> +	int				i;
> +
> +	printf(_("\n\

Do you really love\
the continuation\
lines?

no other command uses this format, they're more "-happy... *shrug*

> + Scrubs a piece of XFS filesystem metadata.  The first argument is the type\n\
> + of metadata to examine.  Allocation group number(s) can be specified to\n\
> + restrict the scrub operation to a subset of allocation groups.\n\

actually, only 1 number can be.  "An allocation group number can be ..."

> + Certain metadata types do not take AG numbers.\n\
> +\n\
> + Example:\n\
> + 'scrub inobt 3' - scrub the inode btree in AG 3.\n\
> + 'scrub bmapbtd 128 13525' - scrubs the extent map of inode 128 gen 13525.\n\

Hm, how is a user to know which format goes with which type?

> +\n\
> + Known metadata scrub types are:"));
> +	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++)
> +		printf(" %s", d->name);
> +	printf("\n");
> +}
> +
> +static void
> +scrub_ioctl(
> +	int				fd,
> +	int				type,
> +	uint64_t			control,
> +	uint32_t			control2)
> +{
> +	struct xfs_scrub_metadata	meta;
> +	const struct scrub_descr	*sc;
> +	int				error;
> +
> +	sc = &scrubbers[type];
> +	memset(&meta, 0, sizeof(meta));
> +	meta.sm_type = type;
> +	switch (sc->type) {
> +	case ST_PERAG:
> +		meta.sm_agno = control;
> +		break;
> +	case ST_INODE:
> +		meta.sm_ino = control;
> +		meta.sm_gen = control2;
> +		break;
> +	case ST_NONE:
> +	case ST_FS:
> +		/* no control parameters */
> +		break;
> +	}
> +	meta.sm_flags = 0;
> +
> +	error = ioctl(fd, XFS_IOC_SCRUB_METADATA, &meta);
> +	if (error)
> +		perror("scrub");
> +	if (meta.sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
> +		printf(_("Corruption detected.\n"));
> +	if (meta.sm_flags & XFS_SCRUB_OFLAG_PREEN)
> +		printf(_("Optimization possible.\n"));
> +	if (meta.sm_flags & XFS_SCRUB_OFLAG_XFAIL)
> +		printf(_("Cross-referencing failed.\n"));
> +	if (meta.sm_flags & XFS_SCRUB_OFLAG_XCORRUPT)
> +		printf(_("Corruption detected during cross-referencing.\n"));
> +	if (meta.sm_flags & XFS_SCRUB_OFLAG_INCOMPLETE)
> +		printf(_("Scan was not complete.\n"));
> +}
> +
> +static int
> +parse_args(
> +	int				argc,
> +	char				**argv,
> +	struct cmdinfo			*cmdinfo,
> +	void				(*fn)(int, int, uint64_t, uint32_t))
> +{
> +	char				*p;
> +	int				type = -1;
> +	int				i, c;
> +	uint64_t			control = 0;
> +	uint32_t			control2 = 0;
> +	const struct scrub_descr	*d = NULL;
> +
> +	while ((c = getopt(argc, argv, "")) != EOF) {
> +		switch (c) {
> +		default:
> +			return command_usage(cmdinfo);
> +		}
> +	}
> +	if (optind > argc - 1)
> +		return command_usage(cmdinfo);
> +
> +	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++) {
> +		if (strcmp(d->name, argv[optind]) == 0) {
> +			type = i;
> +			break;
> +		}
> +	}
> +	optind++;
> +
> +	if (type < 0) {
		("unknown type %s\n", argv[optind]) ?

> +		return command_usage(cmdinfo);
	}
> +
> +	switch (d->type) {
> +	case ST_INODE:
> +		if (optind == argc) {
> +			control = 0;
> +			control2 = 0;
> +		} else if (optind == argc - 2) {
> +			control = strtoull(argv[optind], &p, 0);
> +			if (*p != '\0') {
> +				fprintf(stderr,
> +					_("Bad inode number %s.\n"), argv[i]);

I don't think this does what you think it does ;)  (i s/b optind?)

xfs_io> scrub bmapbtd foo bar
Bad inode number croatian.

(!)

> +				return 0;
> +			}
> +			control2 = strtoul(argv[optind + 1], &p, 0);
> +			if (*p != '\0') {
> +				fprintf(stderr,
> +					_("Bad generation number %s.\n"), argv[i]);

[optind + 1]

> +				return 0;
> +			}
> +		} else {
> +			fprintf(stderr,
> +				_("Must specify inode number and generation.\n"));
> +			return 0;
> +		}
> +		break;
> +	case ST_PERAG:
> +	case ST_NONE:
> +		if (optind != argc - 1) {
> +			fprintf(stderr,
> +				_("Must specify AG number.\n"));

since any arg count mismatch issues this warning, i.e.:

xfs_io> scrub inobt 1 2 3
Must specify AG number.

I wonder if "a single AG number" would be more clear.

(And the same treatment for i.e. "a single inode number and generation")

Also: does ST_NONE: (just "probe?") really take an AG number?  A quick
look at the kernel makes me think that gives -EINVAL?

> +			return 0;
> +		}
> +		control = strtoul(argv[optind], &p, 0);
> +		if (*p != '\0') {
> +			fprintf(stderr,
> +				_("Bad AG number %s.\n"), argv[i]);
> +			return 0;
> +		}
> +		break;
> +	default:

is this ST_FS: then?  Seems odd to catch it under default, wouldn't it make
sense to have it explicit here, and

        ST_FS:
> +		if (optind != argc) {
> +			fprintf(stderr,
> +				_("No parameters allowed.\n"));
> +			return 0;
> +		}
        default:
		ASSERT(0) 

or something?

> +	}
> +	fn(file->fd, type, control, control2);
> +
> +	return 0;
> +}
> +
> +static int
> +scrub_f(
> +	int				argc,
> +	char				**argv)
> +{
> +	return parse_args(argc, argv, &scrub_cmd, scrub_ioctl);

is there any reason to have parse_args factored out of the scrub_f function?

> +}
> +
> +void
> +scrub_init(void)
> +{
> +	scrub_cmd.name = "scrub";
> +	scrub_cmd.altname = "sc";
> +	scrub_cmd.cfunc = scrub_f;
> +	scrub_cmd.argmin = 1;
> +	scrub_cmd.argmax = -1;
> +	scrub_cmd.flags = CMD_NOMAP_OK;
> +	scrub_cmd.args =
> +_("type [agno...]");

don't need to outdent such a short string, no other command does.

> +	scrub_cmd.oneline => +		_("scrubs filesystem metadata");

This can also be on the same line

> +	scrub_cmd.help = scrub_help;
> +
> +	add_command(&scrub_cmd);
> +}
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 9bf1a47..ed10ba6 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -1119,6 +1119,16 @@ version of policy structure (numeric)
>  .BR get_encpolicy
>  On filesystems that support encryption, display the encryption policy of the
>  current file.
> +.TP
> +.BI "scrub " type " [ " agnumber... " | " "ino" " " "gen" " ]"
> +Scrub internal XFS filesystem metadata.  The
> +.BI type
> +parameter specifies which type of metadata to scrub.
> +For AG metadata, AG numbers can optionally be specified to restrict the

"an AG number can optionally ..."

> +scrub operation to a particular set of allocation groups.

"... to that allocation group."

> +By default, all allocation groups are scrubbed.
> +For file metadata, the scrub is applied to the open file unless the

a specific inode number and and ... (?)

> +inode number and generation number are specified.

I think explaining the generation number would be useful, i.e. where
does it come from, how is it found, how does the ioctl use it?
(is that too much?)

>  
>  .SH SEE ALSO
>  .BR mkfs.xfs (8),

add the scrub ioctl manpage here too, and/or in the main scrub section?

> 
> --
> 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
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 5/5] man: describe the metadata scrubbing ioctl
  2017-11-17 19:57 ` [PATCH 5/5] man: describe the metadata scrubbing ioctl Darrick J. Wong
@ 2017-12-01  4:34   ` Eric Sandeen
  2017-12-01  5:00     ` Eric Sandeen
  2017-12-01 18:12     ` Darrick J. Wong
  2017-12-01 19:03   ` [PATCH v2 " Darrick J. Wong
  1 sibling, 2 replies; 26+ messages in thread
From: Eric Sandeen @ 2017-12-01  4:34 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs

On 11/17/17 1:57 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Document the XFS-specific metadata scrub/repair ioctl's behavior,
> arguments, and side effects.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  man/Makefile                        |    2 
>  man/man2/Makefile                   |   22 +++
>  man/man2/ioctl_xfs_scrub_metadata.2 |  298 +++++++++++++++++++++++++++++++++++
>  3 files changed, 321 insertions(+), 1 deletion(-)
>  create mode 100644 man/man2/Makefile
>  create mode 100644 man/man2/ioctl_xfs_scrub_metadata.2
> 
> 
> diff --git a/man/Makefile b/man/Makefile
> index 863284c..cae891f 100644
> --- a/man/Makefile
> +++ b/man/Makefile
> @@ -5,7 +5,7 @@
>  TOPDIR = ..
>  include $(TOPDIR)/include/builddefs
>  
> -SUBDIRS = man3 man5 man8
> +SUBDIRS = man2 man3 man5 man8
>  
>  default : $(SUBDIRS)
>  
> diff --git a/man/man2/Makefile b/man/man2/Makefile
> new file mode 100644
> index 0000000..8aecde3
> --- /dev/null
> +++ b/man/man2/Makefile
> @@ -0,0 +1,22 @@
> +#
> +# Copyright (c) 2000-2001 Silicon Graphics, Inc.  All Rights Reserved.
> +#
> +
> +TOPDIR = ../..
> +include $(TOPDIR)/include/builddefs
> +
> +MAN_SECTION	= 2
> +
> +MAN_PAGES	= $(shell echo *.$(MAN_SECTION))
> +MAN_DEST	= $(PKG_MAN_DIR)/man$(MAN_SECTION)
> +LSRCFILES	= $(MAN_PAGES)
> +
> +default : $(MAN_PAGES)
> +
> +include $(BUILDRULES)
> +
> +install :
> +
> +install-dev : default
> +	$(INSTALL) -m 755 -d $(MAN_DEST)
> +	$(INSTALL_MAN)
> diff --git a/man/man2/ioctl_xfs_scrub_metadata.2 b/man/man2/ioctl_xfs_scrub_metadata.2
> new file mode 100644
> index 0000000..fa1c56d
> --- /dev/null
> +++ b/man/man2/ioctl_xfs_scrub_metadata.2
> @@ -0,0 +1,298 @@
> +.\" Copyright (c) 2017, Oracle.  All rights reserved.
> +.\"
> +.\" %%%LICENSE_START(GPLv2+_DOC_FULL)
> +.\" This is free documentation; 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.
> +.\"
> +.\" The GNU General Public License's references to "object code"
> +.\" and "executables" are to be interpreted as the output of any
> +.\" document formatting or typesetting system, including
> +.\" intermediate and printed output.
> +.\"
> +.\" This manual is distributed in the hope that it will 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 manual; if not, see
> +.\" <http://www.gnu.org/licenses/>.
> +.\" %%%LICENSE_END
> +.TH IOCTL-XFS-SCRUB-METADATA 2 2017-09-21 "Linux" "Linux Programmer's Manual"

This gets so long it doens't fit for me:

IOCTL-XFS-SCRUB-METADATA(2)Linux Programmer’s ManuaIOCTL-XFS-SCRUB-METADATA(2)

would just "xfs_scrub_metadata" be better? (drop the ioctl-?)

> +.SH NAME
> +ioctl_xfs_scrub_metadata \- check XFS filesystem metadata

if so then here too

> +.SH SYNOPSIS
> +.br
> +.B #include <xfs/xfs_fs.h>
> +.PP
> +.BI "int ioctl(int " dest_fd ", XFS_IOC_SCRUB_METADATA, struct xfs_scrub_metadata *" arg );
> +.SH DESCRIPTION
> +This XFS ioctl asks the kernel driver to examine a piece of metadata for
> +errors or suboptimal metadata.

This ioctl asks the xfs kernel driver to examine a piece of filesystem metadata ...

> +Examination includes running the metadata verifiers, checking records

(total editor nitpick: drop "the"?)

> +for obviously incorrect or impossible values, and cross-referencing each
> +record with any other available metadata in the filesystem.
> +This ioctl can also try to repair or optimize metadata, though this may
> +tie up the filesystem for a long period of time.

"tie up" is pretty colloquial.    "Render the filesystem unavailable?"
"Block normal filesystem operations?"

> +The type and location of the metadata 

the metadata to scrub

is conveyed in a structure of the
> +following form:
> +.PP
> +.in +4n
> +.EX
> +struct xfs_scrub_metadata {
> +	__u32 sm_type;
> +	__u32 sm_flags;
> +	__u64 sm_ino;
> +	__u32 sm_gen;
> +	__u32 sm_agno;
> +	__u64 sm_reserved[5];
> +};
> +.EE

this doesn't format properly on my rhel6 box but does
on my rhel7 box ...? weird.  This works for me, modeled on stat(2):

.nf
struct xfs_scrub_metadata {
	__u32 sm_type;
	__u32 sm_flags;
	__u64 sm_ino;
	__u32 sm_gen;
	__u32 sm_agno;
	__u64 sm_reserved[5];
};
.fi

> +.in
> +.PP
> +The field
> +.I sm_reserved
> +must be zero.
> +.PP
> +The field
> +.I sm_type
> +indicates the type of metadata to check:
> +.RS 0.4i
> +.TP
> +.B XFS_SCRUB_TYPE_PROBE
> +Probe the kernel to see if it is willing to try to check or repair this
> +filesystem.
> +If any
> +.B sm_flags
> +output flags are set in
> +.BR sm_gen ", "
> +they will be copied to
> +.B sm_flags
> +before the call returns.

Must sm_ino, sm_gen, or sm_agno be zero?  (I think so)

"All other fields must be zero?"

(Actually,this seems a bit inconsistent in the kernel; some accept
only the proper subset of the above 3, others don't seem to care
at all?)

In general for each type, if there are (or should be?) restrictions
on other fields that should be noted for each I think, or have a 
blanket statement that says any fields not specifically referenced
under a scrub type /must/ be zero.

> +
> +.PD 0
> +.PP
> +.nf
> +.B XFS_SCRUB_TYPE_SB
> +.B XFS_SCRUB_TYPE_AGF
> +.B XFS_SCRUB_TYPE_AGFL
> +.fi
> +.TP
> +.B XFS_SCRUB_TYPE_AGI
> +Examine a given allocation group's superblock, free space header, free
> +block list, or inode header, respectively.
> +Headers are checked for obviously incorrect values and cross-referenced
> +against the allocation group's metadata btrees, if possible.
> +The allocation group number must be given in
> +.BR sm_agno "."
> +
> +.PP
> +.nf
> +.B XFS_SCRUB_TYPE_BNOBT
> +.B XFS_SCRUB_TYPE_CNTBT
> +.B XFS_SCRUB_TYPE_INOBT
> +.B XFS_SCRUB_TYPE_FINOBT
> +.B XFS_SCRUB_TYPE_RMAPBT
> +.fi
> +.TP
> +.B XFS_SCRUB_TYPE_REFCNTBT
> +Examine a given allocation group's free space btrees, inode btress, reverse

inode btrees

> +mapping btrees, or reference count btrees, respectively.

You list 6 types but only 4 descriptions, "respectively"

> +against the allocation group's metadata btrees, if possible.

sentence fragment; comma after "respectively?"

> +Space extent records are checked for obviously incorrect values and
> +cross-referenced with the other space extent metadata to ensure that
> +there are no conflicts.
> +The allocation group number must be given in
> +.BR sm_agno "."
> +
> +.TP
> +.B XFS_SCRUB_TYPE_INODE
> +Examine a given inode's inode record for obviously incorrect values and

inode's inode record?  maybe just "examine a given inode for obviously ...?"

> +discrepancies with the rest of filesystem metadata.
> +Parent pointers are checked for impossible inode values and are then
> +followed up to the parent directory to ensure that the linkage makes
> +sense.

is correct.

> +The inode to examine can be specified either through
s/can be/may be/ manpage-wide (seems more man-style?  take it or leave
it ...)

> +.B sm_ino
> +and
> +.BR sm_gen "; "
> +if not specified, then the file described by
> +.B dest_fd
> +will be examined.
> +
> +.PP
> +.nf
> +.B XFS_SCRUB_TYPE_BMBTD
> +.B XFS_SCRUB_TYPE_BMBTA
> +.fi
> +.TP
> +.B XFS_SCRUB_TYPE_BMBTC

why are these set apart with .nf/.fi?

> +Examine a given inode's data block map, extended attribute block map,
> +copy on write block map, or parent inode pointer.

4 descriptions given for only 3 types.

> +Inode records are examined for obviously incorrect values and
> +discrepancies with the three block map types.
> +The block maps are checked for obviously wrong values and
> +cross-referenced with the allocation group space extent metadata for
> +discrepancies.
> +The inode to examine can be specified in the same manner as
> +.BR XFS_SCRUB_TYPE_INODE "."
> +
> +.TP
> +.B XFS_SCRUB_TYPE_XATTR
> +Examine the extended attribute records and indices of a given inode for
> +incorrect pointers and other signs of damage.
> +The inode to examine can be specified in the same manner as
> +.BR XFS_SCRUB_TYPE_INODE "."
> +
> +.TP
> +.B XFS_SCRUB_TYPE_DIR
> +Examine the entries in a given directory for invalid data or dangling pointers.
> +The directory to examine can be specified in the same manner as
> +.BR XFS_SCRUB_TYPE_INODE "."
> +
> +.TP
> +.B XFS_SCRUB_TYPE_SYMLINK
> +Examine the target of a symbolic link for obvious pathname problems.
> +The link to examine can be specified in the same manner as
> +.BR XFS_SCRUB_TYPE_INODE "."
> +
> +.PP
> +.nf
> +.B XFS_SCRUB_TYPE_RTBITMAP
> +.fi
> +.TP
> +.B XFS_SCRUB_TYPE_RTSUM
> +Examine the realtime block bitmap and realtime summary inodes for
> +corruption.
> +
> +.PP
> +.nf
> +.B XFS_SCRUB_TYPE_UQUOTA
> +.B XFS_SCRUB_TYPE_GQUOTA
> +.fi
> +.TP
> +.B XFS_SCRUB_TYPE_PQUOTA
> +Examine all user, group, or project quota records for corruption.
> +.RE
> +
> +.PD 1
> +.PP
> +The field
> +.I sm_flags
> +control the behavior of the scrub operation and provide more information
> +about the outcome of the operation.

is it worth highlighting IFLAG vs. OFLAG?  (Presumbably OFLAGs must
not be set going in?)

> +If none of the
> +.B XFS_SCRUB_OFLAG_*
> +flags are set upon return, the metadata is clean.
> +.RS 0.4i
> +.TP
> +.B XFS_SCRUB_IFLAG_REPAIR
> +If the caller sets this flag, the checker will examine the metadata and
> +try to fix any problems or suboptimal metadata that it finds.

This reader is left wondering what constitutes "suboptimal?"

> +If no errors occur during the repair operation, the check is performed a
> +second time to determine if the repair succeeded.

whether the repair

> +If errors do occur, the call returns an error status immediately.
> +.TP
> +.B XFS_SCRUB_OFLAG_CORRUPT
> +The metadata was corrupt when the call returned.
> +If
> +.B XFS_SCRUB_IFLAG_REPAIR
> +was specified, then an attempted repair failed to fix the problem.
> +Unmount the filesystem and run
> +.B xfs_repair
> +to fix the filesystem.
> +.TP
> +.B XFS_SCRUB_OFLAG_PREEN
> +The metadata is ok, but some aspect of the metadata could be optimized
> +to increase performance.

call again with "XFS_SCRUB_IFLAG_REPAIR" to optimize?

> +.TP
> +.B XFS_SCRUB_OFLAG_XFAIL
> +Filesystem errors were encountered when accessing other metadata to
> +cross-reference the records attached to this metadata object.
> +.TP
> +.B XFS_SCRUB_OFLAG_XCORRUPT
> +Discrepancies were found when cross-referencing the records attached to
> +this metadata object against all other available metadata in the system.
> +.TP
> +.B XFS_SCRUB_OFLAG_INCOMPLETE
> +The checker was unable to complete its check of all records.
> +.TP
> +.B XFS_SCRUB_OFLAG_WARNING
> +The checker encountered a metadata object with potentially problematic
> +records.
> +However, the records were not obviously corrupt.
> +.RE
> +.PP
> +For metadata checkers that operate on inodes or inode metadata, the fields
> +.IR sm_ino " and " sm_gen
> +are the inode number and generation number of the inode to check.

and where does one get the generation number?  What happens if it doesn't match?
What does it mean if it doesn't?  This is an operational detail that may
confuse many users I think.

> +If the inode number is zero, the inode represented by
> +.I dest_fd
> +is used instead.
> +.PP
> +For metadata checkers that operate on allocation group metadata, the field
> +.I sm_agno
> +indicates the allocation group in which to find the metadata.
> +.PP
> +For metadata checkers that operate on filesystem-wide metadata, no
> +further arguments are required.

are they even allowed?

> +.SH RETURN VALUE
> +On error, \-1 is returned, and
> +.I errno
> +is set to indicate the error.
> +.PP
> +.SH ERRORS
> +Error codes can be one of, but are not limited to, the following:
> +.TP
> +.B EBUSY
> +The filesystem object is busy; the repair will have to be tried agai
(is this only an IFLAG_REPAIR error?)

> +.TP
> +.B EFSCORRUPTED
> +Severe filesystem corruption was detected and could not be repaired.
> +Unmount the filesystem and run
> +.B xfs_repair
> +to fix the filesystem.

Is it always severe?  Does this only happen in response to IFLAG_REPAIR?
(same goes for other return values, I think many are only in response
to a repair request?  Might be worth noting)

> +.TP
> +.B EINVAL
> +One or more of the arguments specified is invalid.
> +.TP
> +.B ENOENT
> +The specified metadata object does not exist.
> +For example, this error code is returned for a
> +.B XFS_SCRUB_TYPE_REFCNTBT
> +request on a filesystem that does not support reflink.
> +.TP
> +.B ENOMEM
> +There was not sufficient memory to perform the scrub or repair operation.
> +Some operations (most notably reference count checking) require a lot of
> +memory.

may require large amounts of memory

> +.TP
> +.B ENOSPC
> +There is not enough free disk space to attempt a repair.
> +.TP
> +.B ENOTRECOVERABLE
> +Filesystem was mounted in
> +.B norecovery
> +mode and therefore has an unclean log.

so it can't be checked, or can't be repaired?
Is this returned for any ioctl on a norecovery mount, or only
repair requests?

> +.TP
> +.B ENOTTY
> +Online scrubbing or repair were not enabled.
> +.TP
> +.B EOPNOTSUPP
> +Repairs of the requested metadata object are not supported.
> +.TP
> +.B EROFS
> +Filesystem is read-only and a repair was requested.
> +.TP
> +.B ESHUTDOWN;

lose ";"

> +Filesystem is shut down due to previous errors.
> +.SH CONFORMING TO
> +This API is specific to XFS on Linux.

to the XFS filesystem on the Linux kernel.

> +.SH NOTES
> +These operations may tie up the filesystem for a long time.

again, colloquial?

> +A calling process can be stop the operation by being sent a fatal

word salad :D

> +signal, but non-fatal signals are blocked.
> +.SH SEE ALSO
> +.BR ioctl (2)

xfs_repair (8)

> 
> --
> 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
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 5/5] man: describe the metadata scrubbing ioctl
  2017-12-01  4:34   ` Eric Sandeen
@ 2017-12-01  5:00     ` Eric Sandeen
  2017-12-01 18:12     ` Darrick J. Wong
  1 sibling, 0 replies; 26+ messages in thread
From: Eric Sandeen @ 2017-12-01  5:00 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs



On 11/30/17 10:34 PM, Eric Sandeen wrote:
> On 11/17/17 1:57 PM, Darrick J. Wong wrote:

...

>> +.B sm_ino
>> +and
>> +.BR sm_gen "; "
>> +if not specified, then the file described by
>> +.B dest_fd
>> +will be examined.
>> +
>> +.PP
>> +.nf
>> +.B XFS_SCRUB_TYPE_BMBTD
>> +.B XFS_SCRUB_TYPE_BMBTA
>> +.fi
>> +.TP
>> +.B XFS_SCRUB_TYPE_BMBTC
> 
> why are these set apart with .nf/.fi?


oops nvm on this, I see what this is for.

-Eric

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/5] xfs_io: provide an interface to the scrub ioctls
  2017-11-17 19:57 ` [PATCH 4/5] xfs_io: provide an interface to the scrub ioctls Darrick J. Wong
  2017-12-01  3:46   ` Eric Sandeen
@ 2017-12-01 16:06   ` Eric Sandeen
  2017-12-01 17:03     ` Darrick J. Wong
  2017-12-01 19:02   ` [PATCH v2 " Darrick J. Wong
  2 siblings, 1 reply; 26+ messages in thread
From: Eric Sandeen @ 2017-12-01 16:06 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs



On 11/17/17 1:57 PM, Darrick J. Wong wrote:
> +/* Type info and names for the scrub types. */
> +enum scrub_type {
> +	ST_NONE,	/* disabled */
> +	ST_PERAG,	/* per-AG metadata */
> +	ST_FS,		/* per-FS metadata */
> +	ST_INODE,	/* per-inode metadata */
> +};

Actually, I'm a little confused by this, what is ST_NONE intended
to be used for?

You have it for probe, but that sounds more like an ST_FS fs-wide
action.

-Eric

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/5] xfs_io: provide an interface to the scrub ioctls
  2017-12-01  3:46   ` Eric Sandeen
@ 2017-12-01 17:02     ` Darrick J. Wong
  0 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2017-12-01 17:02 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: sandeen, linux-xfs

On Thu, Nov 30, 2017 at 09:46:01PM -0600, Eric Sandeen wrote:
> On 11/17/17 1:57 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Create a new xfs_io command to call the new XFS metadata scrub ioctl.
> 
> Looks pretty reasonable, a few things to prove I looked ;)
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  io/Makefile       |    3 -
> >  io/init.c         |    1 
> >  io/io.h           |    2 
> >  io/scrub.c        |  244 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  man/man8/xfs_io.8 |   10 ++
> >  5 files changed, 259 insertions(+), 1 deletion(-)
> >  create mode 100644 io/scrub.c
> > 
> > 
> > diff --git a/io/Makefile b/io/Makefile
> > index 050d6bd..b983bcc 100644
> > --- a/io/Makefile
> > +++ b/io/Makefile
> > @@ -11,7 +11,8 @@ HFILES = init.h io.h
> >  CFILES = init.c \
> >  	attr.c bmap.c cowextsize.c encrypt.c file.c freeze.c fsync.c \
> >  	getrusage.c imap.c link.c mmap.c open.c parent.c pread.c prealloc.c \
> > -	pwrite.c reflink.c seek.c shutdown.c stat.c sync.c truncate.c utimes.c
> > +	pwrite.c reflink.c scrub.c seek.c shutdown.c stat.c sync.c truncate.c \
> > +	utimes.c
> >  
> >  LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD)
> >  LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE)
> > diff --git a/io/init.c b/io/init.c
> > index 20d5f80..9e576fe 100644
> > --- a/io/init.c
> > +++ b/io/init.c
> > @@ -84,6 +84,7 @@ init_commands(void)
> >  	readdir_init();
> >  	reflink_init();
> >  	resblks_init();
> > +	scrub_init();
> >  	seek_init();
> >  	sendfile_init();
> >  	shutdown_init();
> > diff --git a/io/io.h b/io/io.h
> > index 3862985..82e142f 100644
> > --- a/io/io.h
> > +++ b/io/io.h
> > @@ -185,3 +185,5 @@ extern void		fsmap_init(void);
> >  #else
> >  # define fsmap_init()	do { } while (0)
> >  #endif
> > +
> > +extern void		scrub_init(void);
> > diff --git a/io/scrub.c b/io/scrub.c
> > new file mode 100644
> > index 0000000..cd2a1ee
> > --- /dev/null
> > +++ b/io/scrub.c
> > @@ -0,0 +1,244 @@
> > +/*
> > + * Copyright (C) 2017 Oracle.  All Rights Reserved.
> > + *
> > + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> > + *
> > + * 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,
> > + * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> > + */
> > +
> > +#include <sys/uio.h>
> > +#include <xfs/xfs.h>
> > +#include "command.h"
> > +#include "input.h"
> > +#include "init.h"
> > +#include "path.h"
> > +#include "io.h"
> > +
> > +static struct cmdinfo scrub_cmd;
> > +
> > +/* Type info and names for the scrub types. */
> > +enum scrub_type {
> > +	ST_NONE,	/* disabled */
> > +	ST_PERAG,	/* per-AG metadata */
> > +	ST_FS,		/* per-FS metadata */
> > +	ST_INODE,	/* per-inode metadata */
> > +};
> > +
> > +struct scrub_descr {
> > +	const char	*name;
> > +	enum scrub_type	type;
> > +};
> > +
> > +static const struct scrub_descr scrubbers[XFS_SCRUB_TYPE_NR] = {
> > +	[XFS_SCRUB_TYPE_PROBE]		= {"probe",		ST_NONE},
> > +	[XFS_SCRUB_TYPE_SB]		= {"sb",		ST_PERAG},
> > +	[XFS_SCRUB_TYPE_AGF]		= {"agf",		ST_PERAG},
> > +	[XFS_SCRUB_TYPE_AGFL]		= {"agfl",		ST_PERAG},
> > +	[XFS_SCRUB_TYPE_AGI]		= {"agi",		ST_PERAG},
> > +	[XFS_SCRUB_TYPE_BNOBT]		= {"bnobt",		ST_PERAG},
> > +	[XFS_SCRUB_TYPE_CNTBT]		= {"cntbt",		ST_PERAG},
> > +	[XFS_SCRUB_TYPE_INOBT]		= {"inobt",		ST_PERAG},
> > +	[XFS_SCRUB_TYPE_FINOBT]		= {"finobt",		ST_PERAG},
> > +	[XFS_SCRUB_TYPE_RMAPBT]		= {"rmapbt",		ST_PERAG},
> > +	[XFS_SCRUB_TYPE_REFCNTBT]	= {"refcountbt",	ST_PERAG},
> > +	[XFS_SCRUB_TYPE_INODE]		= {"inode",		ST_INODE},
> > +	[XFS_SCRUB_TYPE_BMBTD]		= {"bmapbtd",		ST_INODE},
> > +	[XFS_SCRUB_TYPE_BMBTA]		= {"bmapbta",		ST_INODE},
> > +	[XFS_SCRUB_TYPE_BMBTC]		= {"bmapbtc",		ST_INODE},
> > +	[XFS_SCRUB_TYPE_DIR]		= {"directory",		ST_INODE},
> > +	[XFS_SCRUB_TYPE_XATTR]		= {"xattr",		ST_INODE},
> > +	[XFS_SCRUB_TYPE_SYMLINK]	= {"symlink",		ST_INODE},
> > +	[XFS_SCRUB_TYPE_PARENT]		= {"parent",		ST_INODE},
> > +	[XFS_SCRUB_TYPE_RTBITMAP]	= {"rtbitmap",		ST_FS},
> > +	[XFS_SCRUB_TYPE_RTSUM]		= {"rtsummary",		ST_FS},
> > +	[XFS_SCRUB_TYPE_UQUOTA]		= {"usrquota",		ST_FS},
> > +	[XFS_SCRUB_TYPE_GQUOTA]		= {"grpquota",		ST_FS},
> > +	[XFS_SCRUB_TYPE_PQUOTA]		= {"prjquota",		ST_FS},
> > +};
> > +
> > +static void
> > +scrub_help(void)
> > +{
> > +	const struct scrub_descr	*d;
> > +	int				i;
> > +
> > +	printf(_("\n\
> 
> Do you really love\
> the continuation\
> lines?

I told you before,\
if you review in haiku,\
...oh wait, that ended. :)

> no other command uses this format, they're more "-happy... *shrug*
> 
> > + Scrubs a piece of XFS filesystem metadata.  The first argument is the type\n\
> > + of metadata to examine.  Allocation group number(s) can be specified to\n\
> > + restrict the scrub operation to a subset of allocation groups.\n\
> 
> actually, only 1 number can be.  "An allocation group number can be ..."
> 
> > + Certain metadata types do not take AG numbers.\n\
> > +\n\
> > + Example:\n\
> > + 'scrub inobt 3' - scrub the inode btree in AG 3.\n\
> > + 'scrub bmapbtd 128 13525' - scrubs the extent map of inode 128 gen 13525.\n\
> 
> Hm, how is a user to know which format goes with which type?

	printf(_(
"\n"
" Scrubs a piece of XFS filesystem metadata.  The first argument is the type\n"
" of metadata to examine.  Allocation group metadata types take one AG number\n"
" as the second parameter.  Inode metadata types act on the currently open file\n"
" or (optionally) take an inode number and generation number to act upon as\n"
" the second and third parameters.\n"
"\n"
" Example:\n"
" 'scrub inobt 3' - scrub the inode btree in AG 3.\n"
" 'scrub bmapbtd 128 13525' - scrubs the extent map of inode 128 gen 13525.\n"
"\n"
" Known metadata scrub types are:"));

> 
> > +\n\
> > + Known metadata scrub types are:"));
> > +	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++)
> > +		printf(" %s", d->name);
> > +	printf("\n");
> > +}
> > +
> > +static void
> > +scrub_ioctl(
> > +	int				fd,
> > +	int				type,
> > +	uint64_t			control,
> > +	uint32_t			control2)
> > +{
> > +	struct xfs_scrub_metadata	meta;
> > +	const struct scrub_descr	*sc;
> > +	int				error;
> > +
> > +	sc = &scrubbers[type];
> > +	memset(&meta, 0, sizeof(meta));
> > +	meta.sm_type = type;
> > +	switch (sc->type) {
> > +	case ST_PERAG:
> > +		meta.sm_agno = control;
> > +		break;
> > +	case ST_INODE:
> > +		meta.sm_ino = control;
> > +		meta.sm_gen = control2;
> > +		break;
> > +	case ST_NONE:
> > +	case ST_FS:
> > +		/* no control parameters */
> > +		break;
> > +	}
> > +	meta.sm_flags = 0;
> > +
> > +	error = ioctl(fd, XFS_IOC_SCRUB_METADATA, &meta);
> > +	if (error)
> > +		perror("scrub");
> > +	if (meta.sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
> > +		printf(_("Corruption detected.\n"));
> > +	if (meta.sm_flags & XFS_SCRUB_OFLAG_PREEN)
> > +		printf(_("Optimization possible.\n"));
> > +	if (meta.sm_flags & XFS_SCRUB_OFLAG_XFAIL)
> > +		printf(_("Cross-referencing failed.\n"));
> > +	if (meta.sm_flags & XFS_SCRUB_OFLAG_XCORRUPT)
> > +		printf(_("Corruption detected during cross-referencing.\n"));
> > +	if (meta.sm_flags & XFS_SCRUB_OFLAG_INCOMPLETE)
> > +		printf(_("Scan was not complete.\n"));
> > +}
> > +
> > +static int
> > +parse_args(
> > +	int				argc,
> > +	char				**argv,
> > +	struct cmdinfo			*cmdinfo,
> > +	void				(*fn)(int, int, uint64_t, uint32_t))
> > +{
> > +	char				*p;
> > +	int				type = -1;
> > +	int				i, c;
> > +	uint64_t			control = 0;
> > +	uint32_t			control2 = 0;
> > +	const struct scrub_descr	*d = NULL;
> > +
> > +	while ((c = getopt(argc, argv, "")) != EOF) {
> > +		switch (c) {
> > +		default:
> > +			return command_usage(cmdinfo);
> > +		}
> > +	}
> > +	if (optind > argc - 1)
> > +		return command_usage(cmdinfo);
> > +
> > +	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++) {
> > +		if (strcmp(d->name, argv[optind]) == 0) {
> > +			type = i;
> > +			break;
> > +		}
> > +	}
> > +	optind++;
> > +
> > +	if (type < 0) {
> 		("unknown type %s\n", argv[optind]) ?
> 
> > +		return command_usage(cmdinfo);
> 	}

Ok.

> > +
> > +	switch (d->type) {
> > +	case ST_INODE:
> > +		if (optind == argc) {
> > +			control = 0;
> > +			control2 = 0;
> > +		} else if (optind == argc - 2) {
> > +			control = strtoull(argv[optind], &p, 0);
> > +			if (*p != '\0') {
> > +				fprintf(stderr,
> > +					_("Bad inode number %s.\n"), argv[i]);
> 
> I don't think this does what you think it does ;)  (i s/b optind?)

Inconceivable!

> xfs_io> scrub bmapbtd foo bar
> Bad inode number croatian.
> 
> (!)

Fixed, thanks.

> > +				return 0;
> > +			}
> > +			control2 = strtoul(argv[optind + 1], &p, 0);
> > +			if (*p != '\0') {
> > +				fprintf(stderr,
> > +					_("Bad generation number %s.\n"), argv[i]);
> 
> [optind + 1]
> 
> > +				return 0;
> > +			}
> > +		} else {
> > +			fprintf(stderr,
> > +				_("Must specify inode number and generation.\n"));
> > +			return 0;
> > +		}
> > +		break;
> > +	case ST_PERAG:
> > +	case ST_NONE:
> > +		if (optind != argc - 1) {
> > +			fprintf(stderr,
> > +				_("Must specify AG number.\n"));
> 
> since any arg count mismatch issues this warning, i.e.:
> 
> xfs_io> scrub inobt 1 2 3
> Must specify AG number.
> 
> I wonder if "a single AG number" would be more clear.

Yes.

> (And the same treatment for i.e. "a single inode number and generation")
> 
> Also: does ST_NONE: (just "probe?") really take an AG number?  A quick
> look at the kernel makes me think that gives -EINVAL?

The probe scrubber used to echo back whatever the agno in oflags, but that
went away in the review process so ST_NONE/ST_FS can go at the bottom where
default clause is currently.

> 
> > +			return 0;
> > +		}
> > +		control = strtoul(argv[optind], &p, 0);
> > +		if (*p != '\0') {
> > +			fprintf(stderr,
> > +				_("Bad AG number %s.\n"), argv[i]);
> > +			return 0;
> > +		}
> > +		break;
> > +	default:
> 
> is this ST_FS: then?  Seems odd to catch it under default, wouldn't it make
> sense to have it explicit here, and
> 
>         ST_FS:
> > +		if (optind != argc) {
> > +			fprintf(stderr,
> > +				_("No parameters allowed.\n"));
> > +			return 0;
> > +		}
>         default:
> 		ASSERT(0) 
> 
> or something?

Yes, done.

> > +	}
> > +	fn(file->fd, type, control, control2);
> > +
> > +	return 0;
> > +}
> > +
> > +static int
> > +scrub_f(
> > +	int				argc,
> > +	char				**argv)
> > +{
> > +	return parse_args(argc, argv, &scrub_cmd, scrub_ioctl);
> 
> is there any reason to have parse_args factored out of the scrub_f function?

Yes, repair_f will use the same parse_args some day.

> > +}
> > +
> > +void
> > +scrub_init(void)
> > +{
> > +	scrub_cmd.name = "scrub";
> > +	scrub_cmd.altname = "sc";
> > +	scrub_cmd.cfunc = scrub_f;
> > +	scrub_cmd.argmin = 1;
> > +	scrub_cmd.argmax = -1;
> > +	scrub_cmd.flags = CMD_NOMAP_OK;
> > +	scrub_cmd.args =
> > +_("type [agno...]");
> 
> don't need to outdent such a short string, no other command does.
> 
> > +	scrub_cmd.oneline => +		_("scrubs filesystem metadata");
> 
> This can also be on the same line

Will fix the outdent and bad help text.

> > +	scrub_cmd.help = scrub_help;
> > +
> > +	add_command(&scrub_cmd);
> > +}
> > diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> > index 9bf1a47..ed10ba6 100644
> > --- a/man/man8/xfs_io.8
> > +++ b/man/man8/xfs_io.8
> > @@ -1119,6 +1119,16 @@ version of policy structure (numeric)
> >  .BR get_encpolicy
> >  On filesystems that support encryption, display the encryption policy of the
> >  current file.
> > +.TP
> > +.BI "scrub " type " [ " agnumber... " | " "ino" " " "gen" " ]"
> > +Scrub internal XFS filesystem metadata.  The
> > +.BI type
> > +parameter specifies which type of metadata to scrub.
> > +For AG metadata, AG numbers can optionally be specified to restrict the
> 
> "an AG number can optionally ..."
> 
> > +scrub operation to a particular set of allocation groups.
> 
> "... to that allocation group."
> 
> > +By default, all allocation groups are scrubbed.
> > +For file metadata, the scrub is applied to the open file unless the
> 
> a specific inode number and and ... (?)
> 
> > +inode number and generation number are specified.
> 
> I think explaining the generation number would be useful, i.e. where
> does it come from, how is it found, how does the ioctl use it?
> (is that too much?)

I don't know that I really /want/ people using the raw handle interface
since (afaik) the only way to get the generation number is BULKSTAT or
xfs_db.  If you're going to use the raw xfs_io interface then you had
better know some about how inodes work in xfs.

> >  
> >  .SH SEE ALSO
> >  .BR mkfs.xfs (8),
> 
> add the scrub ioctl manpage here too, and/or in the main scrub section?

Next patch, but sure?  Dunno, we don't link to the getfsmap manpage but
we have an xfs_io command for it.

--D

> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/5] xfs_io: provide an interface to the scrub ioctls
  2017-12-01 16:06   ` Eric Sandeen
@ 2017-12-01 17:03     ` Darrick J. Wong
  0 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2017-12-01 17:03 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: sandeen, linux-xfs

On Fri, Dec 01, 2017 at 10:06:51AM -0600, Eric Sandeen wrote:
> 
> 
> On 11/17/17 1:57 PM, Darrick J. Wong wrote:
> > +/* Type info and names for the scrub types. */
> > +enum scrub_type {
> > +	ST_NONE,	/* disabled */
> > +	ST_PERAG,	/* per-AG metadata */
> > +	ST_FS,		/* per-FS metadata */
> > +	ST_INODE,	/* per-inode metadata */
> > +};
> 
> Actually, I'm a little confused by this, what is ST_NONE intended
> to be used for?
> 
> You have it for probe, but that sounds more like an ST_FS fs-wide
> action.

It means "things that aren't fs, ag, or inode metadata" which at this
point only encompasses the probe command.

--D

> -Eric
> --
> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 5/5] man: describe the metadata scrubbing ioctl
  2017-12-01  4:34   ` Eric Sandeen
  2017-12-01  5:00     ` Eric Sandeen
@ 2017-12-01 18:12     ` Darrick J. Wong
  1 sibling, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2017-12-01 18:12 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: sandeen, linux-xfs

On Thu, Nov 30, 2017 at 10:34:29PM -0600, Eric Sandeen wrote:
> On 11/17/17 1:57 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Document the XFS-specific metadata scrub/repair ioctl's behavior,
> > arguments, and side effects.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  man/Makefile                        |    2 
> >  man/man2/Makefile                   |   22 +++
> >  man/man2/ioctl_xfs_scrub_metadata.2 |  298 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 321 insertions(+), 1 deletion(-)
> >  create mode 100644 man/man2/Makefile
> >  create mode 100644 man/man2/ioctl_xfs_scrub_metadata.2
> > 
> > 
> > diff --git a/man/Makefile b/man/Makefile
> > index 863284c..cae891f 100644
> > --- a/man/Makefile
> > +++ b/man/Makefile
> > @@ -5,7 +5,7 @@
> >  TOPDIR = ..
> >  include $(TOPDIR)/include/builddefs
> >  
> > -SUBDIRS = man3 man5 man8
> > +SUBDIRS = man2 man3 man5 man8
> >  
> >  default : $(SUBDIRS)
> >  
> > diff --git a/man/man2/Makefile b/man/man2/Makefile
> > new file mode 100644
> > index 0000000..8aecde3
> > --- /dev/null
> > +++ b/man/man2/Makefile
> > @@ -0,0 +1,22 @@
> > +#
> > +# Copyright (c) 2000-2001 Silicon Graphics, Inc.  All Rights Reserved.
> > +#
> > +
> > +TOPDIR = ../..
> > +include $(TOPDIR)/include/builddefs
> > +
> > +MAN_SECTION	= 2
> > +
> > +MAN_PAGES	= $(shell echo *.$(MAN_SECTION))
> > +MAN_DEST	= $(PKG_MAN_DIR)/man$(MAN_SECTION)
> > +LSRCFILES	= $(MAN_PAGES)
> > +
> > +default : $(MAN_PAGES)
> > +
> > +include $(BUILDRULES)
> > +
> > +install :
> > +
> > +install-dev : default
> > +	$(INSTALL) -m 755 -d $(MAN_DEST)
> > +	$(INSTALL_MAN)
> > diff --git a/man/man2/ioctl_xfs_scrub_metadata.2 b/man/man2/ioctl_xfs_scrub_metadata.2
> > new file mode 100644
> > index 0000000..fa1c56d
> > --- /dev/null
> > +++ b/man/man2/ioctl_xfs_scrub_metadata.2
> > @@ -0,0 +1,298 @@
> > +.\" Copyright (c) 2017, Oracle.  All rights reserved.
> > +.\"
> > +.\" %%%LICENSE_START(GPLv2+_DOC_FULL)
> > +.\" This is free documentation; 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.
> > +.\"
> > +.\" The GNU General Public License's references to "object code"
> > +.\" and "executables" are to be interpreted as the output of any
> > +.\" document formatting or typesetting system, including
> > +.\" intermediate and printed output.
> > +.\"
> > +.\" This manual is distributed in the hope that it will 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 manual; if not, see
> > +.\" <http://www.gnu.org/licenses/>.
> > +.\" %%%LICENSE_END
> > +.TH IOCTL-XFS-SCRUB-METADATA 2 2017-09-21 "Linux" "Linux Programmer's Manual"
> 
> This gets so long it doens't fit for me:
> 
> IOCTL-XFS-SCRUB-METADATA(2)Linux Programmer’s ManuaIOCTL-XFS-SCRUB-METADATA(2)
> 
> would just "xfs_scrub_metadata" be better? (drop the ioctl-?)
> 
> > +.SH NAME
> > +ioctl_xfs_scrub_metadata \- check XFS filesystem metadata
> 
> if so then here too

<shrug> the weird ioctl[-_] prefix is what gets put on the man-pages.git
entries for ioctls.

However, as this isn't Linux I could change that to:

.TH IOCTL-XFS-SCRUB-METADATA 2 2017-12-01 "XFS"

> 
> > +.SH SYNOPSIS
> > +.br
> > +.B #include <xfs/xfs_fs.h>
> > +.PP
> > +.BI "int ioctl(int " dest_fd ", XFS_IOC_SCRUB_METADATA, struct xfs_scrub_metadata *" arg );
> > +.SH DESCRIPTION
> > +This XFS ioctl asks the kernel driver to examine a piece of metadata for
> > +errors or suboptimal metadata.
> 
> This ioctl asks the xfs kernel driver to examine a piece of filesystem metadata ...

Yes.

> > +Examination includes running the metadata verifiers, checking records
> 
> (total editor nitpick: drop "the"?)

Sure.

> > +for obviously incorrect or impossible values, and cross-referencing each
> > +record with any other available metadata in the filesystem.
> > +This ioctl can also try to repair or optimize metadata, though this may
> > +tie up the filesystem for a long period of time.
> 
> "tie up" is pretty colloquial.    "Render the filesystem unavailable?"
> "Block normal filesystem operations?"

Yes, good call.  I choose the third option.

> > +The type and location of the metadata 
> 
> the metadata to scrub

Ok.

> is conveyed in a structure of the
> > +following form:
> > +.PP
> > +.in +4n
> > +.EX
> > +struct xfs_scrub_metadata {
> > +	__u32 sm_type;
> > +	__u32 sm_flags;
> > +	__u64 sm_ino;
> > +	__u32 sm_gen;
> > +	__u32 sm_agno;
> > +	__u64 sm_reserved[5];
> > +};
> > +.EE
> 
> this doesn't format properly on my rhel6 box but does
> on my rhel7 box ...? weird.  This works for me, modeled on stat(2):
> 
> .nf
> struct xfs_scrub_metadata {
> 	__u32 sm_type;
> 	__u32 sm_flags;
> 	__u64 sm_ino;
> 	__u32 sm_gen;
> 	__u32 sm_agno;
> 	__u64 sm_reserved[5];
> };
> .fi

Either work here, so ... sure?

(I don't remember my *off directives anyway.)

> > +.in
> > +.PP
> > +The field
> > +.I sm_reserved
> > +must be zero.
> > +.PP
> > +The field
> > +.I sm_type
> > +indicates the type of metadata to check:
> > +.RS 0.4i
> > +.TP
> > +.B XFS_SCRUB_TYPE_PROBE
> > +Probe the kernel to see if it is willing to try to check or repair this
> > +filesystem.
> > +If any
> > +.B sm_flags
> > +output flags are set in
> > +.BR sm_gen ", "
> > +they will be copied to
> > +.B sm_flags
> > +before the call returns.
> 
> Must sm_ino, sm_gen, or sm_agno be zero?  (I think so)
> 
> "All other fields must be zero?"
> 
> (Actually,this seems a bit inconsistent in the kernel; some accept
> only the proper subset of the above 3, others don't seem to care
> at all?)
> 
> In general for each type, if there are (or should be?) restrictions
> on other fields that should be noted for each I think, or have a 
> blanket statement that says any fields not specifically referenced
> under a scrub type /must/ be zero.

Ok, I'll do that.  I will also have a look at the scrub init code in
the kernel to make sure that it actually checks these things.

> > +
> > +.PD 0
> > +.PP
> > +.nf
> > +.B XFS_SCRUB_TYPE_SB
> > +.B XFS_SCRUB_TYPE_AGF
> > +.B XFS_SCRUB_TYPE_AGFL
> > +.fi
> > +.TP
> > +.B XFS_SCRUB_TYPE_AGI
> > +Examine a given allocation group's superblock, free space header, free
> > +block list, or inode header, respectively.
> > +Headers are checked for obviously incorrect values and cross-referenced
> > +against the allocation group's metadata btrees, if possible.
> > +The allocation group number must be given in
> > +.BR sm_agno "."
> > +
> > +.PP
> > +.nf
> > +.B XFS_SCRUB_TYPE_BNOBT
> > +.B XFS_SCRUB_TYPE_CNTBT
> > +.B XFS_SCRUB_TYPE_INOBT
> > +.B XFS_SCRUB_TYPE_FINOBT
> > +.B XFS_SCRUB_TYPE_RMAPBT
> > +.fi
> > +.TP
> > +.B XFS_SCRUB_TYPE_REFCNTBT
> > +Examine a given allocation group's free space btrees, inode btress, reverse
> 
> inode btrees
> 
> > +mapping btrees, or reference count btrees, respectively.
> 
> You list 6 types but only 4 descriptions, "respectively"

"Examine a given allocation group's two free space btrees, two inode
btrees, reverse mapping btrees, or reference count btrees, respectively.
Records are checked for obviously incorrect values and cross-referenced
with other allocation group metadata records to ensure that there are no
conflicts.  The allocation group number must be given in sm_agno."

> > +against the allocation group's metadata btrees, if possible.
> 
> sentence fragment; comma after "respectively?"
> 
> > +Space extent records are checked for obviously incorrect values and
> > +cross-referenced with the other space extent metadata to ensure that
> > +there are no conflicts.
> > +The allocation group number must be given in
> > +.BR sm_agno "."
> > +
> > +.TP
> > +.B XFS_SCRUB_TYPE_INODE
> > +Examine a given inode's inode record for obviously incorrect values and
> 
> inode's inode record?  maybe just "examine a given inode for obviously ...?"

"Examine a given inode record for..." ?

> > +discrepancies with the rest of filesystem metadata.
> > +Parent pointers are checked for impossible inode values and are then
> > +followed up to the parent directory to ensure that the linkage makes
> > +sense.
> 
> is correct.

Ok.

> > +The inode to examine can be specified either through
>
> s/can be/may be/ manpage-wide (seems more man-style?  take it or leave
> it ...)

Sure.

> > +.B sm_ino
> > +and
> > +.BR sm_gen "; "
> > +if not specified, then the file described by
> > +.B dest_fd
> > +will be examined.
> > +
> > +.PP
> > +.nf
> > +.B XFS_SCRUB_TYPE_BMBTD
> > +.B XFS_SCRUB_TYPE_BMBTA
> > +.fi
> > +.TP
> > +.B XFS_SCRUB_TYPE_BMBTC
> 
> why are these set apart with .nf/.fi?

/me doesn't know how to make a proper list. :(

> > +Examine a given inode's data block map, extended attribute block map,
> > +copy on write block map, or parent inode pointer.
> 
> 4 descriptions given for only 3 types.

Whoops, add a XFS_SCRUB_TYPE_PARENT into that list.

> > +Inode records are examined for obviously incorrect values and
> > +discrepancies with the three block map types.
> > +The block maps are checked for obviously wrong values and
> > +cross-referenced with the allocation group space extent metadata for
> > +discrepancies.
> > +The inode to examine can be specified in the same manner as
> > +.BR XFS_SCRUB_TYPE_INODE "."
> > +
> > +.TP
> > +.B XFS_SCRUB_TYPE_XATTR
> > +Examine the extended attribute records and indices of a given inode for
> > +incorrect pointers and other signs of damage.
> > +The inode to examine can be specified in the same manner as
> > +.BR XFS_SCRUB_TYPE_INODE "."
> > +
> > +.TP
> > +.B XFS_SCRUB_TYPE_DIR
> > +Examine the entries in a given directory for invalid data or dangling pointers.
> > +The directory to examine can be specified in the same manner as
> > +.BR XFS_SCRUB_TYPE_INODE "."
> > +
> > +.TP
> > +.B XFS_SCRUB_TYPE_SYMLINK
> > +Examine the target of a symbolic link for obvious pathname problems.
> > +The link to examine can be specified in the same manner as
> > +.BR XFS_SCRUB_TYPE_INODE "."
> > +
> > +.PP
> > +.nf
> > +.B XFS_SCRUB_TYPE_RTBITMAP
> > +.fi
> > +.TP
> > +.B XFS_SCRUB_TYPE_RTSUM
> > +Examine the realtime block bitmap and realtime summary inodes for
> > +corruption.
> > +
> > +.PP
> > +.nf
> > +.B XFS_SCRUB_TYPE_UQUOTA
> > +.B XFS_SCRUB_TYPE_GQUOTA
> > +.fi
> > +.TP
> > +.B XFS_SCRUB_TYPE_PQUOTA
> > +Examine all user, group, or project quota records for corruption.
> > +.RE
> > +
> > +.PD 1
> > +.PP
> > +The field
> > +.I sm_flags
> > +control the behavior of the scrub operation and provide more information
> > +about the outcome of the operation.
> 
> is it worth highlighting IFLAG vs. OFLAG?  (Presumbably OFLAGs must
> not be set going in?)

The output flags are cleared at the start of xfs_scrub_metadata so it
doesn't matter if they're set going in.

> > +If none of the
> > +.B XFS_SCRUB_OFLAG_*
> > +flags are set upon return, the metadata is clean.
> > +.RS 0.4i
> > +.TP
> > +.B XFS_SCRUB_IFLAG_REPAIR
> > +If the caller sets this flag, the checker will examine the metadata and
> > +try to fix any problems or suboptimal metadata that it finds.
> 
> This reader is left wondering what constitutes "suboptimal?"
> 
> > +If no errors occur during the repair operation, the check is performed a
> > +second time to determine if the repair succeeded.
> 
> whether the repair

"If the caller sets this flag, the kernel driver will examine the
metadata and try to fix all problems and to optimize metadata when
possible.  If no errors occur during the repair operation, the check is
performed a second time to determine whether the repair succeeded.  If
errors occur, the call returns an error status immediately."

> 
> > +If errors do occur, the call returns an error status immediately.
> > +.TP
> > +.B XFS_SCRUB_OFLAG_CORRUPT
> > +The metadata was corrupt when the call returned.
> > +If
> > +.B XFS_SCRUB_IFLAG_REPAIR
> > +was specified, then an attempted repair failed to fix the problem.
> > +Unmount the filesystem and run
> > +.B xfs_repair
> > +to fix the filesystem.
> > +.TP
> > +.B XFS_SCRUB_OFLAG_PREEN
> > +The metadata is ok, but some aspect of the metadata could be optimized
> > +to increase performance.
> 
> call again with "XFS_SCRUB_IFLAG_REPAIR" to optimize?

Sure.

> > +.TP
> > +.B XFS_SCRUB_OFLAG_XFAIL
> > +Filesystem errors were encountered when accessing other metadata to
> > +cross-reference the records attached to this metadata object.
> > +.TP
> > +.B XFS_SCRUB_OFLAG_XCORRUPT
> > +Discrepancies were found when cross-referencing the records attached to
> > +this metadata object against all other available metadata in the system.
> > +.TP
> > +.B XFS_SCRUB_OFLAG_INCOMPLETE
> > +The checker was unable to complete its check of all records.
> > +.TP
> > +.B XFS_SCRUB_OFLAG_WARNING
> > +The checker encountered a metadata object with potentially problematic
> > +records.
> > +However, the records were not obviously corrupt.
> > +.RE
> > +.PP
> > +For metadata checkers that operate on inodes or inode metadata, the fields
> > +.IR sm_ino " and " sm_gen
> > +are the inode number and generation number of the inode to check.
> 
> and where does one get the generation number?  What happens if it doesn't match?
> What does it mean if it doesn't?  This is an operational detail that may
> confuse many users I think.

Ok, I'll add a sentence:

If the generation number of the inode does not match sm_gen,
the call will return an error code for the invalid argument.

> > +If the inode number is zero, the inode represented by
> > +.I dest_fd
> > +is used instead.
> > +.PP
> > +For metadata checkers that operate on allocation group metadata, the field
> > +.I sm_agno
> > +indicates the allocation group in which to find the metadata.
> > +.PP
> > +For metadata checkers that operate on filesystem-wide metadata, no
> > +further arguments are required.
> 
> are they even allowed?

Will add a sentence explicitly saying they must be zero.

> > +.SH RETURN VALUE
> > +On error, \-1 is returned, and
> > +.I errno
> > +is set to indicate the error.
> > +.PP
> > +.SH ERRORS
> > +Error codes can be one of, but are not limited to, the following:
> > +.TP
> > +.B EBUSY
> > +The filesystem object is busy; the repair will have to be tried agai
> (is this only an IFLAG_REPAIR error?)

Hm, you're right, this should just say "the operation will have to be
tried again".

None of the scrub code actually return EBUSY, but we might as well
reserve the possibility now.

> > +.TP
> > +.B EFSCORRUPTED
> > +Severe filesystem corruption was detected and could not be repaired.
> > +Unmount the filesystem and run
> > +.B xfs_repair
> > +to fix the filesystem.
> 
> Is it always severe?  Does this only happen in response to IFLAG_REPAIR?
> (same goes for other return values, I think many are only in response
> to a repair request?  Might be worth noting)

EFSCORRUPTED means that we tried to rebuild the metadata from other
metadata, but iterating the other metadata produced EFSCORRUPTED so that
means two things are toast and we can't deal with that.

Also I thought "could not be repaired" sufficiently implied that this
only results from repair operations...

> 
> > +.TP
> > +.B EINVAL
> > +One or more of the arguments specified is invalid.
> > +.TP
> > +.B ENOENT
> > +The specified metadata object does not exist.
> > +For example, this error code is returned for a
> > +.B XFS_SCRUB_TYPE_REFCNTBT
> > +request on a filesystem that does not support reflink.
> > +.TP
> > +.B ENOMEM
> > +There was not sufficient memory to perform the scrub or repair operation.
> > +Some operations (most notably reference count checking) require a lot of
> > +memory.
> 
> may require large amounts of memory

Ok.

> > +.TP
> > +.B ENOSPC
> > +There is not enough free disk space to attempt a repair.
> > +.TP
> > +.B ENOTRECOVERABLE
> > +Filesystem was mounted in
> > +.B norecovery
> > +mode and therefore has an unclean log.
> 
> so it can't be checked, or can't be repaired?
> Is this returned for any ioctl on a norecovery mount, or only
> repair requests?

Both scrub and repair.

> > +.TP
> > +.B ENOTTY
> > +Online scrubbing or repair were not enabled.
> > +.TP
> > +.B EOPNOTSUPP
> > +Repairs of the requested metadata object are not supported.
> > +.TP
> > +.B EROFS
> > +Filesystem is read-only and a repair was requested.
> > +.TP
> > +.B ESHUTDOWN;
> 
> lose ";"

Ok.

> > +Filesystem is shut down due to previous errors.
> > +.SH CONFORMING TO
> > +This API is specific to XFS on Linux.
> 
> to the XFS filesystem on the Linux kernel.

Yep.

> > +.SH NOTES
> > +These operations may tie up the filesystem for a long time.
> 
> again, colloquial?

Yep.

> > +A calling process can be stop the operation by being sent a fatal
> 
> word salad :D

'can stop'

> > +signal, but non-fatal signals are blocked.
> > +.SH SEE ALSO
> > +.BR ioctl (2)
> 
> xfs_repair (8)

Might as well add xfs_scrub too.

--D

> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 4/5] xfs_io: provide an interface to the scrub ioctls
  2017-11-17 19:57 ` [PATCH 4/5] xfs_io: provide an interface to the scrub ioctls Darrick J. Wong
  2017-12-01  3:46   ` Eric Sandeen
  2017-12-01 16:06   ` Eric Sandeen
@ 2017-12-01 19:02   ` Darrick J. Wong
  2017-12-04 21:19     ` Eric Sandeen
  2 siblings, 1 reply; 26+ messages in thread
From: Darrick J. Wong @ 2017-12-01 19:02 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a new xfs_io command to call the new XFS metadata scrub ioctl.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 io/Makefile       |    3 -
 io/init.c         |    1 
 io/io.h           |    2 
 io/scrub.c        |  252 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 man/man8/xfs_io.8 |    8 ++
 5 files changed, 265 insertions(+), 1 deletion(-)
 create mode 100644 io/scrub.c

diff --git a/io/Makefile b/io/Makefile
index 6725936..58ae298 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -11,7 +11,8 @@ HFILES = init.h io.h
 CFILES = init.c \
 	attr.c bmap.c cowextsize.c encrypt.c file.c freeze.c fsync.c \
 	getrusage.c imap.c link.c mmap.c open.c parent.c pread.c prealloc.c \
-	pwrite.c reflink.c seek.c shutdown.c stat.c sync.c truncate.c utimes.c
+	pwrite.c reflink.c scrub.c seek.c shutdown.c stat.c sync.c truncate.c \
+	utimes.c
 
 LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD)
 LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE)
diff --git a/io/init.c b/io/init.c
index 20d5f80..9e576fe 100644
--- a/io/init.c
+++ b/io/init.c
@@ -84,6 +84,7 @@ init_commands(void)
 	readdir_init();
 	reflink_init();
 	resblks_init();
+	scrub_init();
 	seek_init();
 	sendfile_init();
 	shutdown_init();
diff --git a/io/io.h b/io/io.h
index 3862985..82e142f 100644
--- a/io/io.h
+++ b/io/io.h
@@ -185,3 +185,5 @@ extern void		fsmap_init(void);
 #else
 # define fsmap_init()	do { } while (0)
 #endif
+
+extern void		scrub_init(void);
diff --git a/io/scrub.c b/io/scrub.c
new file mode 100644
index 0000000..cd26997
--- /dev/null
+++ b/io/scrub.c
@@ -0,0 +1,252 @@
+/*
+ * Copyright (C) 2017 Oracle.  All Rights Reserved.
+ *
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ *
+ * 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,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include <sys/uio.h>
+#include <xfs/xfs.h>
+#include "command.h"
+#include "input.h"
+#include "init.h"
+#include "path.h"
+#include "io.h"
+
+static struct cmdinfo scrub_cmd;
+
+/* Type info and names for the scrub types. */
+enum scrub_type {
+	ST_NONE,	/* disabled */
+	ST_PERAG,	/* per-AG metadata */
+	ST_FS,		/* per-FS metadata */
+	ST_INODE,	/* per-inode metadata */
+};
+
+struct scrub_descr {
+	const char	*name;
+	enum scrub_type	type;
+};
+
+static const struct scrub_descr scrubbers[XFS_SCRUB_TYPE_NR] = {
+	[XFS_SCRUB_TYPE_PROBE]		= {"probe",		ST_NONE},
+	[XFS_SCRUB_TYPE_SB]		= {"sb",		ST_PERAG},
+	[XFS_SCRUB_TYPE_AGF]		= {"agf",		ST_PERAG},
+	[XFS_SCRUB_TYPE_AGFL]		= {"agfl",		ST_PERAG},
+	[XFS_SCRUB_TYPE_AGI]		= {"agi",		ST_PERAG},
+	[XFS_SCRUB_TYPE_BNOBT]		= {"bnobt",		ST_PERAG},
+	[XFS_SCRUB_TYPE_CNTBT]		= {"cntbt",		ST_PERAG},
+	[XFS_SCRUB_TYPE_INOBT]		= {"inobt",		ST_PERAG},
+	[XFS_SCRUB_TYPE_FINOBT]		= {"finobt",		ST_PERAG},
+	[XFS_SCRUB_TYPE_RMAPBT]		= {"rmapbt",		ST_PERAG},
+	[XFS_SCRUB_TYPE_REFCNTBT]	= {"refcountbt",	ST_PERAG},
+	[XFS_SCRUB_TYPE_INODE]		= {"inode",		ST_INODE},
+	[XFS_SCRUB_TYPE_BMBTD]		= {"bmapbtd",		ST_INODE},
+	[XFS_SCRUB_TYPE_BMBTA]		= {"bmapbta",		ST_INODE},
+	[XFS_SCRUB_TYPE_BMBTC]		= {"bmapbtc",		ST_INODE},
+	[XFS_SCRUB_TYPE_DIR]		= {"directory",		ST_INODE},
+	[XFS_SCRUB_TYPE_XATTR]		= {"xattr",		ST_INODE},
+	[XFS_SCRUB_TYPE_SYMLINK]	= {"symlink",		ST_INODE},
+	[XFS_SCRUB_TYPE_PARENT]		= {"parent",		ST_INODE},
+	[XFS_SCRUB_TYPE_RTBITMAP]	= {"rtbitmap",		ST_FS},
+	[XFS_SCRUB_TYPE_RTSUM]		= {"rtsummary",		ST_FS},
+	[XFS_SCRUB_TYPE_UQUOTA]		= {"usrquota",		ST_FS},
+	[XFS_SCRUB_TYPE_GQUOTA]		= {"grpquota",		ST_FS},
+	[XFS_SCRUB_TYPE_PQUOTA]		= {"prjquota",		ST_FS},
+};
+
+static void
+scrub_help(void)
+{
+	const struct scrub_descr	*d;
+	int				i;
+
+	printf(_(
+"\n"
+" Scrubs a piece of XFS filesystem metadata.  The first argument is the type\n"
+" of metadata to examine.  Allocation group metadata types take one AG number\n"
+" as the second parameter.  Inode metadata types act on the currently open file\n"
+" or (optionally) take an inode number and generation number to act upon as\n"
+" the second and third parameters.\n"
+"\n"
+" Example:\n"
+" 'scrub inobt 3' - scrub the inode btree in AG 3.\n"
+" 'scrub bmapbtd 128 13525' - scrubs the extent map of inode 128 gen 13525.\n"
+"\n"
+" Known metadata scrub types are:"));
+	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++)
+		printf(" %s", d->name);
+	printf("\n");
+}
+
+static void
+scrub_ioctl(
+	int				fd,
+	int				type,
+	uint64_t			control,
+	uint32_t			control2)
+{
+	struct xfs_scrub_metadata	meta;
+	const struct scrub_descr	*sc;
+	int				error;
+
+	sc = &scrubbers[type];
+	memset(&meta, 0, sizeof(meta));
+	meta.sm_type = type;
+	switch (sc->type) {
+	case ST_PERAG:
+		meta.sm_agno = control;
+		break;
+	case ST_INODE:
+		meta.sm_ino = control;
+		meta.sm_gen = control2;
+		break;
+	case ST_NONE:
+	case ST_FS:
+		/* no control parameters */
+		break;
+	}
+	meta.sm_flags = 0;
+
+	error = ioctl(fd, XFS_IOC_SCRUB_METADATA, &meta);
+	if (error)
+		perror("scrub");
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
+		printf(_("Corruption detected.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_PREEN)
+		printf(_("Optimization possible.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_XFAIL)
+		printf(_("Cross-referencing failed.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_XCORRUPT)
+		printf(_("Corruption detected during cross-referencing.\n"));
+	if (meta.sm_flags & XFS_SCRUB_OFLAG_INCOMPLETE)
+		printf(_("Scan was not complete.\n"));
+}
+
+static int
+parse_args(
+	int				argc,
+	char				**argv,
+	struct cmdinfo			*cmdinfo,
+	void				(*fn)(int, int, uint64_t, uint32_t))
+{
+	char				*p;
+	int				type = -1;
+	int				i, c;
+	uint64_t			control = 0;
+	uint32_t			control2 = 0;
+	const struct scrub_descr	*d = NULL;
+
+	while ((c = getopt(argc, argv, "")) != EOF) {
+		switch (c) {
+		default:
+			return command_usage(cmdinfo);
+		}
+	}
+	if (optind > argc - 1)
+		return command_usage(cmdinfo);
+
+	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++) {
+		if (strcmp(d->name, argv[optind]) == 0) {
+			type = i;
+			break;
+		}
+	}
+	optind++;
+
+	if (type < 0) {
+		printf(_("Unknown type '%s'.\n"), argv[optind]);
+		return command_usage(cmdinfo);
+	}
+
+	switch (d->type) {
+	case ST_INODE:
+		if (optind == argc) {
+			control = 0;
+			control2 = 0;
+		} else if (optind == argc - 2) {
+			control = strtoull(argv[optind], &p, 0);
+			if (*p != '\0') {
+				fprintf(stderr,
+					_("Bad inode number '%s'.\n"),
+					argv[optind]);
+				return 0;
+			}
+			control2 = strtoul(argv[optind + 1], &p, 0);
+			if (*p != '\0') {
+				fprintf(stderr,
+					_("Bad generation number '%s'.\n"),
+					argv[optind + 1]);
+				return 0;
+			}
+		} else {
+			fprintf(stderr,
+				_("Must specify inode number and generation.\n"));
+			return 0;
+		}
+		break;
+	case ST_PERAG:
+		if (optind != argc - 1) {
+			fprintf(stderr,
+				_("Must specify one AG number.\n"));
+			return 0;
+		}
+		control = strtoul(argv[optind], &p, 0);
+		if (*p != '\0') {
+			fprintf(stderr,
+				_("Bad AG number '%s'.\n"), argv[optind]);
+			return 0;
+		}
+		break;
+	case ST_FS:
+	case ST_NONE:
+		if (optind != argc) {
+			fprintf(stderr,
+				_("No parameters allowed.\n"));
+			return 0;
+		}
+		break;
+	default:
+		ASSERT(0);
+		break;
+	}
+	fn(file->fd, type, control, control2);
+
+	return 0;
+}
+
+static int
+scrub_f(
+	int				argc,
+	char				**argv)
+{
+	return parse_args(argc, argv, &scrub_cmd, scrub_ioctl);
+}
+
+void
+scrub_init(void)
+{
+	scrub_cmd.name = "scrub";
+	scrub_cmd.altname = "sc";
+	scrub_cmd.cfunc = scrub_f;
+	scrub_cmd.argmin = 1;
+	scrub_cmd.argmax = -1;
+	scrub_cmd.flags = CMD_NOMAP_OK;
+	scrub_cmd.args = _("type [agno|ino gen]");
+	scrub_cmd.oneline = _("scrubs filesystem metadata");
+	scrub_cmd.help = scrub_help;
+
+	add_command(&scrub_cmd);
+}
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 9bf1a47..2c594b1 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1119,6 +1119,14 @@ version of policy structure (numeric)
 .BR get_encpolicy
 On filesystems that support encryption, display the encryption policy of the
 current file.
+.TP
+.BI "scrub " type " [ " agnumber " | " "ino" " " "gen" " ]"
+Scrub internal XFS filesystem metadata.  The
+.BI type
+parameter specifies which type of metadata to scrub.
+For AG metadata, one AG number must be specified.
+For file metadata, the scrub is applied to the open file unless the
+inode number and generation number are specified.
 
 .SH SEE ALSO
 .BR mkfs.xfs (8),

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 5/5] man: describe the metadata scrubbing ioctl
  2017-11-17 19:57 ` [PATCH 5/5] man: describe the metadata scrubbing ioctl Darrick J. Wong
  2017-12-01  4:34   ` Eric Sandeen
@ 2017-12-01 19:03   ` Darrick J. Wong
  2017-12-04 21:19     ` Eric Sandeen
  1 sibling, 1 reply; 26+ messages in thread
From: Darrick J. Wong @ 2017-12-01 19:03 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Document the XFS-specific metadata scrub/repair ioctl's behavior,
arguments, and side effects.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 man/Makefile                        |    2 
 man/man2/Makefile                   |   22 ++
 man/man2/ioctl_xfs_scrub_metadata.2 |  318 +++++++++++++++++++++++++++++++++++
 3 files changed, 341 insertions(+), 1 deletion(-)
 create mode 100644 man/man2/Makefile
 create mode 100644 man/man2/ioctl_xfs_scrub_metadata.2

diff --git a/man/Makefile b/man/Makefile
index 863284c..cae891f 100644
--- a/man/Makefile
+++ b/man/Makefile
@@ -5,7 +5,7 @@
 TOPDIR = ..
 include $(TOPDIR)/include/builddefs
 
-SUBDIRS = man3 man5 man8
+SUBDIRS = man2 man3 man5 man8
 
 default : $(SUBDIRS)
 
diff --git a/man/man2/Makefile b/man/man2/Makefile
new file mode 100644
index 0000000..8aecde3
--- /dev/null
+++ b/man/man2/Makefile
@@ -0,0 +1,22 @@
+#
+# Copyright (c) 2000-2001 Silicon Graphics, Inc.  All Rights Reserved.
+#
+
+TOPDIR = ../..
+include $(TOPDIR)/include/builddefs
+
+MAN_SECTION	= 2
+
+MAN_PAGES	= $(shell echo *.$(MAN_SECTION))
+MAN_DEST	= $(PKG_MAN_DIR)/man$(MAN_SECTION)
+LSRCFILES	= $(MAN_PAGES)
+
+default : $(MAN_PAGES)
+
+include $(BUILDRULES)
+
+install :
+
+install-dev : default
+	$(INSTALL) -m 755 -d $(MAN_DEST)
+	$(INSTALL_MAN)
diff --git a/man/man2/ioctl_xfs_scrub_metadata.2 b/man/man2/ioctl_xfs_scrub_metadata.2
new file mode 100644
index 0000000..a0403e6
--- /dev/null
+++ b/man/man2/ioctl_xfs_scrub_metadata.2
@@ -0,0 +1,318 @@
+.\" Copyright (c) 2017, Oracle.  All rights reserved.
+.\"
+.\" %%%LICENSE_START(GPLv2+_DOC_FULL)
+.\" This is free documentation; 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.
+.\"
+.\" The GNU General Public License's references to "object code"
+.\" and "executables" are to be interpreted as the output of any
+.\" document formatting or typesetting system, including
+.\" intermediate and printed output.
+.\"
+.\" This manual is distributed in the hope that it will 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 manual; if not, see
+.\" <http://www.gnu.org/licenses/>.
+.\" %%%LICENSE_END
+.TH IOCTL-XFS-SCRUB-METADATA 2 2017-12-01 "XFS"
+.SH NAME
+ioctl_xfs_scrub_metadata \- check XFS filesystem metadata
+.SH SYNOPSIS
+.br
+.B #include <xfs/xfs_fs.h>
+.PP
+.BI "int ioctl(int " dest_fd ", XFS_IOC_SCRUB_METADATA, struct xfs_scrub_metadata *" arg );
+.SH DESCRIPTION
+This XFS ioctl asks the kernel driver to examine a piece of filesystem
+metadata for errors or suboptimal metadata.
+Examination includes running metadata verifiers, checking records
+for obviously incorrect or impossible values, and cross-referencing each
+record with any other available metadata in the filesystem.
+This ioctl can also try to repair or optimize metadata, though this may
+block normal filesystem operations for a long period of time.
+The type and location of the metadata to scrub is conveyed in a structure
+of the following form:
+.PP
+.in +4n
+.nf
+struct xfs_scrub_metadata {
+	__u32 sm_type;
+	__u32 sm_flags;
+	__u64 sm_ino;
+	__u32 sm_gen;
+	__u32 sm_agno;
+	__u64 sm_reserved[5];
+};
+.fi
+.in
+.PP
+The field
+.I sm_reserved
+must be zero.
+.PP
+The field
+.I sm_type
+indicates the type of metadata to check:
+.RS 0.4i
+.TP
+.B XFS_SCRUB_TYPE_PROBE
+Probe the kernel to see if it is willing to try to check or repair this
+filesystem.
+.BR sm_agno ", " sm_ino ", and " sm_gen
+must be zero.
+
+.PD 0
+.PP
+.nf
+.B XFS_SCRUB_TYPE_SB
+.B XFS_SCRUB_TYPE_AGF
+.B XFS_SCRUB_TYPE_AGFL
+.fi
+.TP
+.B XFS_SCRUB_TYPE_AGI
+Examine a given allocation group's superblock, free space header, free
+block list, or inode header, respectively.
+Headers are checked for obviously incorrect values and cross-referenced
+against the allocation group's metadata btrees, if possible.
+The allocation group number must be given in
+.BR sm_agno "."
+.BR sm_ino " and " sm_gen
+must be zero.
+
+.PP
+.nf
+.B XFS_SCRUB_TYPE_BNOBT
+.B XFS_SCRUB_TYPE_CNTBT
+.B XFS_SCRUB_TYPE_INOBT
+.B XFS_SCRUB_TYPE_FINOBT
+.B XFS_SCRUB_TYPE_RMAPBT
+.fi
+.TP
+.B XFS_SCRUB_TYPE_REFCNTBT
+Examine a given allocation group's two free space btrees, two inode
+btrees, reverse mapping btrees, or reference count btrees, respectively.
+Records are checked for obviously incorrect values and cross-referenced
+with other allocation group metadata records to ensure that there are no
+conflicts.
+The allocation group number must be given in
+.BR sm_agno "."
+.BR sm_ino " and " sm_gen
+must be zero.
+
+.TP
+.B XFS_SCRUB_TYPE_INODE
+Examine a given inode record for obviously incorrect values and
+discrepancies with the rest of filesystem metadata.
+Parent pointers are checked for impossible inode values and are then
+followed up to the parent directory to ensure that the linkage is
+correct.
+The inode to examine may be specified either through
+.B sm_ino
+and
+.BR sm_gen "; "
+if not specified, then the file described by
+.B dest_fd
+will be examined.
+.B sm_agno
+must be zero.
+
+.PP
+.nf
+.B XFS_SCRUB_TYPE_BMBTD
+.B XFS_SCRUB_TYPE_BMBTA
+.B XFS_SCRUB_TYPE_BMBTC
+.fi
+.TP
+.B XFS_SCRUB_TYPE_PARENT
+Examine a given inode's data block map, extended attribute block map,
+copy on write block map, or parent inode pointer.
+Inode records are examined for obviously incorrect values and
+discrepancies with the three block map types.
+The block maps are checked for obviously wrong values and
+cross-referenced with the allocation group space extent metadata for
+discrepancies.
+The inode to examine can be specified in the same manner as
+.BR XFS_SCRUB_TYPE_INODE "."
+
+.TP
+.B XFS_SCRUB_TYPE_XATTR
+Examine the extended attribute records and indices of a given inode for
+incorrect pointers and other signs of damage.
+The inode to examine can be specified in the same manner as
+.BR XFS_SCRUB_TYPE_INODE "."
+
+.TP
+.B XFS_SCRUB_TYPE_DIR
+Examine the entries in a given directory for invalid data or dangling pointers.
+The directory to examine can be specified in the same manner as
+.BR XFS_SCRUB_TYPE_INODE "."
+
+.TP
+.B XFS_SCRUB_TYPE_SYMLINK
+Examine the target of a symbolic link for obvious pathname problems.
+The link to examine can be specified in the same manner as
+.BR XFS_SCRUB_TYPE_INODE "."
+
+.PP
+.nf
+.B XFS_SCRUB_TYPE_RTBITMAP
+.fi
+.TP
+.B XFS_SCRUB_TYPE_RTSUM
+Examine the realtime block bitmap and realtime summary inodes for
+corruption.
+
+.PP
+.nf
+.B XFS_SCRUB_TYPE_UQUOTA
+.B XFS_SCRUB_TYPE_GQUOTA
+.fi
+.TP
+.B XFS_SCRUB_TYPE_PQUOTA
+Examine all user, group, or project quota records for corruption.
+.RE
+
+.PD 1
+.PP
+The field
+.I sm_flags
+control the behavior of the scrub operation and provide more information
+about the outcome of the operation.
+If none of the
+.B XFS_SCRUB_OFLAG_*
+flags are set upon return, the metadata is clean.
+.RS 0.4i
+.TP
+.B XFS_SCRUB_IFLAG_REPAIR
+If the caller sets this flag, the kernel driver will examine the
+metadata and try to fix all problems and to optimize metadata when
+possible.
+If no errors occur during the repair operation, the check is performed a
+second time to determine whether the repair succeeded.
+If errors occur, the call returns an error status immediately.
+.TP
+.B XFS_SCRUB_OFLAG_CORRUPT
+The metadata was corrupt when the call returned.
+If
+.B XFS_SCRUB_IFLAG_REPAIR
+was specified, then an attempted repair failed to fix the problem.
+Unmount the filesystem and run
+.B xfs_repair
+to fix the filesystem.
+.TP
+.B XFS_SCRUB_OFLAG_PREEN
+The metadata is ok, but some aspect of the metadata could be optimized
+to increase performance.
+Call again with
+.B XFS_SCRUB_IFLAG_REPAIR
+to optimize the metadata.
+.TP
+.B XFS_SCRUB_OFLAG_XFAIL
+Filesystem errors were encountered when accessing other metadata to
+cross-reference the records attached to this metadata object.
+.TP
+.B XFS_SCRUB_OFLAG_XCORRUPT
+Discrepancies were found when cross-referencing the records attached to
+this metadata object against all other available metadata in the system.
+.TP
+.B XFS_SCRUB_OFLAG_INCOMPLETE
+The checker was unable to complete its check of all records.
+.TP
+.B XFS_SCRUB_OFLAG_WARNING
+The checker encountered a metadata object with potentially problematic
+records.
+However, the records were not obviously corrupt.
+.RE
+.PP
+For metadata checkers that operate on inodes or inode metadata, the fields
+.IR sm_ino " and " sm_gen
+are the inode number and generation number of the inode to check.
+If the inode number is zero, the inode represented by
+.I dest_fd
+is used instead.
+If the generation number of the inode does not match
+.IR sm_gen ", "
+the call will return an error code for the invalid argument.
+The
+.I sm_agno
+field must be zero.
+.PP
+For metadata checkers that operate on allocation group metadata, the field
+.I sm_agno
+indicates the allocation group in which to find the metadata.
+The
+.IR sm_ino " and " sm_gen
+fields must be zero.
+.PP
+For metadata checkers that operate on filesystem-wide metadata, no
+further arguments are required.
+.IR sm_agno ", " sm_ino ", and " sm_gen
+must all be zero.
+.SH RETURN VALUE
+On error, \-1 is returned, and
+.I errno
+is set to indicate the error.
+.PP
+.SH ERRORS
+Error codes can be one of, but are not limited to, the following:
+.TP
+.B EBUSY
+The filesystem object is busy; the operation will have to be tried again.
+.TP
+.B EFSCORRUPTED
+Severe filesystem corruption was detected and could not be repaired.
+Unmount the filesystem and run
+.B xfs_repair
+to fix the filesystem.
+.TP
+.B EINVAL
+One or more of the arguments specified is invalid.
+.TP
+.B ENOENT
+The specified metadata object does not exist.
+For example, this error code is returned for a
+.B XFS_SCRUB_TYPE_REFCNTBT
+request on a filesystem that does not support reflink.
+.TP
+.B ENOMEM
+There was not sufficient memory to perform the scrub or repair operation.
+Some operations (most notably reference count checking) require large
+amounts of memory.
+.TP
+.B ENOSPC
+There is not enough free disk space to attempt a repair.
+.TP
+.B ENOTRECOVERABLE
+Filesystem was mounted in
+.B norecovery
+mode and therefore has an unclean log.
+Neither scrub nor repair operations can be attempted with an unclean
+log.
+.TP
+.B ENOTTY
+Online scrubbing or repair were not enabled.
+.TP
+.B EOPNOTSUPP
+Repairs of the requested metadata object are not supported.
+.TP
+.B EROFS
+Filesystem is read-only and a repair was requested.
+.TP
+.B ESHUTDOWN
+Filesystem is shut down due to previous errors.
+.SH CONFORMING TO
+This API is specific to XFS filesystem on the Linux kernel.
+.SH NOTES
+These operations may block other filesystem operations for a long time.
+A calling process can stop the operation by being sent a fatal
+signal, but non-fatal signals are blocked.
+.SH SEE ALSO
+.BR ioctl (2)
+.BR xfs_scrub (8)
+.BR xfs_repair (8)

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 4/5] xfs_io: provide an interface to the scrub ioctls
  2017-12-01 19:02   ` [PATCH v2 " Darrick J. Wong
@ 2017-12-04 21:19     ` Eric Sandeen
  0 siblings, 0 replies; 26+ messages in thread
From: Eric Sandeen @ 2017-12-04 21:19 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs

On 12/1/17 1:02 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Create a new xfs_io command to call the new XFS metadata scrub ioctl.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

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

> ---
>  io/Makefile       |    3 -
>  io/init.c         |    1 
>  io/io.h           |    2 
>  io/scrub.c        |  252 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  man/man8/xfs_io.8 |    8 ++
>  5 files changed, 265 insertions(+), 1 deletion(-)
>  create mode 100644 io/scrub.c
> 
> diff --git a/io/Makefile b/io/Makefile
> index 6725936..58ae298 100644
> --- a/io/Makefile
> +++ b/io/Makefile
> @@ -11,7 +11,8 @@ HFILES = init.h io.h
>  CFILES = init.c \
>  	attr.c bmap.c cowextsize.c encrypt.c file.c freeze.c fsync.c \
>  	getrusage.c imap.c link.c mmap.c open.c parent.c pread.c prealloc.c \
> -	pwrite.c reflink.c seek.c shutdown.c stat.c sync.c truncate.c utimes.c
> +	pwrite.c reflink.c scrub.c seek.c shutdown.c stat.c sync.c truncate.c \
> +	utimes.c
>  
>  LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD)
>  LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE)
> diff --git a/io/init.c b/io/init.c
> index 20d5f80..9e576fe 100644
> --- a/io/init.c
> +++ b/io/init.c
> @@ -84,6 +84,7 @@ init_commands(void)
>  	readdir_init();
>  	reflink_init();
>  	resblks_init();
> +	scrub_init();
>  	seek_init();
>  	sendfile_init();
>  	shutdown_init();
> diff --git a/io/io.h b/io/io.h
> index 3862985..82e142f 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -185,3 +185,5 @@ extern void		fsmap_init(void);
>  #else
>  # define fsmap_init()	do { } while (0)
>  #endif
> +
> +extern void		scrub_init(void);
> diff --git a/io/scrub.c b/io/scrub.c
> new file mode 100644
> index 0000000..cd26997
> --- /dev/null
> +++ b/io/scrub.c
> @@ -0,0 +1,252 @@
> +/*
> + * Copyright (C) 2017 Oracle.  All Rights Reserved.
> + *
> + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> + *
> + * 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,
> + * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + */
> +
> +#include <sys/uio.h>
> +#include <xfs/xfs.h>
> +#include "command.h"
> +#include "input.h"
> +#include "init.h"
> +#include "path.h"
> +#include "io.h"
> +
> +static struct cmdinfo scrub_cmd;
> +
> +/* Type info and names for the scrub types. */
> +enum scrub_type {
> +	ST_NONE,	/* disabled */
> +	ST_PERAG,	/* per-AG metadata */
> +	ST_FS,		/* per-FS metadata */
> +	ST_INODE,	/* per-inode metadata */
> +};
> +
> +struct scrub_descr {
> +	const char	*name;
> +	enum scrub_type	type;
> +};
> +
> +static const struct scrub_descr scrubbers[XFS_SCRUB_TYPE_NR] = {
> +	[XFS_SCRUB_TYPE_PROBE]		= {"probe",		ST_NONE},
> +	[XFS_SCRUB_TYPE_SB]		= {"sb",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_AGF]		= {"agf",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_AGFL]		= {"agfl",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_AGI]		= {"agi",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_BNOBT]		= {"bnobt",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_CNTBT]		= {"cntbt",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_INOBT]		= {"inobt",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_FINOBT]		= {"finobt",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_RMAPBT]		= {"rmapbt",		ST_PERAG},
> +	[XFS_SCRUB_TYPE_REFCNTBT]	= {"refcountbt",	ST_PERAG},
> +	[XFS_SCRUB_TYPE_INODE]		= {"inode",		ST_INODE},
> +	[XFS_SCRUB_TYPE_BMBTD]		= {"bmapbtd",		ST_INODE},
> +	[XFS_SCRUB_TYPE_BMBTA]		= {"bmapbta",		ST_INODE},
> +	[XFS_SCRUB_TYPE_BMBTC]		= {"bmapbtc",		ST_INODE},
> +	[XFS_SCRUB_TYPE_DIR]		= {"directory",		ST_INODE},
> +	[XFS_SCRUB_TYPE_XATTR]		= {"xattr",		ST_INODE},
> +	[XFS_SCRUB_TYPE_SYMLINK]	= {"symlink",		ST_INODE},
> +	[XFS_SCRUB_TYPE_PARENT]		= {"parent",		ST_INODE},
> +	[XFS_SCRUB_TYPE_RTBITMAP]	= {"rtbitmap",		ST_FS},
> +	[XFS_SCRUB_TYPE_RTSUM]		= {"rtsummary",		ST_FS},
> +	[XFS_SCRUB_TYPE_UQUOTA]		= {"usrquota",		ST_FS},
> +	[XFS_SCRUB_TYPE_GQUOTA]		= {"grpquota",		ST_FS},
> +	[XFS_SCRUB_TYPE_PQUOTA]		= {"prjquota",		ST_FS},
> +};
> +
> +static void
> +scrub_help(void)
> +{
> +	const struct scrub_descr	*d;
> +	int				i;
> +
> +	printf(_(
> +"\n"
> +" Scrubs a piece of XFS filesystem metadata.  The first argument is the type\n"
> +" of metadata to examine.  Allocation group metadata types take one AG number\n"
> +" as the second parameter.  Inode metadata types act on the currently open file\n"
> +" or (optionally) take an inode number and generation number to act upon as\n"
> +" the second and third parameters.\n"
> +"\n"
> +" Example:\n"
> +" 'scrub inobt 3' - scrub the inode btree in AG 3.\n"
> +" 'scrub bmapbtd 128 13525' - scrubs the extent map of inode 128 gen 13525.\n"
> +"\n"
> +" Known metadata scrub types are:"));
> +	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++)
> +		printf(" %s", d->name);
> +	printf("\n");
> +}
> +
> +static void
> +scrub_ioctl(
> +	int				fd,
> +	int				type,
> +	uint64_t			control,
> +	uint32_t			control2)
> +{
> +	struct xfs_scrub_metadata	meta;
> +	const struct scrub_descr	*sc;
> +	int				error;
> +
> +	sc = &scrubbers[type];
> +	memset(&meta, 0, sizeof(meta));
> +	meta.sm_type = type;
> +	switch (sc->type) {
> +	case ST_PERAG:
> +		meta.sm_agno = control;
> +		break;
> +	case ST_INODE:
> +		meta.sm_ino = control;
> +		meta.sm_gen = control2;
> +		break;
> +	case ST_NONE:
> +	case ST_FS:
> +		/* no control parameters */
> +		break;
> +	}
> +	meta.sm_flags = 0;
> +
> +	error = ioctl(fd, XFS_IOC_SCRUB_METADATA, &meta);
> +	if (error)
> +		perror("scrub");
> +	if (meta.sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
> +		printf(_("Corruption detected.\n"));
> +	if (meta.sm_flags & XFS_SCRUB_OFLAG_PREEN)
> +		printf(_("Optimization possible.\n"));
> +	if (meta.sm_flags & XFS_SCRUB_OFLAG_XFAIL)
> +		printf(_("Cross-referencing failed.\n"));
> +	if (meta.sm_flags & XFS_SCRUB_OFLAG_XCORRUPT)
> +		printf(_("Corruption detected during cross-referencing.\n"));
> +	if (meta.sm_flags & XFS_SCRUB_OFLAG_INCOMPLETE)
> +		printf(_("Scan was not complete.\n"));
> +}
> +
> +static int
> +parse_args(
> +	int				argc,
> +	char				**argv,
> +	struct cmdinfo			*cmdinfo,
> +	void				(*fn)(int, int, uint64_t, uint32_t))
> +{
> +	char				*p;
> +	int				type = -1;
> +	int				i, c;
> +	uint64_t			control = 0;
> +	uint32_t			control2 = 0;
> +	const struct scrub_descr	*d = NULL;
> +
> +	while ((c = getopt(argc, argv, "")) != EOF) {
> +		switch (c) {
> +		default:
> +			return command_usage(cmdinfo);
> +		}
> +	}
> +	if (optind > argc - 1)
> +		return command_usage(cmdinfo);
> +
> +	for (i = 0, d = scrubbers; i < XFS_SCRUB_TYPE_NR; i++, d++) {
> +		if (strcmp(d->name, argv[optind]) == 0) {
> +			type = i;
> +			break;
> +		}
> +	}
> +	optind++;
> +
> +	if (type < 0) {
> +		printf(_("Unknown type '%s'.\n"), argv[optind]);
> +		return command_usage(cmdinfo);
> +	}
> +
> +	switch (d->type) {
> +	case ST_INODE:
> +		if (optind == argc) {
> +			control = 0;
> +			control2 = 0;
> +		} else if (optind == argc - 2) {
> +			control = strtoull(argv[optind], &p, 0);
> +			if (*p != '\0') {
> +				fprintf(stderr,
> +					_("Bad inode number '%s'.\n"),
> +					argv[optind]);
> +				return 0;
> +			}
> +			control2 = strtoul(argv[optind + 1], &p, 0);
> +			if (*p != '\0') {
> +				fprintf(stderr,
> +					_("Bad generation number '%s'.\n"),
> +					argv[optind + 1]);
> +				return 0;
> +			}
> +		} else {
> +			fprintf(stderr,
> +				_("Must specify inode number and generation.\n"));
> +			return 0;
> +		}
> +		break;
> +	case ST_PERAG:
> +		if (optind != argc - 1) {
> +			fprintf(stderr,
> +				_("Must specify one AG number.\n"));
> +			return 0;
> +		}
> +		control = strtoul(argv[optind], &p, 0);
> +		if (*p != '\0') {
> +			fprintf(stderr,
> +				_("Bad AG number '%s'.\n"), argv[optind]);
> +			return 0;
> +		}
> +		break;
> +	case ST_FS:
> +	case ST_NONE:
> +		if (optind != argc) {
> +			fprintf(stderr,
> +				_("No parameters allowed.\n"));
> +			return 0;
> +		}
> +		break;
> +	default:
> +		ASSERT(0);
> +		break;
> +	}
> +	fn(file->fd, type, control, control2);
> +
> +	return 0;
> +}
> +
> +static int
> +scrub_f(
> +	int				argc,
> +	char				**argv)
> +{
> +	return parse_args(argc, argv, &scrub_cmd, scrub_ioctl);
> +}
> +
> +void
> +scrub_init(void)
> +{
> +	scrub_cmd.name = "scrub";
> +	scrub_cmd.altname = "sc";
> +	scrub_cmd.cfunc = scrub_f;
> +	scrub_cmd.argmin = 1;
> +	scrub_cmd.argmax = -1;
> +	scrub_cmd.flags = CMD_NOMAP_OK;
> +	scrub_cmd.args = _("type [agno|ino gen]");
> +	scrub_cmd.oneline = _("scrubs filesystem metadata");
> +	scrub_cmd.help = scrub_help;
> +
> +	add_command(&scrub_cmd);
> +}
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 9bf1a47..2c594b1 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -1119,6 +1119,14 @@ version of policy structure (numeric)
>  .BR get_encpolicy
>  On filesystems that support encryption, display the encryption policy of the
>  current file.
> +.TP
> +.BI "scrub " type " [ " agnumber " | " "ino" " " "gen" " ]"
> +Scrub internal XFS filesystem metadata.  The
> +.BI type
> +parameter specifies which type of metadata to scrub.
> +For AG metadata, one AG number must be specified.
> +For file metadata, the scrub is applied to the open file unless the
> +inode number and generation number are specified.
>  
>  .SH SEE ALSO
>  .BR mkfs.xfs (8),
> --
> 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
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 5/5] man: describe the metadata scrubbing ioctl
  2017-12-01 19:03   ` [PATCH v2 " Darrick J. Wong
@ 2017-12-04 21:19     ` Eric Sandeen
  0 siblings, 0 replies; 26+ messages in thread
From: Eric Sandeen @ 2017-12-04 21:19 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs

On 12/1/17 1:03 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Document the XFS-specific metadata scrub/repair ioctl's behavior,
> arguments, and side effects.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/5 V2] xfs_db: print structure padding fields consistently
  2017-11-17 19:56 ` [PATCH 1/5] xfs_db: print structure padding fields consistently Darrick J. Wong
  2017-11-30 21:37   ` Eric Sandeen
@ 2017-12-04 23:54   ` Eric Sandeen
  2017-12-04 23:56     ` Darrick J. Wong
  1 sibling, 1 reply; 26+ messages in thread
From: Eric Sandeen @ 2017-12-04 23:54 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

We are very inconsistent about how we print padding fields in on-disk
structures -- sometimes we hide it from printall, sometimes we deviate
from unsigned hex values, etc.  Make this all consistent -- always hide
padding values when printing the whole structure, always print them as
unsigned hex integers when explicitly requested.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
[sandeen: switch to never-print instead of always-print]
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---

V2: constant-ify by never printing instead of always printing,
    but still make format consistent.

diff --git a/db/attr.c b/db/attr.c
index 75fe239..9cbb20d 100644
--- a/db/attr.c
+++ b/db/attr.c
@@ -589,7 +589,7 @@ const field_t	attr3_node_hdr_flds[] = {
 	{ "info", FLDT_ATTR3_BLKINFO, OI(H3OFF(info)), C1, 0, TYP_NONE },
 	{ "count", FLDT_UINT16D, OI(H3OFF(__count)), C1, 0, TYP_NONE },
 	{ "level", FLDT_UINT16D, OI(H3OFF(__level)), C1, 0, TYP_NONE },
-	{ "pad", FLDT_UINT32D, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
+	{ "pad", FLDT_UINT32X, OI(H3OFF(__pad32)), C1, FLD_SKIPALL, TYP_NONE },
 	{ NULL }
 };
 
diff --git a/db/dir2.c b/db/dir2.c
index 3e21a7b..7f7ea5a 100644
--- a/db/dir2.c
+++ b/db/dir2.c
@@ -977,7 +977,7 @@ const field_t	da3_node_hdr_flds[] = {
 	{ "info", FLDT_DA3_BLKINFO, OI(H3OFF(info)), C1, 0, TYP_NONE },
 	{ "count", FLDT_UINT16D, OI(H3OFF(__count)), C1, 0, TYP_NONE },
 	{ "level", FLDT_UINT16D, OI(H3OFF(__level)), C1, 0, TYP_NONE },
-	{ "pad", FLDT_UINT32D, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
+	{ "pad", FLDT_UINT32X, OI(H3OFF(__pad32)), C1, FLD_SKIPALL, TYP_NONE },
 	{ NULL }
 };
 
diff --git a/db/dquot.c b/db/dquot.c
index 4e35df4..4fd1289 100644
--- a/db/dquot.c
+++ b/db/dquot.c
@@ -76,7 +76,7 @@ const field_t	disk_dquot_flds[] = {
 	{ "btimer", FLDT_INT32D, OI(DOFF(btimer)), C1, 0, TYP_NONE },
 	{ "iwarns", FLDT_QWARNCNT, OI(DOFF(iwarns)), C1, 0, TYP_NONE },
 	{ "bwarns", FLDT_QWARNCNT, OI(DOFF(bwarns)), C1, 0, TYP_NONE },
-	{ "pad0", FLDT_INT32D, OI(DOFF(pad0)), C1, FLD_SKIPALL, TYP_NONE },
+	{ "pad0", FLDT_UINT32X, OI(DOFF(pad0)), C1, FLD_SKIPALL, TYP_NONE },
 	{ "rtb_hardlimit", FLDT_QCNT, OI(DOFF(rtb_hardlimit)), C1, 0,
 	  TYP_NONE },
 	{ "rtb_softlimit", FLDT_QCNT, OI(DOFF(rtb_softlimit)), C1, 0,


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/5 V2] xfs_db: add missing padding fields
  2017-11-17 19:56 ` [PATCH 2/5] xfs_db: add missing padding fields Darrick J. Wong
@ 2017-12-04 23:54   ` Eric Sandeen
  2017-12-04 23:56     ` Darrick J. Wong
  0 siblings, 1 reply; 26+ messages in thread
From: Eric Sandeen @ 2017-12-04 23:54 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Several data structures are missing padding fields from their field
definitions.  Add them so that they can be printed out if explicitly
requested.

Fix the AGI field order to be consistent with the structure
definition while we're at it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
[sandeen: make them available but not printed by default]
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---

v2: make available but not printed by default

diff --git a/db/agi.c b/db/agi.c
index fb69210..fab146e 100644
--- a/db/agi.c
+++ b/db/agi.c
@@ -55,8 +55,9 @@ const field_t	agi_flds[] = {
 	{ "unlinked", FLDT_AGINONN, OI(OFF(unlinked)),
 	  CI(XFS_AGI_UNLINKED_BUCKETS), FLD_ARRAY, TYP_NONE },
 	{ "uuid", FLDT_UUID, OI(OFF(uuid)), C1, 0, TYP_NONE },
-	{ "lsn", FLDT_UINT64X, OI(OFF(lsn)), C1, 0, TYP_NONE },
 	{ "crc", FLDT_CRC, OI(OFF(crc)), C1, 0, TYP_NONE },
+	{ "pad32", FLDT_UINT32X, OI(OFF(pad32)), C1, FLD_SKIPALL, TYP_NONE },
+	{ "lsn", FLDT_UINT64X, OI(OFF(lsn)), C1, 0, TYP_NONE },
 	{ "free_root", FLDT_AGBLOCK, OI(OFF(free_root)), C1, 0, TYP_FINOBT },
 	{ "free_level", FLDT_UINT32D, OI(OFF(free_level)), C1, 0, TYP_NONE },
 	{ NULL }
diff --git a/db/dir2.c b/db/dir2.c
index 7f7ea5a..0bd3437 100644
--- a/db/dir2.c
+++ b/db/dir2.c
@@ -940,6 +940,7 @@ const field_t	dir3_data_hdr_flds[] = {
 	{ "hdr", FLDT_DIR3_BLKHDR, OI(DH3OFF(hdr)), C1, 0, TYP_NONE },
 	{ "bestfree", FLDT_DIR2_DATA_FREE, OI(DH3OFF(best_free)),
 	  CI(XFS_DIR2_DATA_FD_COUNT), FLD_ARRAY, TYP_NONE },
+	{ "pad", FLDT_UINT32X, OI(DH3OFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
 	{ NULL }
 };
 
@@ -948,6 +949,7 @@ const field_t	dir3_leaf_hdr_flds[] = {
 	{ "info", FLDT_DA3_BLKINFO, OI(LH3OFF(info)), C1, 0, TYP_NONE },
 	{ "count", FLDT_UINT16D, OI(LH3OFF(count)), C1, 0, TYP_NONE },
 	{ "stale", FLDT_UINT16D, OI(LH3OFF(stale)), C1, 0, TYP_NONE },
+	{ "pad", FLDT_UINT32X, OI(LH3OFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
 	{ NULL }
 };
 
@@ -957,6 +959,7 @@ const field_t	dir3_free_hdr_flds[] = {
 	{ "firstdb", FLDT_INT32D, OI(FH3OFF(firstdb)), C1, 0, TYP_NONE },
 	{ "nvalid", FLDT_INT32D, OI(FH3OFF(nvalid)), C1, 0, TYP_NONE },
 	{ "nused", FLDT_INT32D, OI(FH3OFF(nused)), C1, 0, TYP_NONE },
+	{ "pad", FLDT_UINT32X, OI(FH3OFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
 	{ NULL }
 };
 
diff --git a/db/inode.c b/db/inode.c
index 6f971c6..b3f53d3 100644
--- a/db/inode.c
+++ b/db/inode.c
@@ -102,6 +102,7 @@ const field_t	inode_core_flds[] = {
 	  inode_core_projid_count, FLD_COUNT, TYP_NONE },
 	{ "projid_hi", FLDT_UINT16D, OI(COFF(projid_hi)),
 	  inode_core_projid_count, FLD_COUNT, TYP_NONE },
+	{ "pad", FLDT_UINT8X, OI(OFF(pad)), CI(6), FLD_ARRAY|FLD_SKIPALL, TYP_NONE },
 	{ "uid", FLDT_UINT32D, OI(COFF(uid)), C1, 0, TYP_NONE },
 	{ "gid", FLDT_UINT32D, OI(COFF(gid)), C1, 0, TYP_NONE },
 	{ "flushiter", FLDT_UINT16D, OI(COFF(flushiter)), C1, 0, TYP_NONE },
@@ -173,6 +174,7 @@ const field_t	inode_v3_flds[] = {
 	{ "lsn", FLDT_UINT64X, OI(COFF(lsn)), C1, 0, TYP_NONE },
 	{ "flags2", FLDT_UINT64X, OI(COFF(flags2)), C1, 0, TYP_NONE },
 	{ "cowextsize", FLDT_EXTLEN, OI(COFF(cowextsize)), C1, 0, TYP_NONE },
+	{ "pad2", FLDT_UINT8X, OI(OFF(pad2)), CI(12), FLD_ARRAY|FLD_SKIPALL, TYP_NONE },
 	{ "crtime", FLDT_TIMESTAMP, OI(COFF(crtime)), C1, 0, TYP_NONE },
 	{ "inumber", FLDT_INO, OI(COFF(ino)), C1, 0, TYP_NONE },
 	{ "uuid", FLDT_UUID, OI(COFF(uuid)), C1, 0, TYP_NONE },


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/5 V2] xfs_db: add missing padding fields
  2017-12-04 23:54   ` [PATCH 2/5 V2] " Eric Sandeen
@ 2017-12-04 23:56     ` Darrick J. Wong
  0 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2017-12-04 23:56 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: sandeen, linux-xfs

On Mon, Dec 04, 2017 at 05:54:44PM -0600, Eric Sandeen wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Several data structures are missing padding fields from their field
> definitions.  Add them so that they can be printed out if explicitly
> requested.
> 
> Fix the AGI field order to be consistent with the structure
> definition while we're at it.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> [sandeen: make them available but not printed by default]

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
> 
> v2: make available but not printed by default
> 
> diff --git a/db/agi.c b/db/agi.c
> index fb69210..fab146e 100644
> --- a/db/agi.c
> +++ b/db/agi.c
> @@ -55,8 +55,9 @@ const field_t	agi_flds[] = {
>  	{ "unlinked", FLDT_AGINONN, OI(OFF(unlinked)),
>  	  CI(XFS_AGI_UNLINKED_BUCKETS), FLD_ARRAY, TYP_NONE },
>  	{ "uuid", FLDT_UUID, OI(OFF(uuid)), C1, 0, TYP_NONE },
> -	{ "lsn", FLDT_UINT64X, OI(OFF(lsn)), C1, 0, TYP_NONE },
>  	{ "crc", FLDT_CRC, OI(OFF(crc)), C1, 0, TYP_NONE },
> +	{ "pad32", FLDT_UINT32X, OI(OFF(pad32)), C1, FLD_SKIPALL, TYP_NONE },
> +	{ "lsn", FLDT_UINT64X, OI(OFF(lsn)), C1, 0, TYP_NONE },
>  	{ "free_root", FLDT_AGBLOCK, OI(OFF(free_root)), C1, 0, TYP_FINOBT },
>  	{ "free_level", FLDT_UINT32D, OI(OFF(free_level)), C1, 0, TYP_NONE },
>  	{ NULL }
> diff --git a/db/dir2.c b/db/dir2.c
> index 7f7ea5a..0bd3437 100644
> --- a/db/dir2.c
> +++ b/db/dir2.c
> @@ -940,6 +940,7 @@ const field_t	dir3_data_hdr_flds[] = {
>  	{ "hdr", FLDT_DIR3_BLKHDR, OI(DH3OFF(hdr)), C1, 0, TYP_NONE },
>  	{ "bestfree", FLDT_DIR2_DATA_FREE, OI(DH3OFF(best_free)),
>  	  CI(XFS_DIR2_DATA_FD_COUNT), FLD_ARRAY, TYP_NONE },
> +	{ "pad", FLDT_UINT32X, OI(DH3OFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
>  	{ NULL }
>  };
>  
> @@ -948,6 +949,7 @@ const field_t	dir3_leaf_hdr_flds[] = {
>  	{ "info", FLDT_DA3_BLKINFO, OI(LH3OFF(info)), C1, 0, TYP_NONE },
>  	{ "count", FLDT_UINT16D, OI(LH3OFF(count)), C1, 0, TYP_NONE },
>  	{ "stale", FLDT_UINT16D, OI(LH3OFF(stale)), C1, 0, TYP_NONE },
> +	{ "pad", FLDT_UINT32X, OI(LH3OFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
>  	{ NULL }
>  };
>  
> @@ -957,6 +959,7 @@ const field_t	dir3_free_hdr_flds[] = {
>  	{ "firstdb", FLDT_INT32D, OI(FH3OFF(firstdb)), C1, 0, TYP_NONE },
>  	{ "nvalid", FLDT_INT32D, OI(FH3OFF(nvalid)), C1, 0, TYP_NONE },
>  	{ "nused", FLDT_INT32D, OI(FH3OFF(nused)), C1, 0, TYP_NONE },
> +	{ "pad", FLDT_UINT32X, OI(FH3OFF(pad)), C1, FLD_SKIPALL, TYP_NONE },
>  	{ NULL }
>  };
>  
> diff --git a/db/inode.c b/db/inode.c
> index 6f971c6..b3f53d3 100644
> --- a/db/inode.c
> +++ b/db/inode.c
> @@ -102,6 +102,7 @@ const field_t	inode_core_flds[] = {
>  	  inode_core_projid_count, FLD_COUNT, TYP_NONE },
>  	{ "projid_hi", FLDT_UINT16D, OI(COFF(projid_hi)),
>  	  inode_core_projid_count, FLD_COUNT, TYP_NONE },
> +	{ "pad", FLDT_UINT8X, OI(OFF(pad)), CI(6), FLD_ARRAY|FLD_SKIPALL, TYP_NONE },
>  	{ "uid", FLDT_UINT32D, OI(COFF(uid)), C1, 0, TYP_NONE },
>  	{ "gid", FLDT_UINT32D, OI(COFF(gid)), C1, 0, TYP_NONE },
>  	{ "flushiter", FLDT_UINT16D, OI(COFF(flushiter)), C1, 0, TYP_NONE },
> @@ -173,6 +174,7 @@ const field_t	inode_v3_flds[] = {
>  	{ "lsn", FLDT_UINT64X, OI(COFF(lsn)), C1, 0, TYP_NONE },
>  	{ "flags2", FLDT_UINT64X, OI(COFF(flags2)), C1, 0, TYP_NONE },
>  	{ "cowextsize", FLDT_EXTLEN, OI(COFF(cowextsize)), C1, 0, TYP_NONE },
> +	{ "pad2", FLDT_UINT8X, OI(OFF(pad2)), CI(12), FLD_ARRAY|FLD_SKIPALL, TYP_NONE },
>  	{ "crtime", FLDT_TIMESTAMP, OI(COFF(crtime)), C1, 0, TYP_NONE },
>  	{ "inumber", FLDT_INO, OI(COFF(ino)), C1, 0, TYP_NONE },
>  	{ "uuid", FLDT_UUID, OI(COFF(uuid)), C1, 0, TYP_NONE },
> 
> --
> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/5 V2] xfs_db: print structure padding fields consistently
  2017-12-04 23:54   ` [PATCH 1/5 V2] " Eric Sandeen
@ 2017-12-04 23:56     ` Darrick J. Wong
  0 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2017-12-04 23:56 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: sandeen, linux-xfs

On Mon, Dec 04, 2017 at 05:54:09PM -0600, Eric Sandeen wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> We are very inconsistent about how we print padding fields in on-disk
> structures -- sometimes we hide it from printall, sometimes we deviate
> from unsigned hex values, etc.  Make this all consistent -- always hide
> padding values when printing the whole structure, always print them as
> unsigned hex integers when explicitly requested.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> [sandeen: switch to never-print instead of always-print]

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
> 
> V2: constant-ify by never printing instead of always printing,
>     but still make format consistent.
> 
> diff --git a/db/attr.c b/db/attr.c
> index 75fe239..9cbb20d 100644
> --- a/db/attr.c
> +++ b/db/attr.c
> @@ -589,7 +589,7 @@ const field_t	attr3_node_hdr_flds[] = {
>  	{ "info", FLDT_ATTR3_BLKINFO, OI(H3OFF(info)), C1, 0, TYP_NONE },
>  	{ "count", FLDT_UINT16D, OI(H3OFF(__count)), C1, 0, TYP_NONE },
>  	{ "level", FLDT_UINT16D, OI(H3OFF(__level)), C1, 0, TYP_NONE },
> -	{ "pad", FLDT_UINT32D, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
> +	{ "pad", FLDT_UINT32X, OI(H3OFF(__pad32)), C1, FLD_SKIPALL, TYP_NONE },
>  	{ NULL }
>  };
>  
> diff --git a/db/dir2.c b/db/dir2.c
> index 3e21a7b..7f7ea5a 100644
> --- a/db/dir2.c
> +++ b/db/dir2.c
> @@ -977,7 +977,7 @@ const field_t	da3_node_hdr_flds[] = {
>  	{ "info", FLDT_DA3_BLKINFO, OI(H3OFF(info)), C1, 0, TYP_NONE },
>  	{ "count", FLDT_UINT16D, OI(H3OFF(__count)), C1, 0, TYP_NONE },
>  	{ "level", FLDT_UINT16D, OI(H3OFF(__level)), C1, 0, TYP_NONE },
> -	{ "pad", FLDT_UINT32D, OI(H3OFF(__pad32)), C1, 0, TYP_NONE },
> +	{ "pad", FLDT_UINT32X, OI(H3OFF(__pad32)), C1, FLD_SKIPALL, TYP_NONE },
>  	{ NULL }
>  };
>  
> diff --git a/db/dquot.c b/db/dquot.c
> index 4e35df4..4fd1289 100644
> --- a/db/dquot.c
> +++ b/db/dquot.c
> @@ -76,7 +76,7 @@ const field_t	disk_dquot_flds[] = {
>  	{ "btimer", FLDT_INT32D, OI(DOFF(btimer)), C1, 0, TYP_NONE },
>  	{ "iwarns", FLDT_QWARNCNT, OI(DOFF(iwarns)), C1, 0, TYP_NONE },
>  	{ "bwarns", FLDT_QWARNCNT, OI(DOFF(bwarns)), C1, 0, TYP_NONE },
> -	{ "pad0", FLDT_INT32D, OI(DOFF(pad0)), C1, FLD_SKIPALL, TYP_NONE },
> +	{ "pad0", FLDT_UINT32X, OI(DOFF(pad0)), C1, FLD_SKIPALL, TYP_NONE },
>  	{ "rtb_hardlimit", FLDT_QCNT, OI(DOFF(rtb_hardlimit)), C1, 0,
>  	  TYP_NONE },
>  	{ "rtb_softlimit", FLDT_QCNT, OI(DOFF(rtb_softlimit)), C1, 0,
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2017-12-04 23:56 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-17 19:56 [PATCH 0/5] xfsprogs: 4.15 rollup Darrick J. Wong
2017-11-17 19:56 ` [PATCH 1/5] xfs_db: print structure padding fields consistently Darrick J. Wong
2017-11-30 21:37   ` Eric Sandeen
2017-11-30 21:51     ` Darrick J. Wong
2017-11-30 21:57       ` Eric Sandeen
2017-11-30 22:01         ` Darrick J. Wong
2017-12-04 23:54   ` [PATCH 1/5 V2] " Eric Sandeen
2017-12-04 23:56     ` Darrick J. Wong
2017-11-17 19:56 ` [PATCH 2/5] xfs_db: add missing padding fields Darrick J. Wong
2017-12-04 23:54   ` [PATCH 2/5 V2] " Eric Sandeen
2017-12-04 23:56     ` Darrick J. Wong
2017-11-17 19:56 ` [PATCH 3/5] xfs_io: pull xfs errortag definitions from libxfs Darrick J. Wong
2017-11-30 21:40   ` Eric Sandeen
2017-11-17 19:57 ` [PATCH 4/5] xfs_io: provide an interface to the scrub ioctls Darrick J. Wong
2017-12-01  3:46   ` Eric Sandeen
2017-12-01 17:02     ` Darrick J. Wong
2017-12-01 16:06   ` Eric Sandeen
2017-12-01 17:03     ` Darrick J. Wong
2017-12-01 19:02   ` [PATCH v2 " Darrick J. Wong
2017-12-04 21:19     ` Eric Sandeen
2017-11-17 19:57 ` [PATCH 5/5] man: describe the metadata scrubbing ioctl Darrick J. Wong
2017-12-01  4:34   ` Eric Sandeen
2017-12-01  5:00     ` Eric Sandeen
2017-12-01 18:12     ` Darrick J. Wong
2017-12-01 19:03   ` [PATCH v2 " Darrick J. Wong
2017-12-04 21:19     ` Eric Sandeen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.