REVIEW: Make sure xfs_repair uses sector aligned I/O

REVIEW: Make sure xfs_repair uses sector aligned I/O

[Barry Naujok]

[-- Attachment #1: Type: text/plain, Size: 413 bytes --]

Two places it didn't:
  - reading the initial primary superblock
  - checking the log

The log checking fix was basically porting the kernel
xlog_bread/xlog_get_bp/xlog_align from the kernel and
setting up the log_t structure properly in Phase 2.

The initial primary superblock read is now done
using the maximum supported sector size as once the
block is read and translated to another location,
it's discarded.

[-- Attachment #2: sector_aligned_repair.patch --]
[-- Type: application/octet-stream, Size: 6688 bytes --]


===========================================================================
xfsprogs/include/libxfs.h
===========================================================================

--- a/xfsprogs/include/libxfs.h	2007-07-13 10:35:38.000000000 +1000
+++ b/xfsprogs/include/libxfs.h	2007-07-12 18:44:14.474674987 +1000
@@ -236,9 +236,9 @@ enum xfs_buf_flags_t {	/* b_flags bits *
 #define XFS_BUF_COUNT(bp)		((bp)->b_bcount)
 #define XFS_BUF_TARGET(bp)		((bp)->b_dev)
 #define XFS_BUF_SET_PTR(bp,p,cnt)	((bp)->b_addr = (char *)(p)); \
-						XFS_BUF_SETCOUNT(bp,cnt)
+						XFS_BUF_SET_COUNT(bp,cnt)
 #define XFS_BUF_SET_ADDR(bp,blk)	((bp)->b_blkno = (blk))
-#define XFS_BUF_SETCOUNT(bp,cnt)	((bp)->b_bcount = (cnt))
+#define XFS_BUF_SET_COUNT(bp,cnt)	((bp)->b_bcount = (cnt))
 
 #define XFS_BUF_FSPRIVATE(bp,type)	((type)(bp)->b_fsprivate)
 #define XFS_BUF_SET_FSPRIVATE(bp,val)	(bp)->b_fsprivate = (void *)(val)

===========================================================================
xfsprogs/include/libxlog.h
===========================================================================

--- a/xfsprogs/include/libxlog.h	2007-07-13 10:35:38.000000000 +1000
+++ b/xfsprogs/include/libxlog.h	2007-07-12 18:29:08.744562947 +1000
@@ -44,6 +44,8 @@ typedef struct log {
 	int		l_grant_write_cycle;	/* */
 	int		l_grant_write_bytes;	/* */
 	uint		l_sectbb_log;   /* log2 of sector size in bbs */
+	uint		l_sectbb_mask;  /* sector size (in BBs)
+					 * alignment mask */
 } xlog_t;
 
 #include <xfs/xfs_log_recover.h>
@@ -79,13 +81,6 @@ extern void xlog_warn(char *fmt,...);
 extern void xlog_exit(char *fmt,...);
 extern void xlog_panic(char *fmt,...);
 
-#define xlog_get_bp(log,bbs)	libxfs_getbufr(x.logdev, (xfs_daddr_t)-1, (bbs))
-#define xlog_put_bp(bp)		libxfs_putbufr(bp)
-#define xlog_bread(log,blkno,bbs,bp)	\
-	(libxfs_readbufr(x.logdev,	\
-			(log)->l_logBBstart+(blkno), bp, (bbs), 1), 0)
-#define xlog_align(log,blkno,nbblks,bp)	XFS_BUF_PTR(bp)
-
 #define kmem_zalloc(size, foo)			calloc(size,1)
 #define kmem_alloc(size, foo)			calloc(size,1)
 #define kmem_free(ptr, foo)			free(ptr)
@@ -99,6 +94,10 @@ extern int	print_record_header;
 /* libxfs parameters */
 extern libxfs_init_t	x;
 
+extern struct xfs_buf *xlog_get_bp(xlog_t *, int);
+extern void	 xlog_put_bp(struct xfs_buf *);
+extern int	 xlog_bread(xlog_t *, xfs_daddr_t, int, struct xfs_buf *);
+
 extern int  xlog_find_zeroed(xlog_t *log, xfs_daddr_t *blk_no);
 extern int  xlog_find_cycle_start(xlog_t *log, xfs_buf_t *bp,
 		xfs_daddr_t first_blk, xfs_daddr_t *last_blk, uint cycle);

===========================================================================
xfsprogs/libxlog/xfs_log_recover.c
===========================================================================

--- a/xfsprogs/libxlog/xfs_log_recover.c	2007-07-13 10:35:38.000000000 +1000
+++ b/xfsprogs/libxlog/xfs_log_recover.c	2007-07-12 18:43:17.958043752 +1000
@@ -22,6 +22,84 @@
 #define xlog_clear_stale_blocks(log, tail_lsn)		(0)
 #define xfs_readonly_buftarg(buftarg)			(0)
 
+
+/*
+ * Sector aligned buffer routines for buffer create/read/write/access
+ */
+
+#define XLOG_SECTOR_ROUNDUP_BBCOUNT(log, bbs)	\
+	( ((log)->l_sectbb_mask && (bbs & (log)->l_sectbb_mask)) ? \
+	((bbs + (log)->l_sectbb_mask + 1) & ~(log)->l_sectbb_mask) : (bbs) )
+#define XLOG_SECTOR_ROUNDDOWN_BLKNO(log, bno)	((bno) & ~(log)->l_sectbb_mask)
+
+xfs_buf_t *
+xlog_get_bp(
+	xlog_t		*log,
+	int		num_bblks)
+{
+	ASSERT(num_bblks > 0);
+
+	if (log->l_sectbb_log) {
+		if (num_bblks > 1)
+			num_bblks += XLOG_SECTOR_ROUNDUP_BBCOUNT(log, 1);
+		num_bblks = XLOG_SECTOR_ROUNDUP_BBCOUNT(log, num_bblks);
+	}
+	return libxfs_getbufr(log->l_dev, (xfs_daddr_t)-1, num_bblks);
+}
+
+void
+xlog_put_bp(
+	xfs_buf_t	*bp)
+{
+	libxfs_putbufr(bp);
+}
+
+
+/*
+ * nbblks should be uint, but oh well.  Just want to catch that 32-bit length.
+ */
+int
+xlog_bread(
+	xlog_t		*log,
+	xfs_daddr_t	blk_no,
+	int		nbblks,
+	xfs_buf_t	*bp)
+{
+	if (log->l_sectbb_log) {
+		blk_no = XLOG_SECTOR_ROUNDDOWN_BLKNO(log, blk_no);
+		nbblks = XLOG_SECTOR_ROUNDUP_BBCOUNT(log, nbblks);
+	}
+
+	ASSERT(nbblks > 0);
+	ASSERT(BBTOB(nbblks) <= XFS_BUF_SIZE(bp));
+	ASSERT(bp);
+
+	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
+	XFS_BUF_SET_COUNT(bp, BBTOB(nbblks));
+
+	return libxfs_readbufr(log->l_dev, blk_no, bp, nbblks, 0);
+}
+
+
+static xfs_caddr_t
+xlog_align(
+	xlog_t		*log,
+	xfs_daddr_t	blk_no,
+	int		nbblks,
+	xfs_buf_t	*bp)
+{
+	xfs_caddr_t	ptr;
+
+	if (!log->l_sectbb_log)
+		return XFS_BUF_PTR(bp);
+
+	ptr = XFS_BUF_PTR(bp) + BBTOB((int)blk_no & log->l_sectbb_mask);
+	ASSERT(XFS_BUF_SIZE(bp) >=
+		BBTOB(nbblks + (blk_no & log->l_sectbb_mask)));
+	return ptr;
+}
+
+
 /*
  * This routine finds (to an approximation) the first block in the physical
  * log which contains the given cycle.  It uses a binary search algorithm.

===========================================================================
xfsprogs/repair/phase2.c
===========================================================================

--- a/xfsprogs/repair/phase2.c	2007-07-13 10:35:38.000000000 +1000
+++ b/xfsprogs/repair/phase2.c	2007-07-12 18:45:55.305528590 +1000
@@ -50,6 +50,16 @@ zero_log(xfs_mount_t *mp)
 	log.l_logBBsize = x.logBBsize;
 	log.l_logBBstart = x.logBBstart;
 	log.l_mp = mp;
+	if (XFS_SB_VERSION_HASSECTOR(&mp->m_sb)) {
+		log.l_sectbb_log = mp->m_sb.sb_logsectlog - BBSHIFT;
+		ASSERT(log.l_sectbb_log <= mp->m_sectbb_log);
+		/* for larger sector sizes, must have v2 or external log */
+		ASSERT(log.l_sectbb_log == 0 ||
+			log.l_logBBstart == 0 ||
+			XFS_SB_VERSION_HASLOGV2(&mp->m_sb));
+		ASSERT(mp->m_sb.sb_logsectlog >= BBSHIFT);
+	}
+	log.l_sectbb_mask = (1 << log.l_sectbb_log) - 1;
 
 	if ((error = xlog_find_tail(&log, &head_blk, &tail_blk, 0))) {
 		do_warn(_("zero_log: cannot find log head/tail "

===========================================================================
xfsprogs/repair/xfs_repair.c
===========================================================================

--- a/xfsprogs/repair/xfs_repair.c	2007-07-13 10:35:38.000000000 +1000
+++ b/xfsprogs/repair/xfs_repair.c	2007-07-12 16:14:13.347038463 +1000
@@ -501,7 +501,8 @@ main(int argc, char **argv)
 	}
 
 	/* prepare the mount structure */
-	sbp = libxfs_readbuf(x.ddev, XFS_SB_DADDR, 1, 0);
+	sbp = libxfs_readbuf(x.ddev, XFS_SB_DADDR,
+				1 << (XFS_MAX_SECTORSIZE_LOG - BBSHIFT), 0);
 	memset(&xfs_m, 0, sizeof(xfs_mount_t));
 	sb = &xfs_m.m_sb;
 	libxfs_xlate_sb(XFS_BUF_PTR(sbp), sb, 1, XFS_SB_ALL_BITS);
@@ -515,6 +516,7 @@ main(int argc, char **argv)
 		exit(1);
 	}
 	libxfs_putbuf(sbp);
+	libxfs_purgebuf(sbp);
 
 	/*
 	 * set XFS-independent status vars from the mount/sb structure

Re: REVIEW: Make sure xfs_repair uses sector aligned I/O

[David Chinner]

On Fri, Jul 13, 2007 at 10:41:47AM +1000, Barry Naujok wrote:
> Two places it didn't:
>  - reading the initial primary superblock
>  - checking the log
> 
> The log checking fix was basically porting the kernel
> xlog_bread/xlog_get_bp/xlog_align from the kernel and
> setting up the log_t structure properly in Phase 2.
> 
> The initial primary superblock read is now done
> using the maximum supported sector size as once the
> block is read and translated to another location,
> it's discarded.

Seems OK from a quick look of the code. Were you able to
confirm that these are the only places we do non-sector sized
I/O?

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group