All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/14] xfsprogs: make buffer functions return error codes
@ 2020-02-20  1:44 Darrick J. Wong
  2020-02-20  1:44 ` [PATCH 01/14] libxfs: make __cache_lookup return an error code Darrick J. Wong
                   ` (14 more replies)
  0 siblings, 15 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:44 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

Hi all,

Let's fix all the xfs read/get buffer functions to return the usual
integer error codes and pass the buffer pointer as a out-argument.  This
makes it so that we can return useful error output instead of making
callers infer ENOMEM or EAGAIN or whatever other reality they crave from
the NULL pointer that they get when things don't go perfectly.

FWIW, all XBF_TRYLOCK callers must now trigger retries if they receive
EAGAIN.  This may lead to a slight behavioral change in that TRYLOCK
callers will no longer retry for things like ENOMEM, though I didn't see
any obvious changes in user-visible behavior when running fstests.

After finishing the error code conversion, we straighten out the TRYLOCK
callers to remove all this null pointer checks in favor of explicit
EAGAIN checks; and then we change the buffer IO/corruption error
reporting so that we report whoever called the buffer code even when
reading a buffer in transaction context.

The userspace port of the kernel patchset was very difficult, so I am
submitting this series to avoid dumping the work on Eric.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=buf-return-errorcodes

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

* [PATCH 01/14] libxfs: make __cache_lookup return an error code
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
@ 2020-02-20  1:44 ` Darrick J. Wong
  2020-02-21 14:55   ` Christoph Hellwig
  2020-02-20  1:44 ` [PATCH 02/14] libxfs: make libxfs_getbuf_flags " Darrick J. Wong
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:44 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

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

Convert __cache_lookup() to return numeric error codes like most
everywhere else in xfsprogs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/rdwr.c |   68 +++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 24 deletions(-)


diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 52674559..b686d1d5 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -715,30 +715,40 @@ struct list_head	lock_buf_list = {&lock_buf_list, &lock_buf_list};
 int			lock_buf_count = 0;
 #endif
 
-static struct xfs_buf *
-__cache_lookup(struct xfs_bufkey *key, unsigned int flags)
+static int
+__cache_lookup(
+	struct xfs_bufkey	*key,
+	unsigned int		flags,
+	struct xfs_buf		**bpp)
 {
-	struct xfs_buf	*bp;
+	struct cache_node	*cn = NULL;
+	struct xfs_buf		*bp;
 
-	cache_node_get(libxfs_bcache, key, (struct cache_node **)&bp);
-	if (!bp)
-		return NULL;
+	*bpp = NULL;
+
+	cache_node_get(libxfs_bcache, key, &cn);
+	if (!cn)
+		return -ENOMEM;
+	bp = container_of(cn, struct xfs_buf, b_node);
 
 	if (use_xfs_buf_lock) {
-		int ret;
+		int		ret;
 
 		ret = pthread_mutex_trylock(&bp->b_lock);
 		if (ret) {
 			ASSERT(ret == EAGAIN);
-			if (flags & LIBXFS_GETBUF_TRYLOCK)
-				goto out_put;
+			if (flags & LIBXFS_GETBUF_TRYLOCK) {
+				cache_node_put(libxfs_bcache, cn);
+				return -EAGAIN;
+			}
 
 			if (pthread_equal(bp->b_holder, pthread_self())) {
 				fprintf(stderr,
 	_("Warning: recursive buffer locking at block %" PRIu64 " detected\n"),
 					key->blkno);
 				bp->b_recur++;
-				return bp;
+				*bpp = bp;
+				return 0;
 			} else {
 				pthread_mutex_lock(&bp->b_lock);
 			}
@@ -747,9 +757,8 @@ __cache_lookup(struct xfs_bufkey *key, unsigned int flags)
 		bp->b_holder = pthread_self();
 	}
 
-	cache_node_set_priority(libxfs_bcache, (struct cache_node *)bp,
-		cache_node_get_priority((struct cache_node *)bp) -
-						CACHE_PREFETCH_PRIORITY);
+	cache_node_set_priority(libxfs_bcache, cn,
+			cache_node_get_priority(cn) - CACHE_PREFETCH_PRIORITY);
 #ifdef XFS_BUF_TRACING
 	pthread_mutex_lock(&libxfs_bcache->c_mutex);
 	lock_buf_count++;
@@ -762,10 +771,8 @@ __cache_lookup(struct xfs_bufkey *key, unsigned int flags)
 		bp, bp->b_bn, (long long)LIBXFS_BBTOOFF64(key->blkno));
 #endif
 
-	return bp;
-out_put:
-	cache_node_put(libxfs_bcache, (struct cache_node *)bp);
-	return NULL;
+	*bpp = bp;
+	return 0;
 }
 
 static struct xfs_buf *
@@ -775,13 +782,18 @@ libxfs_getbuf_flags(
 	int			len,
 	unsigned int		flags)
 {
-	struct xfs_bufkey key = {NULL};
+	struct xfs_bufkey	key = {NULL};
+	struct xfs_buf		*bp;
+	int			error;
 
 	key.buftarg = btp;
 	key.blkno = blkno;
 	key.bblen = len;
 
-	return __cache_lookup(&key, flags);
+	error = __cache_lookup(&key, flags, &bp);
+	if (error)
+		return NULL;
+	return bp;
 }
 
 /*
@@ -805,11 +817,16 @@ reset_buf_state(
 }
 
 static struct xfs_buf *
-__libxfs_buf_get_map(struct xfs_buftarg *btp, struct xfs_buf_map *map,
-		    int nmaps, int flags)
+__libxfs_buf_get_map(
+	struct xfs_buftarg	*btp,
+	struct xfs_buf_map	*map,
+	int			nmaps,
+	int			flags)
 {
-	struct xfs_bufkey key = {NULL};
-	int i;
+	struct xfs_bufkey	key = {NULL};
+	struct xfs_buf		*bp;
+	int			i;
+	int			error;
 
 	if (nmaps == 1)
 		return libxfs_getbuf_flags(btp, map[0].bm_bn, map[0].bm_len,
@@ -823,7 +840,10 @@ __libxfs_buf_get_map(struct xfs_buftarg *btp, struct xfs_buf_map *map,
 	key.map = map;
 	key.nmaps = nmaps;
 
-	return __cache_lookup(&key, flags);
+	error = __cache_lookup(&key, flags, &bp);
+	if (error)
+		return NULL;
+	return bp;
 }
 
 struct xfs_buf *


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

* [PATCH 02/14] libxfs: make libxfs_getbuf_flags return an error code
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
  2020-02-20  1:44 ` [PATCH 01/14] libxfs: make __cache_lookup return an error code Darrick J. Wong
@ 2020-02-20  1:44 ` Darrick J. Wong
  2020-02-21 14:56   ` Christoph Hellwig
  2020-02-20  1:44 ` [PATCH 03/14] libxfs: make libxfs_buf_get_map " Darrick J. Wong
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:44 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

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

Convert libxfs_getbuf_flags() to return numeric error codes like most
everywhere else in xfsprogs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/rdwr.c |   26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)


diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index b686d1d5..6a7a66ae 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -775,25 +775,21 @@ __cache_lookup(
 	return 0;
 }
 
-static struct xfs_buf *
+static int
 libxfs_getbuf_flags(
 	struct xfs_buftarg	*btp,
 	xfs_daddr_t		blkno,
 	int			len,
-	unsigned int		flags)
+	unsigned int		flags,
+	struct xfs_buf		**bpp)
 {
 	struct xfs_bufkey	key = {NULL};
-	struct xfs_buf		*bp;
-	int			error;
 
 	key.buftarg = btp;
 	key.blkno = blkno;
 	key.bblen = len;
 
-	error = __cache_lookup(&key, flags, &bp);
-	if (error)
-		return NULL;
-	return bp;
+	return __cache_lookup(&key, flags, bpp);
 }
 
 /*
@@ -828,9 +824,13 @@ __libxfs_buf_get_map(
 	int			i;
 	int			error;
 
-	if (nmaps == 1)
-		return libxfs_getbuf_flags(btp, map[0].bm_bn, map[0].bm_len,
-					   flags);
+	if (nmaps == 1) {
+		error = libxfs_getbuf_flags(btp, map[0].bm_bn, map[0].bm_len,
+				flags, &bp);
+		if (error)
+			return NULL;
+		return bp;
+	}
 
 	key.buftarg = btp;
 	key.blkno = map[0].bm_bn;
@@ -977,8 +977,8 @@ libxfs_readbuf(
 	struct xfs_buf		*bp;
 	int			error;
 
-	bp = libxfs_getbuf_flags(btp, blkno, len, 0);
-	if (!bp)
+	error = libxfs_getbuf_flags(btp, blkno, len, 0, &bp);
+	if (error)
 		return NULL;
 
 	/*


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

* [PATCH 03/14] libxfs: make libxfs_buf_get_map return an error code
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
  2020-02-20  1:44 ` [PATCH 01/14] libxfs: make __cache_lookup return an error code Darrick J. Wong
  2020-02-20  1:44 ` [PATCH 02/14] libxfs: make libxfs_getbuf_flags " Darrick J. Wong
@ 2020-02-20  1:44 ` Darrick J. Wong
  2020-02-21 14:57   ` Christoph Hellwig
  2020-02-20  1:44 ` [PATCH 04/14] libxfs: refactor libxfs_readbuf out of existence Darrick J. Wong
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:44 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

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

Convert libxfs_buf_get_map() to return numeric error codes like most
everywhere else in xfsprogs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/libxfs_io.h |   20 +++++++++----
 libxfs/rdwr.c      |   78 ++++++++++++++++++++++++++++------------------------
 libxfs/trans.c     |   16 +++++++----
 repair/prefetch.c  |    6 +++-
 4 files changed, 69 insertions(+), 51 deletions(-)


diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 2a451ab2..189db8d8 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -144,9 +144,9 @@ extern struct cache_operations	libxfs_bcache_operations;
 #define libxfs_buf_get(dev, daddr, len) \
 	libxfs_trace_getbuf(__FUNCTION__, __FILE__, __LINE__, \
 			    (dev), (daddr), (len))
-#define libxfs_buf_get_map(dev, map, nmaps, flags) \
+#define libxfs_buf_get_map(dev, map, nmaps, flags, bpp) \
 	libxfs_trace_getbuf_map(__FUNCTION__, __FILE__, __LINE__, \
-			    (dev), (map), (nmaps), (flags))
+			    (dev), (map), (nmaps), (flags), (bpp))
 #define libxfs_buf_relse(buf) \
 	libxfs_trace_putbuf(__FUNCTION__, __FILE__, __LINE__, (buf))
 
@@ -161,8 +161,9 @@ void libxfs_trace_dirtybuf(const char *func, const char *file, int line,
 struct xfs_buf *libxfs_trace_getbuf(const char *func, const char *file,
 			int line, struct xfs_buftarg *btp, xfs_daddr_t daddr,
 			size_t len);
-extern xfs_buf_t *libxfs_trace_getbuf_map(const char *, const char *, int,
-			struct xfs_buftarg *, struct xfs_buf_map *, int, int);
+int libxfs_trace_getbuf_map(const char *func, const char *file, int line,
+			struct xfs_buftarg *btp, struct xfs_buf_map *map,
+			int nmaps, int flags, struct xfs_buf **bpp);
 extern void	libxfs_trace_putbuf (const char *, const char *, int,
 			xfs_buf_t *);
 
@@ -171,8 +172,8 @@ extern void	libxfs_trace_putbuf (const char *, const char *, int,
 extern xfs_buf_t *libxfs_buf_read_map(struct xfs_buftarg *, struct xfs_buf_map *,
 			int, int, const struct xfs_buf_ops *);
 void libxfs_buf_dirty(struct xfs_buf *bp, int flags);
-extern xfs_buf_t *libxfs_buf_get_map(struct xfs_buftarg *,
-			struct xfs_buf_map *, int, int);
+int libxfs_buf_get_map(struct xfs_buftarg *btp, struct xfs_buf_map *maps,
+			int nmaps, int flags, struct xfs_buf **bpp);
 void	libxfs_buf_relse(struct xfs_buf *);
 
 static inline struct xfs_buf*
@@ -181,9 +182,14 @@ libxfs_buf_get(
 	xfs_daddr_t		blkno,
 	size_t			numblks)
 {
+	struct xfs_buf		*bp;
+	int			error;
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
 
-	return libxfs_buf_get_map(target, &map, 1, 0);
+	error = libxfs_buf_get_map(target, &map, 1, 0, &bp);
+	if (error)
+		return NULL;
+	return bp;
 }
 
 static inline struct xfs_buf*
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 6a7a66ae..4e1665c0 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -392,10 +392,9 @@ libxfs_log_header(
 xfs_buf_t	*libxfs_buf_read_map(struct xfs_buftarg *, struct xfs_buf_map *,
 				int, int, const struct xfs_buf_ops *);
 int		libxfs_writebuf(xfs_buf_t *, int);
-struct xfs_buf *libxfs_buf_get(struct xfs_buftarg *btp, xfs_daddr_t daddr,
-				size_t len);
-xfs_buf_t	*libxfs_buf_get_map(struct xfs_buftarg *, struct xfs_buf_map *,
-				int, int);
+int		libxfs_buf_get_map(struct xfs_buftarg *btp,
+				struct xfs_buf_map *maps, int nmaps, int flags,
+				struct xfs_buf **bpp);
 void		libxfs_buf_relse(struct xfs_buf *);
 
 #define	__add_trace(bp, func, file, line)	\
@@ -450,19 +449,27 @@ libxfs_trace_getbuf(
 	struct xfs_buf		*bp;
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
 
-	bp = libxfs_buf_get_map(target, &map, 1, 0);
+	libxfs_buf_get_map(target, &map, 1, 0, &bp);
 	__add_trace(bp, func, file, line);
 	return bp;
 }
 
-xfs_buf_t *
-libxfs_trace_getbuf_map(const char *func, const char *file, int line,
-		struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
-		int flags)
+int
+libxfs_trace_getbuf_map(
+	const char		*func,
+	const char		*file,
+	int			line,
+	struct xfs_buftarg	*btp,
+	struct xfs_buf_map	*map,
+	int			nmaps,
+	int			flags,
+	struct xfs_buf		**bpp)
 {
-	xfs_buf_t	*bp = libxfs_buf_get_map(btp, map, nmaps, flags);
-	__add_trace(bp, func, file, line);
-	return bp;
+	int			error;
+
+	error = libxfs_buf_get_map(btp, map, nmaps, flags, bpp);
+	__add_trace(*bpp, func, file, line);
+	return error;
 }
 
 void
@@ -812,25 +819,20 @@ reset_buf_state(
 				LIBXFS_B_UPTODATE);
 }
 
-static struct xfs_buf *
+static int
 __libxfs_buf_get_map(
 	struct xfs_buftarg	*btp,
 	struct xfs_buf_map	*map,
 	int			nmaps,
-	int			flags)
+	int			flags,
+	struct xfs_buf		**bpp)
 {
 	struct xfs_bufkey	key = {NULL};
-	struct xfs_buf		*bp;
 	int			i;
-	int			error;
 
-	if (nmaps == 1) {
-		error = libxfs_getbuf_flags(btp, map[0].bm_bn, map[0].bm_len,
-				flags, &bp);
-		if (error)
-			return NULL;
-		return bp;
-	}
+	if (nmaps == 1)
+		return libxfs_getbuf_flags(btp, map[0].bm_bn, map[0].bm_len,
+				flags, bpp);
 
 	key.buftarg = btp;
 	key.blkno = map[0].bm_bn;
@@ -840,21 +842,25 @@ __libxfs_buf_get_map(
 	key.map = map;
 	key.nmaps = nmaps;
 
-	error = __cache_lookup(&key, flags, &bp);
-	if (error)
-		return NULL;
-	return bp;
+	return __cache_lookup(&key, flags, bpp);
 }
 
-struct xfs_buf *
-libxfs_buf_get_map(struct xfs_buftarg *btp, struct xfs_buf_map *map,
-		  int nmaps, int flags)
+int
+libxfs_buf_get_map(
+	struct xfs_buftarg	*btp,
+	struct xfs_buf_map	*map,
+	int			nmaps,
+	int			flags,
+	struct xfs_buf		**bpp)
 {
-	struct xfs_buf	*bp;
+	int			error;
 
-	bp = __libxfs_buf_get_map(btp, map, nmaps, flags);
-	reset_buf_state(bp);
-	return bp;
+	error = __libxfs_buf_get_map(btp, map, nmaps, flags, bpp);
+	if (error)
+		return error;
+
+	reset_buf_state(*bpp);
+	return 0;
 }
 
 void
@@ -1056,8 +1062,8 @@ libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
 		return libxfs_readbuf(btp, map[0].bm_bn, map[0].bm_len,
 					flags, ops);
 
-	bp = __libxfs_buf_get_map(btp, map, nmaps, 0);
-	if (!bp)
+	error = __libxfs_buf_get_map(btp, map, nmaps, 0, &bp);
+	if (error)
 		return NULL;
 
 	bp->b_error = 0;
diff --git a/libxfs/trans.c b/libxfs/trans.c
index f4ce23a7..a7407dc3 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -423,11 +423,16 @@ libxfs_trans_get_buf_map(
 	int			nmaps,
 	xfs_buf_flags_t		flags)
 {
-	xfs_buf_t		*bp;
+	struct xfs_buf		*bp;
 	struct xfs_buf_log_item	*bip;
+	int			error;
 
-	if (!tp)
-		return libxfs_buf_get_map(target, map, nmaps, 0);
+	if (!tp) {
+		error = libxfs_buf_get_map(target, map, nmaps, 0, &bp);
+		if (error)
+			return NULL;
+		return bp;
+	}
 
 	/*
 	 * If we find the buffer in the cache with this transaction
@@ -445,10 +450,9 @@ libxfs_trans_get_buf_map(
 		return bp;
 	}
 
-	bp = libxfs_buf_get_map(target, map, nmaps, 0);
-	if (bp == NULL) {
+	error = libxfs_buf_get_map(target, map, nmaps, 0, &bp);
+	if (error)
 		return NULL;
-	}
 
 	ASSERT(!bp->b_error);
 
diff --git a/repair/prefetch.c b/repair/prefetch.c
index 7f705cc0..587b3b1f 100644
--- a/repair/prefetch.c
+++ b/repair/prefetch.c
@@ -113,6 +113,7 @@ pf_queue_io(
 {
 	struct xfs_buf		*bp;
 	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, map[0].bm_bn);
+	int			error;
 
 	/*
 	 * Never block on a buffer lock here, given that the actual repair
@@ -120,8 +121,9 @@ pf_queue_io(
 	 * the lock holder is either reading it from disk himself or
 	 * completely overwriting it this behaviour is perfectly fine.
 	 */
-	bp = libxfs_buf_get_map(mp->m_dev, map, nmaps, LIBXFS_GETBUF_TRYLOCK);
-	if (!bp)
+	error = -libxfs_buf_get_map(mp->m_dev, map, nmaps, LIBXFS_GETBUF_TRYLOCK,
+			&bp);
+	if (error)
 		return;
 
 	if (bp->b_flags & LIBXFS_B_UPTODATE) {


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

* [PATCH 04/14] libxfs: refactor libxfs_readbuf out of existence
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (2 preceding siblings ...)
  2020-02-20  1:44 ` [PATCH 03/14] libxfs: make libxfs_buf_get_map " Darrick J. Wong
@ 2020-02-20  1:44 ` Darrick J. Wong
  2020-02-21 15:00   ` Christoph Hellwig
  2020-02-20  1:45 ` [PATCH 05/14] libxfs: make libxfs_buf_read_map return an error code Darrick J. Wong
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:44 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

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

The two libxfs_readbuf* functions are awfully similar, so refactor one
into the other to reduce duplicated code.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/rdwr.c |   83 +++++++++++++++++++++------------------------------------
 1 file changed, 30 insertions(+), 53 deletions(-)


diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 4e1665c0..6eaa3e69 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -972,53 +972,6 @@ libxfs_readbuf_verify(
 	return bp->b_error;
 }
 
-static struct xfs_buf *
-libxfs_readbuf(
-	struct xfs_buftarg	*btp,
-	xfs_daddr_t		blkno,
-	size_t			len,
-	int			flags,
-	const struct xfs_buf_ops *ops)
-{
-	struct xfs_buf		*bp;
-	int			error;
-
-	error = libxfs_getbuf_flags(btp, blkno, len, 0, &bp);
-	if (error)
-		return NULL;
-
-	/*
-	 * if the buffer was prefetched, it is likely that it was not validated.
-	 * Hence if we are supplied an ops function and the buffer is marked as
-	 * unchecked, we need to validate it now.
-	 *
-	 * We do this verification even if the buffer is dirty - the
-	 * verification is almost certainly going to fail the CRC check in this
-	 * case as a dirty buffer has not had the CRC recalculated. However, we
-	 * should not be dirtying unchecked buffers and therefore failing it
-	 * here because it's dirty and unchecked indicates we've screwed up
-	 * somewhere else.
-	 */
-	bp->b_error = 0;
-	if ((bp->b_flags & (LIBXFS_B_UPTODATE|LIBXFS_B_DIRTY))) {
-		if (bp->b_flags & LIBXFS_B_UNCHECKED)
-			libxfs_readbuf_verify(bp, ops);
-		return bp;
-	}
-
-	/*
-	 * Set the ops on a cache miss (i.e. first physical read) as the
-	 * verifier may change the ops to match the type of buffer it contains.
-	 * A cache hit might reset the verifier to the original type if we set
-	 * it again, but it won't get called again and set to match the buffer
-	 * contents. *cough* xfs_da_node_buf_ops *cough*.
-	 */
-	error = libxfs_readbufr(btp, blkno, bp, len, flags);
-	if (!error)
-		libxfs_readbuf_verify(bp, ops);
-	return bp;
-}
-
 int
 libxfs_readbufr_map(struct xfs_buftarg *btp, struct xfs_buf *bp, int flags)
 {
@@ -1059,20 +1012,44 @@ libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
 	int		error = 0;
 
 	if (nmaps == 1)
-		return libxfs_readbuf(btp, map[0].bm_bn, map[0].bm_len,
-					flags, ops);
-
-	error = __libxfs_buf_get_map(btp, map, nmaps, 0, &bp);
+		error = libxfs_getbuf_flags(btp, map[0].bm_bn, map[0].bm_len,
+				0, &bp);
+	else
+		error = __libxfs_buf_get_map(btp, map, nmaps, 0, &bp);
 	if (error)
 		return NULL;
 
+	/*
+	 * if the buffer was prefetched, it is likely that it was not validated.
+	 * Hence if we are supplied an ops function and the buffer is marked as
+	 * unchecked, we need to validate it now.
+	 *
+	 * We do this verification even if the buffer is dirty - the
+	 * verification is almost certainly going to fail the CRC check in this
+	 * case as a dirty buffer has not had the CRC recalculated. However, we
+	 * should not be dirtying unchecked buffers and therefore failing it
+	 * here because it's dirty and unchecked indicates we've screwed up
+	 * somewhere else.
+	 */
 	bp->b_error = 0;
-	if ((bp->b_flags & (LIBXFS_B_UPTODATE|LIBXFS_B_DIRTY))) {
+	if (bp->b_flags & (LIBXFS_B_UPTODATE | LIBXFS_B_DIRTY)) {
 		if (bp->b_flags & LIBXFS_B_UNCHECKED)
 			libxfs_readbuf_verify(bp, ops);
 		return bp;
 	}
-	error = libxfs_readbufr_map(btp, bp, flags);
+
+	/*
+	 * Set the ops on a cache miss (i.e. first physical read) as the
+	 * verifier may change the ops to match the type of buffer it contains.
+	 * A cache hit might reset the verifier to the original type if we set
+	 * it again, but it won't get called again and set to match the buffer
+	 * contents. *cough* xfs_da_node_buf_ops *cough*.
+	 */
+	if (nmaps == 1)
+		error = libxfs_readbufr(btp, map[0].bm_bn, bp, map[0].bm_len,
+				flags);
+	else
+		error = libxfs_readbufr_map(btp, bp, flags);
 	if (!error)
 		libxfs_readbuf_verify(bp, ops);
 


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

* [PATCH 05/14] libxfs: make libxfs_buf_read_map return an error code
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (3 preceding siblings ...)
  2020-02-20  1:44 ` [PATCH 04/14] libxfs: refactor libxfs_readbuf out of existence Darrick J. Wong
@ 2020-02-20  1:45 ` Darrick J. Wong
  2020-02-21 15:03   ` Christoph Hellwig
  2020-02-20  1:45 ` [PATCH 06/14] xfs: make xfs_buf_read_map " Darrick J. Wong
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:45 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

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

Make libxfs_buf_read_map() and libxfs_readbuf() return an error code
instead of making callers guess what happened based on whether or not
they got a buffer back.

Add a new SALVAGE flag so that certain utilities (xfs_db and xfs_repair)
can attempt salvage operations even if the verifiers failed, which was
the behavior before this change.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/io.c            |    4 +--
 libxfs/libxfs_io.h |   25 ++++++++++++------
 libxfs/rdwr.c      |   72 ++++++++++++++++++++++++++++++++++++++++++----------
 libxfs/trans.c     |   24 ++++-------------
 repair/da_util.c   |    3 +-
 5 files changed, 84 insertions(+), 44 deletions(-)


diff --git a/db/io.c b/db/io.c
index b81e9969..5c9d72bb 100644
--- a/db/io.c
+++ b/db/io.c
@@ -542,8 +542,8 @@ set_cur(
 		if (!iocur_top->bbmap)
 			return;
 		memcpy(iocur_top->bbmap, bbmap, sizeof(struct bbmap));
-		bp = libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b,
-					bbmap->nmaps, 0, ops);
+		libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b, bbmap->nmaps,
+				LIBXFS_READBUF_SALVAGE, &bp, ops);
 	} else {
 		bp = libxfs_buf_read(mp->m_ddev_targp, blknum, len, 0, ops);
 		iocur_top->bbmap = NULL;
diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 189db8d8..06847ad5 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -126,6 +126,8 @@ extern struct cache_operations	libxfs_bcache_operations;
 
 /* Exit on buffer read error */
 #define LIBXFS_READBUF_FAIL_EXIT	(1 << 0)
+/* Return the buffer even if the IO or verifier fail. */
+#define LIBXFS_READBUF_SALVAGE		(1 << 1)
 
 /* Exit on buffer write error */
 #define LIBXFS_WRITEBUF_FAIL_EXIT	(1 << 0)
@@ -135,9 +137,9 @@ extern struct cache_operations	libxfs_bcache_operations;
 #define libxfs_buf_read(dev, daddr, len, flags, ops) \
 	libxfs_trace_readbuf(__FUNCTION__, __FILE__, __LINE__, \
 			    (dev), (daddr), (len), (flags), (ops))
-#define libxfs_buf_read_map(dev, map, nmaps, flags, ops) \
+#define libxfs_buf_read_map(dev, map, nmaps, flags, bpp, ops) \
 	libxfs_trace_readbuf_map(__FUNCTION__, __FILE__, __LINE__, \
-			    (dev), (map), (nmaps), (flags), (ops))
+			    (dev), (map), (nmaps), (flags), (bpp), (ops))
 #define libxfs_buf_dirty(buf, flags) \
 	libxfs_trace_dirtybuf(__FUNCTION__, __FILE__, __LINE__, \
 			      (buf), (flags))
@@ -153,9 +155,10 @@ extern struct cache_operations	libxfs_bcache_operations;
 struct xfs_buf *libxfs_trace_readbuf(const char *func, const char *file,
 			int line, struct xfs_buftarg *btp, xfs_daddr_t daddr,
 			size_t len, int flags, const struct xfs_buf_ops *ops);
-extern xfs_buf_t *libxfs_trace_readbuf_map(const char *, const char *, int,
-			struct xfs_buftarg *, struct xfs_buf_map *, int, int,
-			const struct xfs_buf_ops *);
+int libxfs_trace_readbuf_map(const char *func, const char *file, int line,
+			struct xfs_buftarg *btp, struct xfs_buf_map *maps,
+			int nmaps, int flags, struct xfs_buf **bpp,
+			const struct xfs_buf_ops *ops);
 void libxfs_trace_dirtybuf(const char *func, const char *file, int line,
 			struct xfs_buf *bp, int flags);
 struct xfs_buf *libxfs_trace_getbuf(const char *func, const char *file,
@@ -169,8 +172,9 @@ extern void	libxfs_trace_putbuf (const char *, const char *, int,
 
 #else
 
-extern xfs_buf_t *libxfs_buf_read_map(struct xfs_buftarg *, struct xfs_buf_map *,
-			int, int, const struct xfs_buf_ops *);
+int libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *maps,
+			int nmaps, int flags, struct xfs_buf **bpp,
+			const struct xfs_buf_ops *ops);
 void libxfs_buf_dirty(struct xfs_buf *bp, int flags);
 int libxfs_buf_get_map(struct xfs_buftarg *btp, struct xfs_buf_map *maps,
 			int nmaps, int flags, struct xfs_buf **bpp);
@@ -200,9 +204,14 @@ libxfs_buf_read(
 	xfs_buf_flags_t		flags,
 	const struct xfs_buf_ops *ops)
 {
+	struct xfs_buf		*bp;
+	int			error;
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
 
-	return libxfs_buf_read_map(target, &map, 1, flags, ops);
+	error = libxfs_buf_read_map(target, &map, 1, flags, &bp, ops);
+	if (error)
+		return NULL;
+	return bp;
 }
 
 #endif /* XFS_BUF_TRACING */
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 6eaa3e69..222ef441 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -389,8 +389,10 @@ libxfs_log_header(
 #undef libxfs_writebuf
 #undef libxfs_buf_get_map
 
-xfs_buf_t	*libxfs_buf_read_map(struct xfs_buftarg *, struct xfs_buf_map *,
-				int, int, const struct xfs_buf_ops *);
+int		libxfs_buf_read_map(struct xfs_buftarg *btp,
+				struct xfs_buf_map *maps, int nmaps, int flags,
+				struct xfs_buf **bpp,
+				const struct xfs_buf_ops *ops);
 int		libxfs_writebuf(xfs_buf_t *, int);
 int		libxfs_buf_get_map(struct xfs_buftarg *btp,
 				struct xfs_buf_map *maps, int nmaps, int flags,
@@ -420,11 +422,30 @@ libxfs_trace_readbuf(
 	struct xfs_buf		*bp;
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
 
-	bp = libxfs_buf_read_map(btp, &map, 1, flags, ops);
+	libxfs_buf_read_map(btp, &map, 1, flags, &bp, ops);
 	__add_trace(bp, func, file, line);
 	return bp;
 }
 
+int
+libxfs_trace_readbuf_map(
+	const char		*func,
+	const char		*file,
+	int			line,
+	struct xfs_buftarg	*btp,
+	struct xfs_buf_map	*map,
+	int			nmaps,
+	int			flags,
+	struct xfs_buf		**bpp,
+	const struct xfs_buf_ops *ops)
+{
+	int			error;
+
+	error = libxfs_buf_read_map(btp, map, nmaps, flags, bpp, ops);
+	__add_trace(*bpp, func, file, line);
+	return error;
+}
+
 void
 libxfs_trace_dirtybuf(
 	const char		*func,
@@ -1004,20 +1025,27 @@ libxfs_readbufr_map(struct xfs_buftarg *btp, struct xfs_buf *bp, int flags)
 	return error;
 }
 
-struct xfs_buf *
-libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
-		int flags, const struct xfs_buf_ops *ops)
+int
+libxfs_buf_read_map(
+	struct xfs_buftarg	*btp,
+	struct xfs_buf_map	*map,
+	int			nmaps,
+	int			flags,
+	struct xfs_buf		**bpp,
+	const struct xfs_buf_ops *ops)
 {
-	struct xfs_buf	*bp;
-	int		error = 0;
+	struct xfs_buf		*bp;
+	bool			salvage = flags & LIBXFS_READBUF_SALVAGE;
+	int			error = 0;
 
+	*bpp = NULL;
 	if (nmaps == 1)
 		error = libxfs_getbuf_flags(btp, map[0].bm_bn, map[0].bm_len,
 				0, &bp);
 	else
 		error = __libxfs_buf_get_map(btp, map, nmaps, 0, &bp);
 	if (error)
-		return NULL;
+		return error;
 
 	/*
 	 * if the buffer was prefetched, it is likely that it was not validated.
@@ -1030,12 +1058,17 @@ libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
 	 * should not be dirtying unchecked buffers and therefore failing it
 	 * here because it's dirty and unchecked indicates we've screwed up
 	 * somewhere else.
+	 *
+	 * Note that if the caller passes in LIBXFS_READBUF_SALVAGE, that means
+	 * they want the buffer even if it contains error status.
 	 */
 	bp->b_error = 0;
 	if (bp->b_flags & (LIBXFS_B_UPTODATE | LIBXFS_B_DIRTY)) {
 		if (bp->b_flags & LIBXFS_B_UNCHECKED)
-			libxfs_readbuf_verify(bp, ops);
-		return bp;
+			error = libxfs_readbuf_verify(bp, ops);
+		if (error && !salvage)
+			goto err;
+		goto ok;
 	}
 
 	/*
@@ -1050,15 +1083,26 @@ libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
 				flags);
 	else
 		error = libxfs_readbufr_map(btp, bp, flags);
-	if (!error)
-		libxfs_readbuf_verify(bp, ops);
+	if (error == -EIO && salvage)
+		goto ok;
+	if (error)
+		goto err;
+
+	error = libxfs_readbuf_verify(bp, ops);
+	if (error && !salvage)
+		goto err;
 
+ok:
 #ifdef IO_DEBUGX
 	printf("%lx: %s: read %lu bytes, error %d, blkno=%llu(%llu), %p\n",
 		pthread_self(), __FUNCTION__, buf - (char *)bp->b_addr, error,
 		(long long)LIBXFS_BBTOOFF64(bp->b_bn), (long long)bp->b_bn, bp);
 #endif
-	return bp;
+	*bpp = bp;
+	return 0;
+err:
+	libxfs_buf_relse(bp);
+	return error;
 }
 
 /* Allocate a raw uncached buffer. */
diff --git a/libxfs/trans.c b/libxfs/trans.c
index a7407dc3..b833e369 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -510,15 +510,8 @@ libxfs_trans_read_buf_map(
 
 	*bpp = NULL;
 
-	if (tp == NULL) {
-		bp = libxfs_buf_read_map(target, map, nmaps, flags, ops);
-		if (!bp) {
-			return (flags & XBF_TRYLOCK) ?  -EAGAIN : -ENOMEM;
-		}
-		if (bp->b_error)
-			goto out_relse;
-		goto done;
-	}
+	if (tp == NULL)
+		return libxfs_buf_read_map(target, map, nmaps, flags, bpp, ops);
 
 	bp = xfs_trans_buf_item_match(tp, target, map, nmaps);
 	if (bp) {
@@ -530,22 +523,15 @@ libxfs_trans_read_buf_map(
 		goto done;
 	}
 
-	bp = libxfs_buf_read_map(target, map, nmaps, flags, ops);
-	if (!bp) {
-		return (flags & XBF_TRYLOCK) ?  -EAGAIN : -ENOMEM;
-	}
-	if (bp->b_error)
-		goto out_relse;
+	error = libxfs_buf_read_map(target, map, nmaps, flags, &bp, ops);
+	if (error)
+		return error;
 
 	_libxfs_trans_bjoin(tp, bp, 1);
 done:
 	trace_xfs_trans_read_buf(bp->b_log_item);
 	*bpp = bp;
 	return 0;
-out_relse:
-	error = bp->b_error;
-	xfs_buf_relse(bp);
-	return error;
 }
 
 /*
diff --git a/repair/da_util.c b/repair/da_util.c
index ed2ec3ba..add93ad8 100644
--- a/repair/da_util.c
+++ b/repair/da_util.c
@@ -64,7 +64,8 @@ da_read_buf(
 		map[i].bm_bn = XFS_FSB_TO_DADDR(mp, bmp[i].startblock);
 		map[i].bm_len = XFS_FSB_TO_BB(mp, bmp[i].blockcount);
 	}
-	bp = libxfs_buf_read_map(mp->m_dev, map, nex, 0, ops);
+	libxfs_buf_read_map(mp->m_dev, map, nex, LIBXFS_READBUF_SALVAGE,
+			&bp, ops);
 	if (map != map_array)
 		free(map);
 	return bp;


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

* [PATCH 06/14] xfs: make xfs_buf_read_map return an error code
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (4 preceding siblings ...)
  2020-02-20  1:45 ` [PATCH 05/14] libxfs: make libxfs_buf_read_map return an error code Darrick J. Wong
@ 2020-02-20  1:45 ` Darrick J. Wong
  2020-02-20  1:45 ` [PATCH 07/14] xfs: make xfs_buf_get " Darrick J. Wong
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:45 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs, Christoph Hellwig, Dave Chinner

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

Source kernel commit: 4ed8e27b4f755f50d78dc3d9f9760b60e891f97b

Convert xfs_buf_read_map() to return numeric error codes like most
everywhere else in xfs.  This involves moving the open-coded logic that
reports metadata IO read / corruption errors and stales the buffer into
xfs_buf_read_map so that the logic is all in one place.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/xfs_alloc.c       |   11 +++++++----
 libxfs/xfs_attr_remote.c |   10 ----------
 2 files changed, 7 insertions(+), 14 deletions(-)


diff --git a/libxfs/xfs_alloc.c b/libxfs/xfs_alloc.c
index ec233d0d..1bf813e3 100644
--- a/libxfs/xfs_alloc.c
+++ b/libxfs/xfs_alloc.c
@@ -2952,14 +2952,17 @@ xfs_read_agf(
 	trace_xfs_read_agf(mp, agno);
 
 	ASSERT(agno != NULLAGNUMBER);
-	error = xfs_trans_read_buf(
-			mp, tp, mp->m_ddev_targp,
+	error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
 			XFS_AG_DADDR(mp, agno, XFS_AGF_DADDR(mp)),
 			XFS_FSS_TO_BB(mp, 1), flags, bpp, &xfs_agf_buf_ops);
+	/*
+	 * Callers of xfs_read_agf() currently interpret a NULL bpp as EAGAIN
+	 * and need to be converted to check for EAGAIN specifically.
+	 */
+	if (error == -EAGAIN)
+		return 0;
 	if (error)
 		return error;
-	if (!*bpp)
-		return 0;
 
 	ASSERT(!(*bpp)->b_error);
 	xfs_buf_set_ref(*bpp, XFS_AGF_REF);
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index c7a4be35..00371bdc 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -421,16 +421,6 @@ xfs_attr_rmtval_get(
 					&xfs_attr3_rmt_buf_ops);
 			if (!bp)
 				return -ENOMEM;
-			error = bp->b_error;
-			if (error) {
-				xfs_buf_ioerror_alert(bp, __func__);
-				xfs_buf_relse(bp);
-
-				/* bad CRC means corrupted metadata */
-				if (error == -EFSBADCRC)
-					error = -EFSCORRUPTED;
-				return error;
-			}
 
 			error = xfs_attr_rmtval_copyout(mp, bp, args->dp->i_ino,
 							&offset, &valuelen,


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

* [PATCH 07/14] xfs: make xfs_buf_get return an error code
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (5 preceding siblings ...)
  2020-02-20  1:45 ` [PATCH 06/14] xfs: make xfs_buf_read_map " Darrick J. Wong
@ 2020-02-20  1:45 ` Darrick J. Wong
  2020-02-20  1:45 ` [PATCH 08/14] xfs: make xfs_buf_get_uncached " Darrick J. Wong
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:45 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs, Christoph Hellwig, Dave Chinner

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

Source kernel commit: 841263e93310595c30653a9f530b2d7bbeed5aae

Convert xfs_buf_get() to return numeric error codes like most
everywhere else in xfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/libxfs_io.h       |   22 +++----
 libxfs/rdwr.c            |   11 ++--
 libxfs/xfs_attr_remote.c |    6 +-
 libxfs/xfs_sb.c          |    8 +--
 repair/phase5.c          |  138 +++++++++++++++++++++++++++++++++-------------
 5 files changed, 122 insertions(+), 63 deletions(-)


diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 06847ad5..064587a6 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -143,9 +143,9 @@ extern struct cache_operations	libxfs_bcache_operations;
 #define libxfs_buf_dirty(buf, flags) \
 	libxfs_trace_dirtybuf(__FUNCTION__, __FILE__, __LINE__, \
 			      (buf), (flags))
-#define libxfs_buf_get(dev, daddr, len) \
+#define libxfs_buf_get(dev, daddr, len, bpp) \
 	libxfs_trace_getbuf(__FUNCTION__, __FILE__, __LINE__, \
-			    (dev), (daddr), (len))
+			    (dev), (daddr), (len), (bpp))
 #define libxfs_buf_get_map(dev, map, nmaps, flags, bpp) \
 	libxfs_trace_getbuf_map(__FUNCTION__, __FILE__, __LINE__, \
 			    (dev), (map), (nmaps), (flags), (bpp))
@@ -161,9 +161,9 @@ int libxfs_trace_readbuf_map(const char *func, const char *file, int line,
 			const struct xfs_buf_ops *ops);
 void libxfs_trace_dirtybuf(const char *func, const char *file, int line,
 			struct xfs_buf *bp, int flags);
-struct xfs_buf *libxfs_trace_getbuf(const char *func, const char *file,
-			int line, struct xfs_buftarg *btp, xfs_daddr_t daddr,
-			size_t len);
+int libxfs_trace_getbuf(const char *func, const char *file, int line,
+			struct xfs_buftarg *btp, xfs_daddr_t daddr,
+			size_t len, struct xfs_buf **bpp);
 int libxfs_trace_getbuf_map(const char *func, const char *file, int line,
 			struct xfs_buftarg *btp, struct xfs_buf_map *map,
 			int nmaps, int flags, struct xfs_buf **bpp);
@@ -180,20 +180,16 @@ int libxfs_buf_get_map(struct xfs_buftarg *btp, struct xfs_buf_map *maps,
 			int nmaps, int flags, struct xfs_buf **bpp);
 void	libxfs_buf_relse(struct xfs_buf *);
 
-static inline struct xfs_buf*
+static inline int
 libxfs_buf_get(
 	struct xfs_buftarg	*target,
 	xfs_daddr_t		blkno,
-	size_t			numblks)
+	size_t			numblks,
+	struct xfs_buf		**bpp)
 {
-	struct xfs_buf		*bp;
-	int			error;
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
 
-	error = libxfs_buf_get_map(target, &map, 1, 0, &bp);
-	if (error)
-		return NULL;
-	return bp;
+	return libxfs_buf_get_map(target, &map, 1, 0, bpp);
 }
 
 static inline struct xfs_buf*
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 222ef441..a05e0fee 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -458,21 +458,22 @@ libxfs_trace_dirtybuf(
 	libxfs_buf_dirty(bp, flags);
 }
 
-struct xfs_buf *
+int
 libxfs_trace_getbuf(
 	const char		*func,
 	const char		*file,
 	int			line,
 	struct xfs_buftarg	*btp,
 	xfs_daddr_t		blkno,
-	size_t			len)
+	size_t			len,
+	struct xfs_buf		**bpp)
 {
-	struct xfs_buf		*bp;
+	int			error;
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
 
-	libxfs_buf_get_map(target, &map, 1, 0, &bp);
+	error = libxfs_buf_get_map(target, &map, 1, 0, bpp);
 	__add_trace(bp, func, file, line);
-	return bp;
+	return error;
 }
 
 int
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index 00371bdc..88163ea8 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -544,9 +544,9 @@ xfs_attr_rmtval_set(
 		dblkno = XFS_FSB_TO_DADDR(mp, map.br_startblock),
 		dblkcnt = XFS_FSB_TO_BB(mp, map.br_blockcount);
 
-		bp = xfs_buf_get(mp->m_ddev_targp, dblkno, dblkcnt);
-		if (!bp)
-			return -ENOMEM;
+		error = xfs_buf_get(mp->m_ddev_targp, dblkno, dblkcnt, &bp);
+		if (error)
+			return error;
 		bp->b_ops = &xfs_attr3_rmt_buf_ops;
 
 		xfs_attr_rmtval_copyin(mp, bp, args->dp->i_ino, &offset,
diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index 4f750d19..d3d5e11d 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -962,9 +962,9 @@ xfs_update_secondary_sbs(
 	for (agno = 1; agno < mp->m_sb.sb_agcount; agno++) {
 		struct xfs_buf		*bp;
 
-		bp = xfs_buf_get(mp->m_ddev_targp,
+		error = xfs_buf_get(mp->m_ddev_targp,
 				 XFS_AG_DADDR(mp, agno, XFS_SB_DADDR),
-				 XFS_FSS_TO_BB(mp, 1));
+				 XFS_FSS_TO_BB(mp, 1), &bp);
 		/*
 		 * If we get an error reading or writing alternate superblocks,
 		 * continue.  xfs_repair chooses the "best" superblock based
@@ -972,12 +972,12 @@ xfs_update_secondary_sbs(
 		 * superblocks un-updated than updated, and xfs_repair may
 		 * pick them over the properly-updated primary.
 		 */
-		if (!bp) {
+		if (error) {
 			xfs_warn(mp,
 		"error allocating secondary superblock for ag %d",
 				agno);
 			if (!saved_error)
-				saved_error = -ENOMEM;
+				saved_error = error;
 			continue;
 		}
 
diff --git a/repair/phase5.c b/repair/phase5.c
index 561a6b3f..a505715f 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -652,6 +652,7 @@ prop_freespace_cursor(xfs_mount_t *mp, xfs_agnumber_t agno,
 	xfs_agblock_t		agbno;
 	bt_stat_level_t		*lptr;
 	const struct xfs_buf_ops *ops = btnum_to_ops(btnum);
+	int			error;
 
 	ASSERT(btnum == XFS_BTNUM_BNO || btnum == XFS_BTNUM_CNT);
 
@@ -692,9 +693,13 @@ prop_freespace_cursor(xfs_mount_t *mp, xfs_agnumber_t agno,
 
 		bt_hdr->bb_u.s.bb_rightsib = cpu_to_be32(agbno);
 
-		lptr->buf_p = libxfs_buf_get(mp->m_dev,
-					XFS_AGB_TO_DADDR(mp, agno, agbno),
-					XFS_FSB_TO_BB(mp, 1));
+		error = -libxfs_buf_get(mp->m_dev,
+				XFS_AGB_TO_DADDR(mp, agno, agbno),
+				XFS_FSB_TO_BB(mp, 1), &lptr->buf_p);
+		if (error)
+			do_error(
+	_("Cannot grab free space btree buffer, err=%d"),
+					error);
 		lptr->agbno = agbno;
 
 		if (lptr->modulo)
@@ -752,6 +757,7 @@ build_freespace_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
 	bt_stat_level_t		*lptr;
 	xfs_extlen_t		freeblks;
 	const struct xfs_buf_ops *ops = btnum_to_ops(btnum);
+	int			error;
 
 	ASSERT(btnum == XFS_BTNUM_BNO || btnum == XFS_BTNUM_CNT);
 
@@ -770,9 +776,13 @@ build_freespace_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
 		lptr = &btree_curs->level[i];
 
 		agbno = get_next_blockaddr(agno, i, btree_curs);
-		lptr->buf_p = libxfs_buf_get(mp->m_dev,
-					XFS_AGB_TO_DADDR(mp, agno, agbno),
-					XFS_FSB_TO_BB(mp, 1));
+		error = -libxfs_buf_get(mp->m_dev,
+				XFS_AGB_TO_DADDR(mp, agno, agbno),
+				XFS_FSB_TO_BB(mp, 1), &lptr->buf_p);
+		if (error)
+			do_error(
+	_("Cannot grab free space btree buffer, err=%d"),
+					error);
 
 		if (i == btree_curs->num_levels - 1)
 			btree_curs->root = agbno;
@@ -881,9 +891,14 @@ build_freespace_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
 			lptr->agbno = get_next_blockaddr(agno, 0, btree_curs);
 			bt_hdr->bb_u.s.bb_rightsib = cpu_to_be32(lptr->agbno);
 
-			lptr->buf_p = libxfs_buf_get(mp->m_dev,
+			error = -libxfs_buf_get(mp->m_dev,
 					XFS_AGB_TO_DADDR(mp, agno, lptr->agbno),
-					XFS_FSB_TO_BB(mp, 1));
+					XFS_FSB_TO_BB(mp, 1),
+					&lptr->buf_p);
+			if (error)
+				do_error(
+	_("Cannot grab free space btree buffer, err=%d"),
+						error);
 		}
 	}
 
@@ -1021,6 +1036,7 @@ prop_ino_cursor(xfs_mount_t *mp, xfs_agnumber_t agno, bt_status_t *btree_curs,
 	xfs_agblock_t		agbno;
 	bt_stat_level_t		*lptr;
 	const struct xfs_buf_ops *ops = btnum_to_ops(btnum);
+	int			error;
 
 	level++;
 
@@ -1059,9 +1075,12 @@ prop_ino_cursor(xfs_mount_t *mp, xfs_agnumber_t agno, bt_status_t *btree_curs,
 
 		bt_hdr->bb_u.s.bb_rightsib = cpu_to_be32(agbno);
 
-		lptr->buf_p = libxfs_buf_get(mp->m_dev,
-					XFS_AGB_TO_DADDR(mp, agno, agbno),
-					XFS_FSB_TO_BB(mp, 1));
+		error = -libxfs_buf_get(mp->m_dev,
+				XFS_AGB_TO_DADDR(mp, agno, agbno),
+				XFS_FSB_TO_BB(mp, 1), &lptr->buf_p);
+		if (error)
+			do_error(_("Cannot grab inode btree buffer, err=%d"),
+					error);
 		lptr->agbno = agbno;
 
 		if (lptr->modulo)
@@ -1108,10 +1127,14 @@ build_agi(xfs_mount_t *mp, xfs_agnumber_t agno, bt_status_t *btree_curs,
 	xfs_buf_t	*agi_buf;
 	xfs_agi_t	*agi;
 	int		i;
+	int		error;
 
-	agi_buf = libxfs_buf_get(mp->m_dev,
+	error = -libxfs_buf_get(mp->m_dev,
 			XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
-			mp->m_sb.sb_sectsize/BBSIZE);
+			mp->m_sb.sb_sectsize / BBSIZE, &agi_buf);
+	if (error)
+		do_error(_("Cannot grab AG %u AGI buffer, err=%d"),
+				agno, error);
 	agi_buf->b_ops = &xfs_agi_buf_ops;
 	agi = XFS_BUF_TO_AGI(agi_buf);
 	memset(agi, 0, mp->m_sb.sb_sectsize);
@@ -1173,6 +1196,7 @@ build_ino_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
 	int			spmask;
 	uint64_t		sparse;
 	uint16_t		holemask;
+	int			error;
 
 	ASSERT(btnum == XFS_BTNUM_INO || btnum == XFS_BTNUM_FINO);
 
@@ -1180,9 +1204,12 @@ build_ino_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
 		lptr = &btree_curs->level[i];
 
 		agbno = get_next_blockaddr(agno, i, btree_curs);
-		lptr->buf_p = libxfs_buf_get(mp->m_dev,
-					XFS_AGB_TO_DADDR(mp, agno, agbno),
-					XFS_FSB_TO_BB(mp, 1));
+		error = -libxfs_buf_get(mp->m_dev,
+				XFS_AGB_TO_DADDR(mp, agno, agbno),
+				XFS_FSB_TO_BB(mp, 1), &lptr->buf_p);
+		if (error)
+			do_error(_("Cannot grab inode btree buffer, err=%d"),
+					error);
 
 		if (i == btree_curs->num_levels - 1)
 			btree_curs->root = agbno;
@@ -1313,9 +1340,14 @@ build_ino_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
 			lptr->agbno = get_next_blockaddr(agno, 0, btree_curs);
 			bt_hdr->bb_u.s.bb_rightsib = cpu_to_be32(lptr->agbno);
 
-			lptr->buf_p = libxfs_buf_get(mp->m_dev,
+			error = -libxfs_buf_get(mp->m_dev,
 					XFS_AGB_TO_DADDR(mp, agno, lptr->agbno),
-					XFS_FSB_TO_BB(mp, 1));
+					XFS_FSB_TO_BB(mp, 1),
+					&lptr->buf_p);
+			if (error)
+				do_error(
+	_("Cannot grab inode btree buffer, err=%d"),
+						error);
 		}
 	}
 
@@ -1429,6 +1461,7 @@ prop_rmap_cursor(
 	xfs_agblock_t		agbno;
 	struct bt_stat_level	*lptr;
 	const struct xfs_buf_ops *ops = btnum_to_ops(XFS_BTNUM_RMAP);
+	int			error;
 
 	level++;
 
@@ -1467,9 +1500,12 @@ prop_rmap_cursor(
 
 		bt_hdr->bb_u.s.bb_rightsib = cpu_to_be32(agbno);
 
-		lptr->buf_p = libxfs_buf_get(mp->m_dev,
-					XFS_AGB_TO_DADDR(mp, agno, agbno),
-					XFS_FSB_TO_BB(mp, 1));
+		error = -libxfs_buf_get(mp->m_dev,
+				XFS_AGB_TO_DADDR(mp, agno, agbno),
+				XFS_FSB_TO_BB(mp, 1), &lptr->buf_p);
+		if (error)
+			do_error(_("Cannot grab rmapbt buffer, err=%d"),
+					error);
 		lptr->agbno = agbno;
 
 		if (lptr->modulo)
@@ -1577,9 +1613,12 @@ build_rmap_tree(
 		lptr = &btree_curs->level[i];
 
 		agbno = get_next_blockaddr(agno, i, btree_curs);
-		lptr->buf_p = libxfs_buf_get(mp->m_dev,
-					XFS_AGB_TO_DADDR(mp, agno, agbno),
-					XFS_FSB_TO_BB(mp, 1));
+		error = -libxfs_buf_get(mp->m_dev,
+				XFS_AGB_TO_DADDR(mp, agno, agbno),
+				XFS_FSB_TO_BB(mp, 1), &lptr->buf_p);
+		if (error)
+			do_error(_("Cannot grab rmapbt buffer, err=%d"),
+					error);
 
 		if (i == btree_curs->num_levels - 1)
 			btree_curs->root = agbno;
@@ -1677,9 +1716,14 @@ _("Insufficient memory to construct reverse-map cursor."));
 			lptr->agbno = get_next_blockaddr(agno, 0, btree_curs);
 			bt_hdr->bb_u.s.bb_rightsib = cpu_to_be32(lptr->agbno);
 
-			lptr->buf_p = libxfs_buf_get(mp->m_dev,
+			error = -libxfs_buf_get(mp->m_dev,
 					XFS_AGB_TO_DADDR(mp, agno, lptr->agbno),
-					XFS_FSB_TO_BB(mp, 1));
+					XFS_FSB_TO_BB(mp, 1),
+					&lptr->buf_p);
+			if (error)
+				do_error(
+	_("Cannot grab rmapbt buffer, err=%d"),
+						error);
 		}
 	}
 	free_slab_cursor(&rmap_cur);
@@ -1781,6 +1825,7 @@ prop_refc_cursor(
 	xfs_agblock_t		agbno;
 	struct bt_stat_level	*lptr;
 	const struct xfs_buf_ops *ops = btnum_to_ops(XFS_BTNUM_REFC);
+	int			error;
 
 	level++;
 
@@ -1819,9 +1864,12 @@ prop_refc_cursor(
 
 		bt_hdr->bb_u.s.bb_rightsib = cpu_to_be32(agbno);
 
-		lptr->buf_p = libxfs_buf_get(mp->m_dev,
-					XFS_AGB_TO_DADDR(mp, agno, agbno),
-					XFS_FSB_TO_BB(mp, 1));
+		error = -libxfs_buf_get(mp->m_dev,
+				XFS_AGB_TO_DADDR(mp, agno, agbno),
+				XFS_FSB_TO_BB(mp, 1), &lptr->buf_p);
+		if (error)
+			do_error(_("Cannot grab refcountbt buffer, err=%d"),
+					error);
 		lptr->agbno = agbno;
 
 		if (lptr->modulo)
@@ -1884,9 +1932,12 @@ build_refcount_tree(
 		lptr = &btree_curs->level[i];
 
 		agbno = get_next_blockaddr(agno, i, btree_curs);
-		lptr->buf_p = libxfs_buf_get(mp->m_dev,
-					XFS_AGB_TO_DADDR(mp, agno, agbno),
-					XFS_FSB_TO_BB(mp, 1));
+		error = -libxfs_buf_get(mp->m_dev,
+				XFS_AGB_TO_DADDR(mp, agno, agbno),
+				XFS_FSB_TO_BB(mp, 1), &lptr->buf_p);
+		if (error)
+			do_error(_("Cannot grab refcountbt buffer, err=%d"),
+					error);
 
 		if (i == btree_curs->num_levels - 1)
 			btree_curs->root = agbno;
@@ -1972,9 +2023,14 @@ _("Insufficient memory to construct refcount cursor."));
 			lptr->agbno = get_next_blockaddr(agno, 0, btree_curs);
 			bt_hdr->bb_u.s.bb_rightsib = cpu_to_be32(lptr->agbno);
 
-			lptr->buf_p = libxfs_buf_get(mp->m_dev,
+			error = -libxfs_buf_get(mp->m_dev,
 					XFS_AGB_TO_DADDR(mp, agno, lptr->agbno),
-					XFS_FSB_TO_BB(mp, 1));
+					XFS_FSB_TO_BB(mp, 1),
+					&lptr->buf_p);
+			if (error)
+				do_error(
+	_("Cannot grab refcountbt buffer, err=%d"),
+						error);
 		}
 	}
 	free_slab_cursor(&refc_cur);
@@ -2007,9 +2063,12 @@ build_agf_agfl(
 	__be32			*freelist;
 	int			error;
 
-	agf_buf = libxfs_buf_get(mp->m_dev,
+	error = -libxfs_buf_get(mp->m_dev,
 			XFS_AG_DADDR(mp, agno, XFS_AGF_DADDR(mp)),
-			mp->m_sb.sb_sectsize/BBSIZE);
+			mp->m_sb.sb_sectsize / BBSIZE, &agf_buf);
+	if (error)
+		do_error(_("Cannot grab AG %u AGF buffer, err=%d"),
+				agno, error);
 	agf_buf->b_ops = &xfs_agf_buf_ops;
 	agf = XFS_BUF_TO_AGF(agf_buf);
 	memset(agf, 0, mp->m_sb.sb_sectsize);
@@ -2079,9 +2138,12 @@ build_agf_agfl(
 		platform_uuid_copy(&agf->agf_uuid, &mp->m_sb.sb_meta_uuid);
 
 	/* initialise the AGFL, then fill it if there are blocks left over. */
-	agfl_buf = libxfs_buf_get(mp->m_dev,
+	error = -libxfs_buf_get(mp->m_dev,
 			XFS_AG_DADDR(mp, agno, XFS_AGFL_DADDR(mp)),
-			mp->m_sb.sb_sectsize/BBSIZE);
+			mp->m_sb.sb_sectsize / BBSIZE, &agfl_buf);
+	if (error)
+		do_error(_("Cannot grab AG %u AGFL buffer, err=%d"),
+				agno, error);
 	agfl_buf->b_ops = &xfs_agfl_buf_ops;
 	agfl = XFS_BUF_TO_AGFL(agfl_buf);
 


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

* [PATCH 08/14] xfs: make xfs_buf_get_uncached return an error code
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (6 preceding siblings ...)
  2020-02-20  1:45 ` [PATCH 07/14] xfs: make xfs_buf_get " Darrick J. Wong
@ 2020-02-20  1:45 ` Darrick J. Wong
  2020-02-20  1:45 ` [PATCH 09/14] xfs: make xfs_buf_read " Darrick J. Wong
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:45 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs, Christoph Hellwig, Dave Chinner

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

Source kernel commit: 2842b6db3d539bec08d080b22635b6e8acaa30ec

Convert xfs_buf_get_uncached() to return numeric error codes like most
everywhere else in xfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/libxfs_io.h        |    4 ++--
 libxfs/rdwr.c             |    8 +++++---
 libxfs/xfs_ag.c           |   21 ++++++++++++---------
 libxlog/xfs_log_recover.c |    7 +++++--
 mkfs/xfs_mkfs.c           |    8 +++++++-
 5 files changed, 31 insertions(+), 17 deletions(-)


diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 064587a6..6fbf976b 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -256,8 +256,8 @@ xfs_buf_associate_memory(struct xfs_buf *bp, void *mem, size_t len)
 	return 0;
 }
 
-struct xfs_buf *libxfs_buf_get_uncached(struct xfs_buftarg *targ, size_t bblen,
-		int flags);
+int libxfs_buf_get_uncached(struct xfs_buftarg *targ, size_t bblen, int flags,
+		struct xfs_buf **bpp);
 int libxfs_buf_read_uncached(struct xfs_buftarg *targ, xfs_daddr_t daddr,
 		size_t bblen, int flags, struct xfs_buf **bpp,
 		const struct xfs_buf_ops *ops);
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index a05e0fee..0bfe46f3 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1128,13 +1128,15 @@ libxfs_getbufr_uncached(
  * Allocate an uncached buffer that points nowhere.  The refcount will be 1,
  * and the cache node hash list will be empty to indicate that it's uncached.
  */
-struct xfs_buf *
+int
 libxfs_buf_get_uncached(
 	struct xfs_buftarg	*targ,
 	size_t			bblen,
-	int			flags)
+	int			flags,
+	struct xfs_buf		**bpp)
 {
-	return libxfs_getbufr_uncached(targ, XFS_BUF_DADDR_NULL, bblen);
+	*bpp = libxfs_getbufr_uncached(targ, XFS_BUF_DADDR_NULL, bblen);
+	return *bpp != NULL ? 0 : -ENOMEM;
 }
 
 /*
diff --git a/libxfs/xfs_ag.c b/libxfs/xfs_ag.c
index 5bf6975d..73fb30cb 100644
--- a/libxfs/xfs_ag.c
+++ b/libxfs/xfs_ag.c
@@ -23,25 +23,28 @@
 #include "xfs_ag_resv.h"
 #include "xfs_health.h"
 
-static struct xfs_buf *
+static int
 xfs_get_aghdr_buf(
 	struct xfs_mount	*mp,
 	xfs_daddr_t		blkno,
 	size_t			numblks,
+	struct xfs_buf		**bpp,
 	const struct xfs_buf_ops *ops)
 {
 	struct xfs_buf		*bp;
+	int			error;
 
-	bp = xfs_buf_get_uncached(mp->m_ddev_targp, numblks, 0);
-	if (!bp)
-		return NULL;
+	error = xfs_buf_get_uncached(mp->m_ddev_targp, numblks, 0, &bp);
+	if (error)
+		return error;
 
 	xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
 	bp->b_bn = blkno;
 	bp->b_maps[0].bm_bn = blkno;
 	bp->b_ops = ops;
 
-	return bp;
+	*bpp = bp;
+	return 0;
 }
 
 static inline bool is_log_ag(struct xfs_mount *mp, struct aghdr_init_data *id)
@@ -340,13 +343,13 @@ xfs_ag_init_hdr(
 	struct aghdr_init_data	*id,
 	aghdr_init_work_f	work,
 	const struct xfs_buf_ops *ops)
-
 {
 	struct xfs_buf		*bp;
+	int			error;
 
-	bp = xfs_get_aghdr_buf(mp, id->daddr, id->numblks, ops);
-	if (!bp)
-		return -ENOMEM;
+	error = xfs_get_aghdr_buf(mp, id->daddr, id->numblks, &bp, ops);
+	if (error)
+		return error;
 
 	(*work)(mp, bp, id);
 
diff --git a/libxlog/xfs_log_recover.c b/libxlog/xfs_log_recover.c
index e7e57bd2..78fbafab 100644
--- a/libxlog/xfs_log_recover.c
+++ b/libxlog/xfs_log_recover.c
@@ -35,11 +35,13 @@ xlog_buf_bbcount_valid(
  * to map to a range of nbblks basic blocks at any valid (basic
  * block) offset within the log.
  */
-xfs_buf_t *
+struct xfs_buf *
 xlog_get_bp(
 	struct xlog	*log,
 	int		nbblks)
 {
+	struct xfs_buf	*bp;
+
 	if (!xlog_buf_bbcount_valid(log, nbblks)) {
 		xfs_warn(log->l_mp, "Invalid block length (0x%x) for buffer",
 			nbblks);
@@ -67,7 +69,8 @@ xlog_get_bp(
 		nbblks += log->l_sectBBsize;
 	nbblks = round_up(nbblks, log->l_sectBBsize);
 
-	return libxfs_buf_get_uncached(log->l_dev, nbblks, 0);
+	libxfs_buf_get_uncached(log->l_dev, nbblks, 0, &bp);
+	return bp;
 }
 
 /*
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index b50b8b3a..e6b44575 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3404,8 +3404,14 @@ get_write_buf(
 	int			bblen)
 {
 	struct xfs_buf		*bp;
+	int			error;
 
-	bp = libxfs_buf_get_uncached(btp, bblen, 0);
+	error = -libxfs_buf_get_uncached(btp, bblen, 0, &bp);
+	if (error) {
+		fprintf(stderr, _("Could not get memory for buffer, err=%d\n"),
+				error);
+		exit(1);
+	}
 	bp->b_bn = daddr;
 	bp->b_maps[0].bm_bn = daddr;
 	return bp;


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

* [PATCH 09/14] xfs: make xfs_buf_read return an error code
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (7 preceding siblings ...)
  2020-02-20  1:45 ` [PATCH 08/14] xfs: make xfs_buf_get_uncached " Darrick J. Wong
@ 2020-02-20  1:45 ` Darrick J. Wong
  2020-02-20  1:45 ` [PATCH 10/14] xfs: make xfs_trans_get_buf_map " Darrick J. Wong
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:45 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs, Christoph Hellwig, Dave Chinner

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

Source kernel commit: 0e3eccce5e0e438bc1aa3c2913221d3d43a1bef4

Convert xfs_buf_read() to return numeric error codes like most
everywhere else in xfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 copy/xfs_copy.c          |   11 +++++++---
 db/io.c                  |    3 ++-
 libxfs/init.c            |   36 +++++++++++++++++----------------
 libxfs/libxfs_io.h       |   21 ++++++++-----------
 libxfs/rdwr.c            |   20 +++++++++++-------
 libxfs/xfs_attr_remote.c |    8 ++++---
 mkfs/xfs_mkfs.c          |    8 ++++---
 repair/attr_repair.c     |   29 ++++++++++++++++-----------
 repair/dino_chunks.c     |   23 ++++++++++++---------
 repair/dinode.c          |   25 +++++++++++++++--------
 repair/phase3.c          |    8 +++++--
 repair/prefetch.c        |    8 +++++--
 repair/rt.c              |   12 ++++++-----
 repair/scan.c            |   50 ++++++++++++++++++++++++++++------------------
 14 files changed, 149 insertions(+), 113 deletions(-)


diff --git a/copy/xfs_copy.c b/copy/xfs_copy.c
index 75c39407..26d7d7d0 100644
--- a/copy/xfs_copy.c
+++ b/copy/xfs_copy.c
@@ -725,9 +725,14 @@ main(int argc, char **argv)
 	/* Do it again, now with proper length and verifier */
 	libxfs_buf_relse(sbp);
 
-	sbp = libxfs_buf_read(mbuf.m_ddev_targp, XFS_SB_DADDR,
-			     1 << (sb->sb_sectlog - BBSHIFT),
-			     0, &xfs_sb_buf_ops);
+	error = -libxfs_buf_read(mbuf.m_ddev_targp, XFS_SB_DADDR,
+			1 << (sb->sb_sectlog - BBSHIFT), 0, &sbp,
+			&xfs_sb_buf_ops);
+	if (error) {
+		do_log(_("%s: couldn't read superblock, error=%d\n"),
+				progname, error);
+		exit(1);
+	}
 	libxfs_buf_relse(sbp);
 
 	mp = libxfs_mount(&mbuf, sb, xargs.ddev, xargs.logdev, xargs.rtdev, 0);
diff --git a/db/io.c b/db/io.c
index 5c9d72bb..384e4c0f 100644
--- a/db/io.c
+++ b/db/io.c
@@ -545,7 +545,8 @@ set_cur(
 		libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b, bbmap->nmaps,
 				LIBXFS_READBUF_SALVAGE, &bp, ops);
 	} else {
-		bp = libxfs_buf_read(mp->m_ddev_targp, blknum, len, 0, ops);
+		libxfs_buf_read(mp->m_ddev_targp, blknum, len,
+				LIBXFS_READBUF_SALVAGE, &bp, ops);
 		iocur_top->bbmap = NULL;
 	}
 
diff --git a/libxfs/init.c b/libxfs/init.c
index c66cb785..4551b6d6 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -403,9 +403,10 @@ rtmount_init(
 	xfs_mount_t	*mp,	/* file system mount structure */
 	int		flags)
 {
-	xfs_buf_t	*bp;	/* buffer for last block of subvolume */
+	struct xfs_buf	*bp;	/* buffer for last block of subvolume */
+	struct xfs_sb	*sbp;	/* filesystem superblock copy in mount */
 	xfs_daddr_t	d;	/* address of last block of subvolume */
-	xfs_sb_t	*sbp;	/* filesystem superblock copy in mount */
+	int		error;
 
 	sbp = &mp->m_sb;
 	if (sbp->sb_rblocks == 0)
@@ -438,9 +439,9 @@ rtmount_init(
 			(unsigned long long) mp->m_sb.sb_rblocks);
 		return -1;
 	}
-	bp = libxfs_buf_read(mp->m_rtdev,
-			d - XFS_FSB_TO_BB(mp, 1), XFS_FSB_TO_BB(mp, 1), 0, NULL);
-	if (bp == NULL) {
+	error = libxfs_buf_read(mp->m_rtdev, d - XFS_FSB_TO_BB(mp, 1),
+			XFS_FSB_TO_BB(mp, 1), 0, &bp, NULL);
+	if (error) {
 		fprintf(stderr, _("%s: realtime size check failed\n"),
 			progname);
 		return -1;
@@ -721,10 +722,10 @@ libxfs_mount(
 	if (!(flags & LIBXFS_MOUNT_DEBUGGER))
 		readflags |= LIBXFS_READBUF_FAIL_EXIT;
 
-	bp = libxfs_buf_read(mp->m_dev,
+	error = libxfs_buf_read(mp->m_dev,
 			d - XFS_FSS_TO_BB(mp, 1), XFS_FSS_TO_BB(mp, 1),
-			readflags, NULL);
-	if (!bp) {
+			readflags, &bp, NULL);
+	if (error) {
 		fprintf(stderr, _("%s: data size check failed\n"), progname);
 		if (!(flags & LIBXFS_MOUNT_DEBUGGER))
 			return NULL;
@@ -734,11 +735,10 @@ libxfs_mount(
 	if (mp->m_logdev_targp->dev &&
 	    mp->m_logdev_targp->dev != mp->m_ddev_targp->dev) {
 		d = (xfs_daddr_t) XFS_FSB_TO_BB(mp, mp->m_sb.sb_logblocks);
-		if ( (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_logblocks) ||
-		     (!(bp = libxfs_buf_read(mp->m_logdev_targp,
-					d - XFS_FSB_TO_BB(mp, 1),
-					XFS_FSB_TO_BB(mp, 1),
-					readflags, NULL))) ) {
+		if (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_logblocks ||
+		    libxfs_buf_read(mp->m_logdev_targp,
+				d - XFS_FSB_TO_BB(mp, 1), XFS_FSB_TO_BB(mp, 1),
+				readflags, &bp, NULL)) {
 			fprintf(stderr, _("%s: log size checks failed\n"),
 					progname);
 			if (!(flags & LIBXFS_MOUNT_DEBUGGER))
@@ -763,10 +763,10 @@ libxfs_mount(
 	 * read the first one and let the user know to check the geometry.
 	 */
 	if (sbp->sb_agcount > 1000000) {
-		bp = libxfs_buf_read(mp->m_dev,
+		error = libxfs_buf_read(mp->m_dev,
 				XFS_AG_DADDR(mp, sbp->sb_agcount - 1, 0), 1,
-				readflags, NULL);
-		if (bp->b_error) {
+				readflags, &bp, NULL);
+		if (error) {
 			fprintf(stderr, _("%s: read of AG %u failed\n"),
 						progname, sbp->sb_agcount);
 			if (!(flags & LIBXFS_MOUNT_DEBUGGER))
@@ -774,8 +774,8 @@ libxfs_mount(
 			fprintf(stderr, _("%s: limiting reads to AG 0\n"),
 								progname);
 			sbp->sb_agcount = 1;
-		}
-		libxfs_buf_relse(bp);
+		} else
+			libxfs_buf_relse(bp);
 	}
 
 	error = libxfs_initialize_perag(mp, sbp->sb_agcount, &mp->m_maxagi);
diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 6fbf976b..81f4269c 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -134,9 +134,9 @@ extern struct cache_operations	libxfs_bcache_operations;
 
 #ifdef XFS_BUF_TRACING
 
-#define libxfs_buf_read(dev, daddr, len, flags, ops) \
+#define libxfs_buf_read(dev, daddr, len, flags, bpp, ops) \
 	libxfs_trace_readbuf(__FUNCTION__, __FILE__, __LINE__, \
-			    (dev), (daddr), (len), (flags), (ops))
+			    (dev), (daddr), (len), (flags), (bpp), (ops))
 #define libxfs_buf_read_map(dev, map, nmaps, flags, bpp, ops) \
 	libxfs_trace_readbuf_map(__FUNCTION__, __FILE__, __LINE__, \
 			    (dev), (map), (nmaps), (flags), (bpp), (ops))
@@ -152,9 +152,10 @@ extern struct cache_operations	libxfs_bcache_operations;
 #define libxfs_buf_relse(buf) \
 	libxfs_trace_putbuf(__FUNCTION__, __FILE__, __LINE__, (buf))
 
-struct xfs_buf *libxfs_trace_readbuf(const char *func, const char *file,
-			int line, struct xfs_buftarg *btp, xfs_daddr_t daddr,
-			size_t len, int flags, const struct xfs_buf_ops *ops);
+int libxfs_trace_readbuf(const char *func, const char *file, int line,
+			struct xfs_buftarg *btp, xfs_daddr_t daddr, size_t len,
+			int flags, const struct xfs_buf_ops *ops,
+			struct xfs_buf **bpp);
 int libxfs_trace_readbuf_map(const char *func, const char *file, int line,
 			struct xfs_buftarg *btp, struct xfs_buf_map *maps,
 			int nmaps, int flags, struct xfs_buf **bpp,
@@ -192,22 +193,18 @@ libxfs_buf_get(
 	return libxfs_buf_get_map(target, &map, 1, 0, bpp);
 }
 
-static inline struct xfs_buf*
+static inline int
 libxfs_buf_read(
 	struct xfs_buftarg	*target,
 	xfs_daddr_t		blkno,
 	size_t			numblks,
 	xfs_buf_flags_t		flags,
+	struct xfs_buf		**bpp,
 	const struct xfs_buf_ops *ops)
 {
-	struct xfs_buf		*bp;
-	int			error;
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
 
-	error = libxfs_buf_read_map(target, &map, 1, flags, &bp, ops);
-	if (error)
-		return NULL;
-	return bp;
+	return libxfs_buf_read_map(target, &map, 1, flags, bpp, ops);
 }
 
 #endif /* XFS_BUF_TRACING */
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 0bfe46f3..91de86ec 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -408,7 +408,7 @@ do {						\
 	}					\
 } while (0)
 
-struct xfs_buf *
+int
 libxfs_trace_readbuf(
 	const char		*func,
 	const char		*file,
@@ -417,14 +417,15 @@ libxfs_trace_readbuf(
 	xfs_daddr_t		blkno,
 	size_t			len,
 	int			flags,
-	const struct xfs_buf_ops *ops)
+	const struct xfs_buf_ops *ops,
+	struct xfs_buf		**bpp)
 {
-	struct xfs_buf		*bp;
+	int			error;
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
 
-	libxfs_buf_read_map(btp, &map, 1, flags, &bp, ops);
-	__add_trace(bp, func, file, line);
-	return bp;
+	error = libxfs_buf_read_map(btp, &map, 1, flags, bpp, ops);
+	__add_trace(*bpp, func, file, line);
+	return error;
 }
 
 int
@@ -509,8 +510,11 @@ struct xfs_buf *
 libxfs_getsb(
 	struct xfs_mount	*mp)
 {
-	return libxfs_buf_read(mp->m_ddev_targp, XFS_SB_DADDR,
-			XFS_FSS_TO_BB(mp, 1), 0, &xfs_sb_buf_ops);
+	struct xfs_buf		*bp;
+
+	libxfs_buf_read(mp->m_ddev_targp, XFS_SB_DADDR, XFS_FSS_TO_BB(mp, 1),
+			0, &bp, &xfs_sb_buf_ops);
+	return bp;
 }
 
 kmem_zone_t			*xfs_buf_zone;
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index 88163ea8..b2a01567 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -417,10 +417,10 @@ xfs_attr_rmtval_get(
 			       (map[i].br_startblock != HOLESTARTBLOCK));
 			dblkno = XFS_FSB_TO_DADDR(mp, map[i].br_startblock);
 			dblkcnt = XFS_FSB_TO_BB(mp, map[i].br_blockcount);
-			bp = xfs_buf_read(mp->m_ddev_targp, dblkno, dblkcnt, 0,
-					&xfs_attr3_rmt_buf_ops);
-			if (!bp)
-				return -ENOMEM;
+			error = xfs_buf_read(mp->m_ddev_targp, dblkno, dblkcnt,
+					0, &bp, &xfs_attr3_rmt_buf_ops);
+			if (error)
+				return error;
 
 			error = xfs_attr_rmtval_copyout(mp, bp, args->dp->i_ino,
 							&offset, &valuelen,
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index e6b44575..83098c35 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3595,11 +3595,11 @@ rewrite_secondary_superblocks(
 	struct xfs_buf		*buf;
 
 	/* rewrite the last superblock */
-	buf = libxfs_buf_read(mp->m_dev,
+	libxfs_buf_read(mp->m_dev,
 			XFS_AGB_TO_DADDR(mp, mp->m_sb.sb_agcount - 1,
 				XFS_SB_DADDR),
 			XFS_FSS_TO_BB(mp, 1),
-			LIBXFS_READBUF_FAIL_EXIT, &xfs_sb_buf_ops);
+			LIBXFS_READBUF_FAIL_EXIT, &buf, &xfs_sb_buf_ops);
 	XFS_BUF_TO_SBP(buf)->sb_rootino = cpu_to_be64(mp->m_sb.sb_rootino);
 	libxfs_buf_dirty(buf, LIBXFS_WRITEBUF_FAIL_EXIT);
 	libxfs_buf_relse(buf);
@@ -3608,11 +3608,11 @@ rewrite_secondary_superblocks(
 	if (mp->m_sb.sb_agcount <= 2)
 		return;
 
-	buf = libxfs_buf_read(mp->m_dev,
+	libxfs_buf_read(mp->m_dev,
 			XFS_AGB_TO_DADDR(mp, (mp->m_sb.sb_agcount - 1) / 2,
 				XFS_SB_DADDR),
 			XFS_FSS_TO_BB(mp, 1),
-			LIBXFS_READBUF_FAIL_EXIT, &xfs_sb_buf_ops);
+			LIBXFS_READBUF_FAIL_EXIT, &buf, &xfs_sb_buf_ops);
 	XFS_BUF_TO_SBP(buf)->sb_rootino = cpu_to_be64(mp->m_sb.sb_rootino);
 	libxfs_buf_dirty(buf, LIBXFS_WRITEBUF_FAIL_EXIT);
 	libxfs_buf_relse(buf);
diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index fb64b0be..8352f17e 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -391,6 +391,7 @@ rmtval_get(xfs_mount_t *mp, xfs_ino_t ino, blkmap_t *blkmap,
 	xfs_buf_t	*bp;
 	int		clearit = 0, i = 0, length = 0, amountdone = 0;
 	int		hdrsize = 0;
+	int		error;
 
 	if (xfs_sb_version_hascrc(&mp->m_sb))
 		hdrsize = sizeof(struct xfs_attr3_rmt_hdr);
@@ -405,10 +406,10 @@ rmtval_get(xfs_mount_t *mp, xfs_ino_t ino, blkmap_t *blkmap,
 			clearit = 1;
 			break;
 		}
-		bp = libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
-				    XFS_FSB_TO_BB(mp, 1), 0,
-				    &xfs_attr3_rmt_buf_ops);
-		if (!bp) {
+		error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
+				XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE,
+				&bp, &xfs_attr3_rmt_buf_ops);
+		if (error) {
 			do_warn(
 	_("can't read remote block for attributes of inode %" PRIu64 "\n"), ino);
 			clearit = 1;
@@ -737,6 +738,7 @@ process_leaf_attr_level(xfs_mount_t	*mp,
 	xfs_dahash_t		current_hashval = 0;
 	xfs_dahash_t		greatest_hashval;
 	struct xfs_attr3_icleaf_hdr leafhdr;
+	int			error;
 
 	da_bno = da_cursor->level[0].bno;
 	ino = da_cursor->ino;
@@ -763,10 +765,11 @@ process_leaf_attr_level(xfs_mount_t	*mp,
 			goto error_out;
 		}
 
-		bp = libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, dev_bno),
-				    XFS_FSB_TO_BB(mp, 1), 0,
-				    &xfs_attr3_leaf_buf_ops);
-		if (!bp) {
+		error = -libxfs_buf_read(mp->m_dev,
+				XFS_FSB_TO_DADDR(mp, dev_bno),
+				XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE,
+				&bp, &xfs_attr3_leaf_buf_ops);
+		if (error) {
 			do_warn(
 	_("can't read file block %u (fsbno %" PRIu64 ") for attribute fork of inode %" PRIu64 "\n"),
 				da_bno, dev_bno, ino);
@@ -1073,6 +1076,7 @@ process_longform_attr(
 	xfs_fsblock_t		bno;
 	struct xfs_buf		*bp;
 	struct xfs_da_blkinfo	*info;
+	int			error;
 
 	*repair = 0;
 
@@ -1094,13 +1098,14 @@ process_longform_attr(
 		return 1;
 	}
 
-	bp = libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
-				XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
-	if (!bp) {
+	error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
+			XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &bp,
+			&xfs_da3_node_buf_ops);
+	if (error) {
 		do_warn(
 	_("can't read block 0 of inode %" PRIu64 " attribute fork\n"),
 			ino);
-		return(1);
+		return 1;
 	}
 	if (bp->b_error == -EFSBADCRC)
 		(*repair)++;
diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
index 1378fc59..028541d4 100644
--- a/repair/dino_chunks.c
+++ b/repair/dino_chunks.c
@@ -31,6 +31,7 @@ check_aginode_block(xfs_mount_t	*mp,
 	int		i;
 	int		cnt = 0;
 	xfs_buf_t	*bp;
+	int		error;
 
 	/*
 	 * it's ok to read these possible inode blocks in one at
@@ -39,9 +40,10 @@ check_aginode_block(xfs_mount_t	*mp,
 	 * tree and we wouldn't be here and we stale the buffers out
 	 * so no one else will overlap them.
 	 */
-	bp = libxfs_buf_read(mp->m_dev, XFS_AGB_TO_DADDR(mp, agno, agbno),
-			XFS_FSB_TO_BB(mp, 1), 0, NULL);
-	if (!bp) {
+	error = -libxfs_buf_read(mp->m_dev, XFS_AGB_TO_DADDR(mp, agno, agbno),
+			XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &bp,
+			NULL);
+	if (error) {
 		do_warn(_("cannot read agbno (%u/%u), disk block %" PRId64 "\n"),
 			agno, agbno, XFS_AGB_TO_DADDR(mp, agno, agbno));
 		return(0);
@@ -612,6 +614,7 @@ process_inode_chunk(
 	int			bp_index;
 	int			cluster_offset;
 	struct xfs_ino_geometry	*igeo = M_IGEO(mp);
+	int			error;
 
 	ASSERT(first_irec != NULL);
 	ASSERT(XFS_AGINO_TO_OFFSET(mp, first_irec->ino_startnum) == 0);
@@ -656,13 +659,13 @@ process_inode_chunk(
 		pftrace("about to read off %llu in AG %d",
 			XFS_AGB_TO_DADDR(mp, agno, agbno), agno);
 
-		bplist[bp_index] = libxfs_buf_read(mp->m_dev,
-					XFS_AGB_TO_DADDR(mp, agno, agbno),
-					XFS_FSB_TO_BB(mp,
-						M_IGEO(mp)->blocks_per_cluster),
-					0,
-					&xfs_inode_buf_ops);
-		if (!bplist[bp_index]) {
+		error = -libxfs_buf_read(mp->m_dev,
+				XFS_AGB_TO_DADDR(mp, agno, agbno),
+				XFS_FSB_TO_BB(mp,
+					M_IGEO(mp)->blocks_per_cluster),
+				LIBXFS_READBUF_SALVAGE, &bplist[bp_index],
+				&xfs_inode_buf_ops);
+		if (error) {
 			do_warn(_("cannot read inode %" PRIu64 ", disk block %" PRId64 ", cnt %d\n"),
 				XFS_AGINO_TO_INO(mp, agno, first_irec->ino_startnum),
 				XFS_AGB_TO_DADDR(mp, agno, agbno),
diff --git a/repair/dinode.c b/repair/dinode.c
index 276dd2e1..1f5fa4f8 100644
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -730,6 +730,7 @@ get_agino_buf(
 	xfs_daddr_t		cluster_daddr;
 	xfs_daddr_t		cluster_blks;
 	struct xfs_ino_geometry	*igeo = M_IGEO(mp);
+	int			error;
 
 	/*
 	 * Inode buffers have been read into memory in inode_cluster_size
@@ -748,9 +749,9 @@ get_agino_buf(
 		cluster_agino, cluster_daddr, cluster_blks);
 #endif
 
-	bp = libxfs_buf_read(mp->m_dev, cluster_daddr, cluster_blks,
-			0, &xfs_inode_buf_ops);
-	if (!bp) {
+	error = -libxfs_buf_read(mp->m_dev, cluster_daddr, cluster_blks, 0,
+			&bp, &xfs_inode_buf_ops);
+	if (error) {
 		do_warn(_("cannot read inode (%u/%u), disk block %" PRIu64 "\n"),
 			agno, cluster_agino, cluster_daddr);
 		return NULL;
@@ -1149,6 +1150,7 @@ process_quota_inode(
 	xfs_fileoff_t		qbno;
 	int			i;
 	int			t = 0;
+	int			error;
 
 	switch (ino_type) {
 		case XR_INO_UQUOTA:
@@ -1179,9 +1181,11 @@ process_quota_inode(
 		fsbno = blkmap_get(blkmap, qbno);
 		dqid = (xfs_dqid_t)qbno * dqperchunk;
 
-		bp = libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
-				    dqchunklen, 0, &xfs_dquot_buf_ops);
-		if (!bp) {
+		error = -libxfs_buf_read(mp->m_dev,
+				XFS_FSB_TO_DADDR(mp, fsbno), dqchunklen,
+				LIBXFS_READBUF_SALVAGE, &bp,
+				&xfs_dquot_buf_ops);
+		if (error) {
 			do_warn(
 _("cannot read inode %" PRIu64 ", file block %" PRIu64 ", disk block %" PRIu64 "\n"),
 				lino, qbno, fsbno);
@@ -1255,6 +1259,7 @@ process_symlink_remote(
 	int			pathlen;
 	int			offset;
 	int			i;
+	int			error;
 
 	offset = 0;
 	pathlen = be64_to_cpu(dino->di_size);
@@ -1286,9 +1291,11 @@ _("cannot read inode %" PRIu64 ", file block %d, NULL disk block\n"),
 
 		byte_cnt = XFS_FSB_TO_B(mp, blk_cnt);
 
-		bp = libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
-				    BTOBB(byte_cnt), 0, &xfs_symlink_buf_ops);
-		if (!bp) {
+		error = -libxfs_buf_read(mp->m_dev,
+				XFS_FSB_TO_DADDR(mp, fsbno), BTOBB(byte_cnt),
+				LIBXFS_READBUF_SALVAGE, &bp,
+				&xfs_symlink_buf_ops);
+		if (error) {
 			do_warn(
 _("cannot read inode %" PRIu64 ", file block %d, disk block %" PRIu64 "\n"),
 				lino, i, fsbno);
diff --git a/repair/phase3.c b/repair/phase3.c
index 396743a4..8bbc4e6b 100644
--- a/repair/phase3.c
+++ b/repair/phase3.c
@@ -27,11 +27,13 @@ process_agi_unlinked(
 	struct xfs_agi		*agip;
 	xfs_agnumber_t		i;
 	int			agi_dirty = 0;
+	int			error;
 
-	bp = libxfs_buf_read(mp->m_dev,
+	error = -libxfs_buf_read(mp->m_dev,
 			XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
-			mp->m_sb.sb_sectsize/BBSIZE, 0, &xfs_agi_buf_ops);
-	if (!bp)
+			mp->m_sb.sb_sectsize / BBSIZE, LIBXFS_READBUF_SALVAGE,
+			&bp, &xfs_agi_buf_ops);
+	if (error)
 		do_error(_("cannot read agi block %" PRId64 " for ag %u\n"),
 			XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)), agno);
 
diff --git a/repair/prefetch.c b/repair/prefetch.c
index 587b3b1f..9a6de206 100644
--- a/repair/prefetch.c
+++ b/repair/prefetch.c
@@ -272,10 +272,12 @@ pf_scan_lbtree(
 {
 	xfs_buf_t		*bp;
 	int			rc;
+	int			error;
 
-	bp = libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, dbno),
-			XFS_FSB_TO_BB(mp, 1), 0, &xfs_bmbt_buf_ops);
-	if (!bp)
+	error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, dbno),
+			XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &bp,
+			&xfs_bmbt_buf_ops);
+	if (error)
 		return 0;
 
 	XFS_BUF_SET_PRIORITY(bp, isadir ? B_DIR_BMAP : B_BMAP);
diff --git a/repair/rt.c b/repair/rt.c
index b514998d..d901e751 100644
--- a/repair/rt.c
+++ b/repair/rt.c
@@ -193,9 +193,9 @@ process_rtbitmap(xfs_mount_t	*mp,
 			error = 1;
 			continue;
 		}
-		bp = libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
-				XFS_FSB_TO_BB(mp, 1), NULL);
-		if (!bp) {
+		error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
+				XFS_FSB_TO_BB(mp, 1), 0, NULL, &bp);
+		if (error) {
 			do_warn(_("can't read block %d for rtbitmap inode\n"),
 					bmbno);
 			error = 1;
@@ -255,9 +255,9 @@ process_rtsummary(xfs_mount_t	*mp,
 			error++;
 			continue;
 		}
-		bp = libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
-				XFS_FSB_TO_BB(mp, 1), NULL);
-		if (!bp) {
+		error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
+				XFS_FSB_TO_BB(mp, 1), 0, NULL, &bp);
+		if (error) {
 			do_warn(_("can't read block %d for rtsummary inode\n"),
 					sumbno);
 			error++;
diff --git a/repair/scan.c b/repair/scan.c
index 8b91b27e..f8cc9590 100644
--- a/repair/scan.c
+++ b/repair/scan.c
@@ -66,11 +66,13 @@ scan_sbtree(
 	void		*priv,
 	const struct xfs_buf_ops *ops)
 {
-	xfs_buf_t	*bp;
+	struct xfs_buf	*bp;
+	int		error;
 
-	bp = libxfs_buf_read(mp->m_dev, XFS_AGB_TO_DADDR(mp, agno, root),
-			XFS_FSB_TO_BB(mp, 1), 0, ops);
-	if (!bp) {
+	error = -libxfs_buf_read(mp->m_dev, XFS_AGB_TO_DADDR(mp, agno, root),
+			XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &bp,
+			ops);
+	if (error) {
 		do_error(_("can't read btree block %d/%d\n"), agno, root);
 		return;
 	}
@@ -123,9 +125,10 @@ scan_lbtree(
 	int		dirty = 0;
 	bool		badcrc = false;
 
-	bp = libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, root),
-		      XFS_FSB_TO_BB(mp, 1), 0, ops);
-	if (!bp)  {
+	err = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, root),
+			XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &bp,
+			ops);
+	if (err) {
 		do_error(_("can't read btree block %d/%d\n"),
 			XFS_FSB_TO_AGNO(mp, root),
 			XFS_FSB_TO_AGBNO(mp, root));
@@ -2102,6 +2105,7 @@ scan_freelist(
 	xfs_buf_t		*agflbuf;
 	xfs_agnumber_t		agno;
 	struct agfl_state	state;
+	int			error;
 
 	agno = be32_to_cpu(agf->agf_seqno);
 
@@ -2113,10 +2117,11 @@ scan_freelist(
 	if (be32_to_cpu(agf->agf_flcount) == 0)
 		return;
 
-	agflbuf = libxfs_buf_read(mp->m_dev,
-				 XFS_AG_DADDR(mp, agno, XFS_AGFL_DADDR(mp)),
-				 XFS_FSS_TO_BB(mp, 1), 0, &xfs_agfl_buf_ops);
-	if (!agflbuf)  {
+	error = -libxfs_buf_read(mp->m_dev,
+			XFS_AG_DADDR(mp, agno, XFS_AGFL_DADDR(mp)),
+			XFS_FSS_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &agflbuf,
+			&xfs_agfl_buf_ops);
+	if (error) {
 		do_abort(_("can't read agfl block for ag %d\n"), agno);
 		return;
 	}
@@ -2330,6 +2335,7 @@ scan_ag(
 	int		sb_dirty = 0;
 	int		status;
 	char		*objname = NULL;
+	int		error;
 
 	sb = (struct xfs_sb *)calloc(BBTOB(XFS_FSS_TO_BB(mp, 1)), 1);
 	if (!sb) {
@@ -2337,27 +2343,31 @@ scan_ag(
 		return;
 	}
 
-	sbbuf = libxfs_buf_read(mp->m_dev, XFS_AG_DADDR(mp, agno, XFS_SB_DADDR),
-				XFS_FSS_TO_BB(mp, 1), 0, &xfs_sb_buf_ops);
-	if (!sbbuf)  {
+	error = -libxfs_buf_read(mp->m_dev,
+			XFS_AG_DADDR(mp, agno, XFS_SB_DADDR),
+			XFS_FSS_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &sbbuf,
+			&xfs_sb_buf_ops);
+	if (error) {
 		objname = _("root superblock");
 		goto out_free_sb;
 	}
 	libxfs_sb_from_disk(sb, XFS_BUF_TO_SBP(sbbuf));
 
-	agfbuf = libxfs_buf_read(mp->m_dev,
+	error = -libxfs_buf_read(mp->m_dev,
 			XFS_AG_DADDR(mp, agno, XFS_AGF_DADDR(mp)),
-			XFS_FSS_TO_BB(mp, 1), 0, &xfs_agf_buf_ops);
-	if (!agfbuf)  {
+			XFS_FSS_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &agfbuf,
+			&xfs_agf_buf_ops);
+	if (error) {
 		objname = _("agf block");
 		goto out_free_sbbuf;
 	}
 	agf = XFS_BUF_TO_AGF(agfbuf);
 
-	agibuf = libxfs_buf_read(mp->m_dev,
+	error = -libxfs_buf_read(mp->m_dev,
 			XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
-			XFS_FSS_TO_BB(mp, 1), 0, &xfs_agi_buf_ops);
-	if (!agibuf)  {
+			XFS_FSS_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &agibuf,
+			&xfs_agi_buf_ops);
+	if (error) {
 		objname = _("agi block");
 		goto out_free_agfbuf;
 	}


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

* [PATCH 10/14] xfs: make xfs_trans_get_buf_map return an error code
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (8 preceding siblings ...)
  2020-02-20  1:45 ` [PATCH 09/14] xfs: make xfs_buf_read " Darrick J. Wong
@ 2020-02-20  1:45 ` Darrick J. Wong
  2020-02-20  1:45 ` [PATCH 11/14] xfs: make xfs_trans_get_buf " Darrick J. Wong
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:45 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs, Christoph Hellwig, Dave Chinner

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

Source kernel commit: 9676b54e6e28689af1b4247569f14466bdfc5390

Convert xfs_trans_get_buf_map() to return numeric error codes like most
everywhere else in xfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 include/xfs_trans.h   |   15 ++++++++++-----
 libxfs/trans.c        |   22 +++++++++++-----------
 libxfs/xfs_da_btree.c |    8 ++------
 3 files changed, 23 insertions(+), 22 deletions(-)


diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index cff27546..d94617e0 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -105,10 +105,9 @@ void	libxfs_trans_log_buf(struct xfs_trans *, struct xfs_buf *,
 				uint, uint);
 bool	libxfs_trans_ordered_buf(xfs_trans_t *, struct xfs_buf *);
 
-struct xfs_buf	*libxfs_trans_get_buf_map(struct xfs_trans *tp,
-					struct xfs_buftarg *btp,
-					struct xfs_buf_map *map, int nmaps,
-					xfs_buf_flags_t flags);
+int 	libxfs_trans_get_buf_map(struct xfs_trans *tp, struct xfs_buftarg *btp,
+		struct xfs_buf_map *map, int nmaps, xfs_buf_flags_t flags,
+		struct xfs_buf **bpp);
 
 int	libxfs_trans_read_buf_map(struct xfs_mount *mp, struct xfs_trans *tp,
 				  struct xfs_buftarg *btp,
@@ -123,8 +122,14 @@ libxfs_trans_get_buf(
 	int			numblks,
 	uint			flags)
 {
+	struct xfs_buf		*bp;
+	int			error;
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
-	return libxfs_trans_get_buf_map(tp, btp, &map, 1, flags);
+
+	error = libxfs_trans_get_buf_map(tp, btp, &map, 1, flags, &bp);
+	if (error)
+		return NULL;
+	return bp;
 }
 
 static inline int
diff --git a/libxfs/trans.c b/libxfs/trans.c
index b833e369..d200038b 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -415,24 +415,22 @@ libxfs_trans_bhold_release(
  * If the transaction pointer is NULL, make this just a normal
  * get_buf() call.
  */
-struct xfs_buf *
+int
 libxfs_trans_get_buf_map(
 	struct xfs_trans	*tp,
 	struct xfs_buftarg	*target,
 	struct xfs_buf_map	*map,
 	int			nmaps,
-	xfs_buf_flags_t		flags)
+	xfs_buf_flags_t		flags,
+	struct xfs_buf		**bpp)
 {
 	struct xfs_buf		*bp;
 	struct xfs_buf_log_item	*bip;
 	int			error;
 
-	if (!tp) {
-		error = libxfs_buf_get_map(target, map, nmaps, 0, &bp);
-		if (error)
-			return NULL;
-		return bp;
-	}
+	*bpp = NULL;
+	if (!tp)
+		return libxfs_buf_get_map(target, map, nmaps, 0, bpp);
 
 	/*
 	 * If we find the buffer in the cache with this transaction
@@ -447,18 +445,20 @@ libxfs_trans_get_buf_map(
 		ASSERT(bip != NULL);
 		bip->bli_recur++;
 		trace_xfs_trans_get_buf_recur(bip);
-		return bp;
+		*bpp = bp;
+		return 0;
 	}
 
 	error = libxfs_buf_get_map(target, map, nmaps, 0, &bp);
 	if (error)
-		return NULL;
+		return error;
 
 	ASSERT(!bp->b_error);
 
 	_libxfs_trans_bjoin(tp, bp, 1);
 	trace_xfs_trans_get_buf(bp->b_log_item);
-	return bp;
+	*bpp = bp;
+	return 0;
 }
 
 xfs_buf_t *
diff --git a/libxfs/xfs_da_btree.c b/libxfs/xfs_da_btree.c
index 5102c6e0..3f40e99e 100644
--- a/libxfs/xfs_da_btree.c
+++ b/libxfs/xfs_da_btree.c
@@ -2588,13 +2588,9 @@ xfs_da_get_buf(
 	if (error || nmap == 0)
 		goto out_free;
 
-	bp = xfs_trans_get_buf_map(tp, mp->m_ddev_targp, mapp, nmap, 0);
-	error = bp ? bp->b_error : -EIO;
-	if (error) {
-		if (bp)
-			xfs_trans_brelse(tp, bp);
+	error = xfs_trans_get_buf_map(tp, mp->m_ddev_targp, mapp, nmap, 0, &bp);
+	if (error)
 		goto out_free;
-	}
 
 	*bpp = bp;
 


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

* [PATCH 11/14] xfs: make xfs_trans_get_buf return an error code
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (9 preceding siblings ...)
  2020-02-20  1:45 ` [PATCH 10/14] xfs: make xfs_trans_get_buf_map " Darrick J. Wong
@ 2020-02-20  1:45 ` Darrick J. Wong
  2020-02-20  1:45 ` [PATCH 12/14] xfs: remove the xfs_btree_get_buf[ls] functions Darrick J. Wong
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:45 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs, Christoph Hellwig, Dave Chinner

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

Source kernel commit: ce92464c180b60e79022bdf1175b7737a11f59b7

Convert xfs_trans_get_buf() to return numeric error codes like most
everywhere else in xfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 include/xfs_trans.h |   12 ++++--------
 libxfs/xfs_btree.c  |   23 ++++++++++++++++-------
 libxfs/xfs_ialloc.c |   12 ++++++------
 libxfs/xfs_sb.c     |    9 +++++----
 mkfs/proto.c        |   10 ++++++++--
 5 files changed, 39 insertions(+), 27 deletions(-)


diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index d94617e0..33c92cb2 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -114,22 +114,18 @@ int	libxfs_trans_read_buf_map(struct xfs_mount *mp, struct xfs_trans *tp,
 				  struct xfs_buf_map *map, int nmaps,
 				  xfs_buf_flags_t flags, struct xfs_buf **bpp,
 				  const struct xfs_buf_ops *ops);
-static inline struct xfs_buf *
+static inline int
 libxfs_trans_get_buf(
 	struct xfs_trans	*tp,
 	struct xfs_buftarg	*btp,
 	xfs_daddr_t		blkno,
 	int			numblks,
-	uint			flags)
+	uint			flags,
+	struct xfs_buf		**bpp)
 {
-	struct xfs_buf		*bp;
-	int			error;
 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
 
-	error = libxfs_trans_get_buf_map(tp, btp, &map, 1, flags, &bp);
-	if (error)
-		return NULL;
-	return bp;
+	return libxfs_trans_get_buf_map(tp, btp, &map, 1, flags, bpp);
 }
 
 static inline int
diff --git a/libxfs/xfs_btree.c b/libxfs/xfs_btree.c
index ead05a46..aeaa9623 100644
--- a/libxfs/xfs_btree.c
+++ b/libxfs/xfs_btree.c
@@ -685,11 +685,16 @@ xfs_btree_get_bufl(
 	xfs_trans_t	*tp,		/* transaction pointer */
 	xfs_fsblock_t	fsbno)		/* file system block number */
 {
+	struct xfs_buf		*bp;
 	xfs_daddr_t		d;		/* real disk block address */
+	int			error;
 
 	ASSERT(fsbno != NULLFSBLOCK);
 	d = XFS_FSB_TO_DADDR(mp, fsbno);
-	return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0);
+	error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, &bp);
+	if (error)
+		return NULL;
+	return bp;
 }
 
 /*
@@ -703,12 +708,17 @@ xfs_btree_get_bufs(
 	xfs_agnumber_t	agno,		/* allocation group number */
 	xfs_agblock_t	agbno)		/* allocation group block number */
 {
+	struct xfs_buf		*bp;
 	xfs_daddr_t		d;		/* real disk block address */
+	int			error;
 
 	ASSERT(agno != NULLAGNUMBER);
 	ASSERT(agbno != NULLAGBLOCK);
 	d = XFS_AGB_TO_DADDR(mp, agno, agbno);
-	return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0);
+	error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, &bp);
+	if (error)
+		return NULL;
+	return bp;
 }
 
 /*
@@ -1267,11 +1277,10 @@ xfs_btree_get_buf_block(
 	error = xfs_btree_ptr_to_daddr(cur, ptr, &d);
 	if (error)
 		return error;
-	*bpp = xfs_trans_get_buf(cur->bc_tp, mp->m_ddev_targp, d,
-				 mp->m_bsize, 0);
-
-	if (!*bpp)
-		return -ENOMEM;
+	error = xfs_trans_get_buf(cur->bc_tp, mp->m_ddev_targp, d, mp->m_bsize,
+			0, bpp);
+	if (error)
+		return error;
 
 	(*bpp)->b_ops = cur->bc_ops->buf_ops;
 	*block = XFS_BUF_TO_BLOCK(*bpp);
diff --git a/libxfs/xfs_ialloc.c b/libxfs/xfs_ialloc.c
index baa99551..00b33263 100644
--- a/libxfs/xfs_ialloc.c
+++ b/libxfs/xfs_ialloc.c
@@ -271,6 +271,7 @@ xfs_ialloc_inode_init(
 	int			i, j;
 	xfs_daddr_t		d;
 	xfs_ino_t		ino = 0;
+	int			error;
 
 	/*
 	 * Loop over the new block(s), filling in the inodes.  For small block
@@ -322,12 +323,11 @@ xfs_ialloc_inode_init(
 		 */
 		d = XFS_AGB_TO_DADDR(mp, agno, agbno +
 				(j * M_IGEO(mp)->blocks_per_cluster));
-		fbuf = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
-					 mp->m_bsize *
-					 M_IGEO(mp)->blocks_per_cluster,
-					 XBF_UNMAPPED);
-		if (!fbuf)
-			return -ENOMEM;
+		error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
+				mp->m_bsize * M_IGEO(mp)->blocks_per_cluster,
+				XBF_UNMAPPED, &fbuf);
+		if (error)
+			return error;
 
 		/* Initialize the inode buffers and log them appropriately. */
 		fbuf->b_ops = &xfs_inode_buf_ops;
diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index d3d5e11d..687e33d8 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -1162,13 +1162,14 @@ xfs_sb_get_secondary(
 	struct xfs_buf		**bpp)
 {
 	struct xfs_buf		*bp;
+	int			error;
 
 	ASSERT(agno != 0 && agno != NULLAGNUMBER);
-	bp = xfs_trans_get_buf(tp, mp->m_ddev_targp,
+	error = xfs_trans_get_buf(tp, mp->m_ddev_targp,
 			XFS_AG_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
-			XFS_FSS_TO_BB(mp, 1), 0);
-	if (!bp)
-		return -ENOMEM;
+			XFS_FSS_TO_BB(mp, 1), 0, &bp);
+	if (error)
+		return error;
 	bp->b_ops = &xfs_sb_buf_ops;
 	xfs_buf_oneshot(bp);
 	*bpp = bp;
diff --git a/mkfs/proto.c b/mkfs/proto.c
index de5ae306..6eeedcd6 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -254,8 +254,14 @@ newfile(
 			exit(1);
 		}
 		d = XFS_FSB_TO_DADDR(mp, map.br_startblock);
-		bp = libxfs_trans_get_buf(logit ? tp : NULL, mp->m_dev, d,
-			nb << mp->m_blkbb_log, 0);
+		error = -libxfs_trans_get_buf(logit ? tp : NULL, mp->m_dev, d,
+				nb << mp->m_blkbb_log, 0, &bp);
+		if (error) {
+			fprintf(stderr,
+				_("%s: cannot allocate buffer for file\n"),
+				progname);
+			exit(1);
+		}
 		memmove(bp->b_addr, buf, len);
 		if (len < bp->b_bcount)
 			memset((char *)bp->b_addr + len, 0, bp->b_bcount - len);


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

* [PATCH 12/14] xfs: remove the xfs_btree_get_buf[ls] functions
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (10 preceding siblings ...)
  2020-02-20  1:45 ` [PATCH 11/14] xfs: make xfs_trans_get_buf " Darrick J. Wong
@ 2020-02-20  1:45 ` Darrick J. Wong
  2020-02-20  1:45 ` [PATCH 13/14] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers Darrick J. Wong
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:45 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs, Christoph Hellwig, Dave Chinner

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

Source kernel commit: ee647f85cb81b09bbfa2886954828ed03fa3ec38

Remove the xfs_btree_get_bufs and xfs_btree_get_bufl functions, since
they're pretty trivial oneliners.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/xfs_alloc.c |   16 +++++++++-------
 libxfs/xfs_bmap.c  |   14 +++++++++-----
 libxfs/xfs_btree.c |   46 ----------------------------------------------
 libxfs/xfs_btree.h |   21 ---------------------
 4 files changed, 18 insertions(+), 79 deletions(-)


diff --git a/libxfs/xfs_alloc.c b/libxfs/xfs_alloc.c
index 1bf813e3..be1bf736 100644
--- a/libxfs/xfs_alloc.c
+++ b/libxfs/xfs_alloc.c
@@ -1066,11 +1066,11 @@ xfs_alloc_ag_vextent_small(
 	if (args->datatype & XFS_ALLOC_USERDATA) {
 		struct xfs_buf	*bp;
 
-		bp = xfs_btree_get_bufs(args->mp, args->tp, args->agno, fbno);
-		if (XFS_IS_CORRUPT(args->mp, !bp)) {
-			error = -EFSCORRUPTED;
+		error = xfs_trans_get_buf(args->tp, args->mp->m_ddev_targp,
+				XFS_AGB_TO_DADDR(args->mp, args->agno, fbno),
+				args->mp->m_bsize, 0, &bp);
+		if (error)
 			goto error;
-		}
 		xfs_trans_binval(args->tp, bp);
 	}
 	*fbnop = args->agbno = fbno;
@@ -2343,9 +2343,11 @@ xfs_free_agfl_block(
 	if (error)
 		return error;
 
-	bp = xfs_btree_get_bufs(tp->t_mountp, tp, agno, agbno);
-	if (XFS_IS_CORRUPT(tp->t_mountp, !bp))
-		return -EFSCORRUPTED;
+	error = xfs_trans_get_buf(tp, tp->t_mountp->m_ddev_targp,
+			XFS_AGB_TO_DADDR(tp->t_mountp, agno, agbno),
+			tp->t_mountp->m_bsize, 0, &bp);
+	if (error)
+		return error;
 	xfs_trans_binval(tp, bp);
 
 	return 0;
diff --git a/libxfs/xfs_bmap.c b/libxfs/xfs_bmap.c
index 5efa2f0c..f917a6eb 100644
--- a/libxfs/xfs_bmap.c
+++ b/libxfs/xfs_bmap.c
@@ -723,11 +723,11 @@ xfs_bmap_extents_to_btree(
 	cur->bc_private.b.allocated++;
 	ip->i_d.di_nblocks++;
 	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT, 1L);
-	abp = xfs_btree_get_bufl(mp, tp, args.fsbno);
-	if (XFS_IS_CORRUPT(mp, !abp)) {
-		error = -EFSCORRUPTED;
+	error = xfs_trans_get_buf(tp, mp->m_ddev_targp,
+			XFS_FSB_TO_DADDR(mp, args.fsbno),
+			mp->m_bsize, 0, &abp);
+	if (error)
 		goto out_unreserve_dquot;
-	}
 
 	/*
 	 * Fill in the child block.
@@ -871,7 +871,11 @@ xfs_bmap_local_to_extents(
 	ASSERT(args.fsbno != NULLFSBLOCK);
 	ASSERT(args.len == 1);
 	tp->t_firstblock = args.fsbno;
-	bp = xfs_btree_get_bufl(args.mp, tp, args.fsbno);
+	error = xfs_trans_get_buf(tp, args.mp->m_ddev_targp,
+			XFS_FSB_TO_DADDR(args.mp, args.fsbno),
+			args.mp->m_bsize, 0, &bp);
+	if (error)
+		goto done;
 
 	/*
 	 * Initialize the block, copy the data and log the remote buffer.
diff --git a/libxfs/xfs_btree.c b/libxfs/xfs_btree.c
index aeaa9623..57862dfa 100644
--- a/libxfs/xfs_btree.c
+++ b/libxfs/xfs_btree.c
@@ -675,52 +675,6 @@ xfs_btree_get_block(
 	return XFS_BUF_TO_BLOCK(*bpp);
 }
 
-/*
- * Get a buffer for the block, return it with no data read.
- * Long-form addressing.
- */
-xfs_buf_t *				/* buffer for fsbno */
-xfs_btree_get_bufl(
-	xfs_mount_t	*mp,		/* file system mount point */
-	xfs_trans_t	*tp,		/* transaction pointer */
-	xfs_fsblock_t	fsbno)		/* file system block number */
-{
-	struct xfs_buf		*bp;
-	xfs_daddr_t		d;		/* real disk block address */
-	int			error;
-
-	ASSERT(fsbno != NULLFSBLOCK);
-	d = XFS_FSB_TO_DADDR(mp, fsbno);
-	error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, &bp);
-	if (error)
-		return NULL;
-	return bp;
-}
-
-/*
- * Get a buffer for the block, return it with no data read.
- * Short-form addressing.
- */
-xfs_buf_t *				/* buffer for agno/agbno */
-xfs_btree_get_bufs(
-	xfs_mount_t	*mp,		/* file system mount point */
-	xfs_trans_t	*tp,		/* transaction pointer */
-	xfs_agnumber_t	agno,		/* allocation group number */
-	xfs_agblock_t	agbno)		/* allocation group block number */
-{
-	struct xfs_buf		*bp;
-	xfs_daddr_t		d;		/* real disk block address */
-	int			error;
-
-	ASSERT(agno != NULLAGNUMBER);
-	ASSERT(agbno != NULLAGBLOCK);
-	d = XFS_AGB_TO_DADDR(mp, agno, agbno);
-	error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, &bp);
-	if (error)
-		return NULL;
-	return bp;
-}
-
 /*
  * Change the cursor to point to the first record at the given level.
  * Other levels are unaffected.
diff --git a/libxfs/xfs_btree.h b/libxfs/xfs_btree.h
index 2e727f23..8bead747 100644
--- a/libxfs/xfs_btree.h
+++ b/libxfs/xfs_btree.h
@@ -296,27 +296,6 @@ xfs_btree_dup_cursor(
 	xfs_btree_cur_t		*cur,	/* input cursor */
 	xfs_btree_cur_t		**ncur);/* output cursor */
 
-/*
- * Get a buffer for the block, return it with no data read.
- * Long-form addressing.
- */
-struct xfs_buf *				/* buffer for fsbno */
-xfs_btree_get_bufl(
-	struct xfs_mount	*mp,	/* file system mount point */
-	struct xfs_trans	*tp,	/* transaction pointer */
-	xfs_fsblock_t		fsbno);	/* file system block number */
-
-/*
- * Get a buffer for the block, return it with no data read.
- * Short-form addressing.
- */
-struct xfs_buf *				/* buffer for agno/agbno */
-xfs_btree_get_bufs(
-	struct xfs_mount	*mp,	/* file system mount point */
-	struct xfs_trans	*tp,	/* transaction pointer */
-	xfs_agnumber_t		agno,	/* allocation group number */
-	xfs_agblock_t		agbno);	/* allocation group block number */
-
 /*
  * Compute first and last byte offsets for the fields given.
  * Interprets the offsets table, which contains struct field offsets.


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

* [PATCH 13/14] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (11 preceding siblings ...)
  2020-02-20  1:45 ` [PATCH 12/14] xfs: remove the xfs_btree_get_buf[ls] functions Darrick J. Wong
@ 2020-02-20  1:45 ` Darrick J. Wong
  2020-02-20  1:45 ` [PATCH 14/14] xfs: remove unnecessary null pointer checks from _read_agf callers Darrick J. Wong
  2020-02-20  1:53 ` [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
  14 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:45 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs, Christoph Hellwig, Dave Chinner

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

Source kernel commit: f48e2df8a877ca1c19d92cfd7e74cc5956fa84cb

Refactor xfs_read_agf and xfs_alloc_read_agf to return EAGAIN if the
caller passed TRYLOCK and we weren't able to get the lock; and change
the callers to recognize this.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/xfs_alloc.c |   36 ++++++++++++++----------------------
 libxfs/xfs_bmap.c  |   11 ++++++-----
 2 files changed, 20 insertions(+), 27 deletions(-)


diff --git a/libxfs/xfs_alloc.c b/libxfs/xfs_alloc.c
index be1bf736..a92ca524 100644
--- a/libxfs/xfs_alloc.c
+++ b/libxfs/xfs_alloc.c
@@ -2498,12 +2498,11 @@ xfs_alloc_fix_freelist(
 
 	if (!pag->pagf_init) {
 		error = xfs_alloc_read_agf(mp, tp, args->agno, flags, &agbp);
-		if (error)
+		if (error) {
+			/* Couldn't lock the AGF so skip this AG. */
+			if (error == -EAGAIN)
+				error = 0;
 			goto out_no_agbp;
-		if (!pag->pagf_init) {
-			ASSERT(flags & XFS_ALLOC_FLAG_TRYLOCK);
-			ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));
-			goto out_agbp_relse;
 		}
 	}
 
@@ -2529,11 +2528,10 @@ xfs_alloc_fix_freelist(
 	 */
 	if (!agbp) {
 		error = xfs_alloc_read_agf(mp, tp, args->agno, flags, &agbp);
-		if (error)
-			goto out_no_agbp;
-		if (!agbp) {
-			ASSERT(flags & XFS_ALLOC_FLAG_TRYLOCK);
-			ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));
+		if (error) {
+			/* Couldn't lock the AGF so skip this AG. */
+			if (error == -EAGAIN)
+				error = 0;
 			goto out_no_agbp;
 		}
 	}
@@ -2764,11 +2762,10 @@ xfs_alloc_pagf_init(
 	xfs_buf_t		*bp;
 	int			error;
 
-	if ((error = xfs_alloc_read_agf(mp, tp, agno, flags, &bp)))
-		return error;
-	if (bp)
+	error = xfs_alloc_read_agf(mp, tp, agno, flags, &bp);
+	if (!error)
 		xfs_trans_brelse(tp, bp);
-	return 0;
+	return error;
 }
 
 /*
@@ -2957,12 +2954,6 @@ xfs_read_agf(
 	error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
 			XFS_AG_DADDR(mp, agno, XFS_AGF_DADDR(mp)),
 			XFS_FSS_TO_BB(mp, 1), flags, bpp, &xfs_agf_buf_ops);
-	/*
-	 * Callers of xfs_read_agf() currently interpret a NULL bpp as EAGAIN
-	 * and need to be converted to check for EAGAIN specifically.
-	 */
-	if (error == -EAGAIN)
-		return 0;
 	if (error)
 		return error;
 
@@ -2988,14 +2979,15 @@ xfs_alloc_read_agf(
 
 	trace_xfs_alloc_read_agf(mp, agno);
 
+	/* We don't support trylock when freeing. */
+	ASSERT((flags & (XFS_ALLOC_FLAG_FREEING | XFS_ALLOC_FLAG_TRYLOCK)) !=
+			(XFS_ALLOC_FLAG_FREEING | XFS_ALLOC_FLAG_TRYLOCK));
 	ASSERT(agno != NULLAGNUMBER);
 	error = xfs_read_agf(mp, tp, agno,
 			(flags & XFS_ALLOC_FLAG_TRYLOCK) ? XBF_TRYLOCK : 0,
 			bpp);
 	if (error)
 		return error;
-	if (!*bpp)
-		return 0;
 	ASSERT(!(*bpp)->b_error);
 
 	agf = XFS_BUF_TO_AGF(*bpp);
diff --git a/libxfs/xfs_bmap.c b/libxfs/xfs_bmap.c
index f917a6eb..d43155d0 100644
--- a/libxfs/xfs_bmap.c
+++ b/libxfs/xfs_bmap.c
@@ -3304,11 +3304,12 @@ xfs_bmap_longest_free_extent(
 	pag = xfs_perag_get(mp, ag);
 	if (!pag->pagf_init) {
 		error = xfs_alloc_pagf_init(mp, tp, ag, XFS_ALLOC_FLAG_TRYLOCK);
-		if (error)
-			goto out;
-
-		if (!pag->pagf_init) {
-			*notinit = 1;
+		if (error) {
+			/* Couldn't lock the AGF, so skip this AG. */
+			if (error == -EAGAIN) {
+				*notinit = 1;
+				error = 0;
+			}
 			goto out;
 		}
 	}


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

* [PATCH 14/14] xfs: remove unnecessary null pointer checks from _read_agf callers
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (12 preceding siblings ...)
  2020-02-20  1:45 ` [PATCH 13/14] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers Darrick J. Wong
@ 2020-02-20  1:45 ` Darrick J. Wong
  2020-02-20  1:53 ` [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
  14 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:45 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs, Christoph Hellwig, Dave Chinner

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

Source kernel commit: 706b8c5bc70391be510a5454f307db90b622b279

Drop the null buffer pointer checks in all code that calls
xfs_alloc_read_agf and doesn't pass XFS_ALLOC_FLAG_TRYLOCK because
they're no longer necessary.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/xfs_refcount.c |    6 ------
 1 file changed, 6 deletions(-)


diff --git a/libxfs/xfs_refcount.c b/libxfs/xfs_refcount.c
index 71d29486..b8a45b15 100644
--- a/libxfs/xfs_refcount.c
+++ b/libxfs/xfs_refcount.c
@@ -1176,8 +1176,6 @@ xfs_refcount_finish_one(
 				XFS_ALLOC_FLAG_FREEING, &agbp);
 		if (error)
 			return error;
-		if (XFS_IS_CORRUPT(tp->t_mountp, !agbp))
-			return -EFSCORRUPTED;
 
 		rcur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno);
 		if (!rcur) {
@@ -1717,10 +1715,6 @@ xfs_refcount_recover_cow_leftovers(
 	error = xfs_alloc_read_agf(mp, tp, agno, 0, &agbp);
 	if (error)
 		goto out_trans;
-	if (!agbp) {
-		error = -ENOMEM;
-		goto out_trans;
-	}
 	cur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno);
 
 	/* Find all the leftover CoW staging extents. */


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

* Re: [PATCH 00/14] xfsprogs: make buffer functions return error codes
  2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
                   ` (13 preceding siblings ...)
  2020-02-20  1:45 ` [PATCH 14/14] xfs: remove unnecessary null pointer checks from _read_agf callers Darrick J. Wong
@ 2020-02-20  1:53 ` Darrick J. Wong
  14 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-20  1:53 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs

On Wed, Feb 19, 2020 at 05:44:29PM -0800, Darrick J. Wong wrote:
> Hi all,
> 
> Let's fix all the xfs read/get buffer functions to return the usual
> integer error codes and pass the buffer pointer as a out-argument.  This
> makes it so that we can return useful error output instead of making
> callers infer ENOMEM or EAGAIN or whatever other reality they crave from
> the NULL pointer that they get when things don't go perfectly.
> 
> FWIW, all XBF_TRYLOCK callers must now trigger retries if they receive
> EAGAIN.  This may lead to a slight behavioral change in that TRYLOCK
> callers will no longer retry for things like ENOMEM, though I didn't see
> any obvious changes in user-visible behavior when running fstests.
> 
> After finishing the error code conversion, we straighten out the TRYLOCK
> callers to remove all this null pointer checks in favor of explicit
> EAGAIN checks; and then we change the buffer IO/corruption error
> reporting so that we report whoever called the buffer code even when
> reading a buffer in transaction context.
> 
> The userspace port of the kernel patchset was very difficult, so I am
> submitting this series to avoid dumping the work on Eric.

I should've noted that the "xfsprogs: refactor buffer function names"
series applies against 5.5; this series is a backport of the second half
of the kernel 5.6 merge.  I didn't send that part because I didn't feel
that it was significant enough to increase the patchbomb spew.

SO... the order is:

1. "refactor buffer function names", which is the thread before this one
2. "sync with 5.6", available at [1]
3. "make buffer functions return error codes", which is this one

--D

[1] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfsprogs-dev.git/tag/?h=libxfs-5.6-sync_2020-02-19

> If you're going to start using this mess, you probably ought to just
> pull from my git trees, which are linked below.
> 
> This is an extraordinary way to destroy everything.  Enjoy!
> Comments and questions are, as always, welcome.
> 
> --D
> 
> xfsprogs git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=buf-return-errorcodes

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

* Re: [PATCH 01/14] libxfs: make __cache_lookup return an error code
  2020-02-20  1:44 ` [PATCH 01/14] libxfs: make __cache_lookup return an error code Darrick J. Wong
@ 2020-02-21 14:55   ` Christoph Hellwig
  0 siblings, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2020-02-21 14:55 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: sandeen, linux-xfs

On Wed, Feb 19, 2020 at 05:44:35PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Convert __cache_lookup() to return numeric error codes like most
> everywhere else in xfsprogs.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 02/14] libxfs: make libxfs_getbuf_flags return an error code
  2020-02-20  1:44 ` [PATCH 02/14] libxfs: make libxfs_getbuf_flags " Darrick J. Wong
@ 2020-02-21 14:56   ` Christoph Hellwig
  0 siblings, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2020-02-21 14:56 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: sandeen, linux-xfs

On Wed, Feb 19, 2020 at 05:44:41PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Convert libxfs_getbuf_flags() to return numeric error codes like most
> everywhere else in xfsprogs.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 03/14] libxfs: make libxfs_buf_get_map return an error code
  2020-02-20  1:44 ` [PATCH 03/14] libxfs: make libxfs_buf_get_map " Darrick J. Wong
@ 2020-02-21 14:57   ` Christoph Hellwig
  2020-02-21 16:01     ` Darrick J. Wong
  0 siblings, 1 reply; 25+ messages in thread
From: Christoph Hellwig @ 2020-02-21 14:57 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: sandeen, linux-xfs

> @@ -120,8 +121,9 @@ pf_queue_io(
>  	 * the lock holder is either reading it from disk himself or
>  	 * completely overwriting it this behaviour is perfectly fine.

> -	bp = libxfs_buf_get_map(mp->m_dev, map, nmaps, LIBXFS_GETBUF_TRYLOCK);
> -	if (!bp)
> +	error = -libxfs_buf_get_map(mp->m_dev, map, nmaps, LIBXFS_GETBUF_TRYLOCK,

This adds an overly long line.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 04/14] libxfs: refactor libxfs_readbuf out of existence
  2020-02-20  1:44 ` [PATCH 04/14] libxfs: refactor libxfs_readbuf out of existence Darrick J. Wong
@ 2020-02-21 15:00   ` Christoph Hellwig
  2020-02-21 22:13     ` Darrick J. Wong
  0 siblings, 1 reply; 25+ messages in thread
From: Christoph Hellwig @ 2020-02-21 15:00 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: sandeen, linux-xfs

On Wed, Feb 19, 2020 at 05:44:54PM -0800, Darrick J. Wong wrote:
> +	/*
> +	 * if the buffer was prefetched, it is likely that it was not validated.

Please capitalize the first character in multi-line comments.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 05/14] libxfs: make libxfs_buf_read_map return an error code
  2020-02-20  1:45 ` [PATCH 05/14] libxfs: make libxfs_buf_read_map return an error code Darrick J. Wong
@ 2020-02-21 15:03   ` Christoph Hellwig
  2020-02-21 16:15     ` Darrick J. Wong
  0 siblings, 1 reply; 25+ messages in thread
From: Christoph Hellwig @ 2020-02-21 15:03 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: sandeen, linux-xfs

On Wed, Feb 19, 2020 at 05:45:01PM -0800, Darrick J. Wong wrote:
> @@ -1050,15 +1083,26 @@ libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
>  				flags);
>  	else
>  		error = libxfs_readbufr_map(btp, bp, flags);
> +	if (error == -EIO && salvage)
> +		goto ok;

I understand the part about skipping the verifiers.  But how does ignoring
EIO in this case fir the scheme?

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

* Re: [PATCH 03/14] libxfs: make libxfs_buf_get_map return an error code
  2020-02-21 14:57   ` Christoph Hellwig
@ 2020-02-21 16:01     ` Darrick J. Wong
  0 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-21 16:01 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: sandeen, linux-xfs

On Fri, Feb 21, 2020 at 06:57:43AM -0800, Christoph Hellwig wrote:
> > @@ -120,8 +121,9 @@ pf_queue_io(
> >  	 * the lock holder is either reading it from disk himself or
> >  	 * completely overwriting it this behaviour is perfectly fine.
> 
> > -	bp = libxfs_buf_get_map(mp->m_dev, map, nmaps, LIBXFS_GETBUF_TRYLOCK);
> > -	if (!bp)
> > +	error = -libxfs_buf_get_map(mp->m_dev, map, nmaps, LIBXFS_GETBUF_TRYLOCK,
> 
> This adds an overly long line.

Oops, will fix that.

--D

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 05/14] libxfs: make libxfs_buf_read_map return an error code
  2020-02-21 15:03   ` Christoph Hellwig
@ 2020-02-21 16:15     ` Darrick J. Wong
  2020-02-21 16:28       ` Christoph Hellwig
  0 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-21 16:15 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: sandeen, linux-xfs

On Fri, Feb 21, 2020 at 07:03:39AM -0800, Christoph Hellwig wrote:
> On Wed, Feb 19, 2020 at 05:45:01PM -0800, Darrick J. Wong wrote:
> > @@ -1050,15 +1083,26 @@ libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
> >  				flags);
> >  	else
> >  		error = libxfs_readbufr_map(btp, bp, flags);
> > +	if (error == -EIO && salvage)
> > +		goto ok;
> 
> I understand the part about skipping the verifiers.  But how does ignoring
> EIO in this case fir the scheme?

"Salvage" mode means that the caller wants an xfs_buf even if the
contents are invalid or missing due to EIO.  This is useful for db and
repair because we can fill the buffer with fixed or new metadata and
write it back to disk.

On a practical level this means I don't have to amend all callsites:

	err = libxfs_buf_read(...LIBXFS_READBUF_SALVAGE..., &bp);
	if (err == -EIO)
		err = libxfs_buf_get(..., &bp);
	if (err)
		goto barf;

...since EIO doesn't seem that much more special than EFSCORRUPTED.

--D

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

* Re: [PATCH 05/14] libxfs: make libxfs_buf_read_map return an error code
  2020-02-21 16:15     ` Darrick J. Wong
@ 2020-02-21 16:28       ` Christoph Hellwig
  0 siblings, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2020-02-21 16:28 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, sandeen, linux-xfs

On Fri, Feb 21, 2020 at 08:15:30AM -0800, Darrick J. Wong wrote:
> > I understand the part about skipping the verifiers.  But how does ignoring
> > EIO in this case fir the scheme?
> 
> "Salvage" mode means that the caller wants an xfs_buf even if the
> contents are invalid or missing due to EIO.  This is useful for db and
> repair because we can fill the buffer with fixed or new metadata and
> write it back to disk.

I thought the aim was to look at the buffer for the content even if
said content fails the verifiers, for which it makes sense to me.

> On a practical level this means I don't have to amend all callsites:
> 
> 	err = libxfs_buf_read(...LIBXFS_READBUF_SALVAGE..., &bp);
> 	if (err == -EIO)
> 		err = libxfs_buf_get(..., &bp);
> 	if (err)
> 		goto barf;
> 
> ...since EIO doesn't seem that much more special than EFSCORRUPTED.

I actually prefer that, as it really documents what is going on.  That
being said if reading the block fails with EIO chances are writing will
fail as well..

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

* Re: [PATCH 04/14] libxfs: refactor libxfs_readbuf out of existence
  2020-02-21 15:00   ` Christoph Hellwig
@ 2020-02-21 22:13     ` Darrick J. Wong
  0 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-02-21 22:13 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: sandeen, linux-xfs

On Fri, Feb 21, 2020 at 07:00:01AM -0800, Christoph Hellwig wrote:
> On Wed, Feb 19, 2020 at 05:44:54PM -0800, Darrick J. Wong wrote:
> > +	/*
> > +	 * if the buffer was prefetched, it is likely that it was not validated.
> 
> Please capitalize the first character in multi-line comments.

Will fix.

--D

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

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

end of thread, other threads:[~2020-02-21 22:13 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20  1:44 [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong
2020-02-20  1:44 ` [PATCH 01/14] libxfs: make __cache_lookup return an error code Darrick J. Wong
2020-02-21 14:55   ` Christoph Hellwig
2020-02-20  1:44 ` [PATCH 02/14] libxfs: make libxfs_getbuf_flags " Darrick J. Wong
2020-02-21 14:56   ` Christoph Hellwig
2020-02-20  1:44 ` [PATCH 03/14] libxfs: make libxfs_buf_get_map " Darrick J. Wong
2020-02-21 14:57   ` Christoph Hellwig
2020-02-21 16:01     ` Darrick J. Wong
2020-02-20  1:44 ` [PATCH 04/14] libxfs: refactor libxfs_readbuf out of existence Darrick J. Wong
2020-02-21 15:00   ` Christoph Hellwig
2020-02-21 22:13     ` Darrick J. Wong
2020-02-20  1:45 ` [PATCH 05/14] libxfs: make libxfs_buf_read_map return an error code Darrick J. Wong
2020-02-21 15:03   ` Christoph Hellwig
2020-02-21 16:15     ` Darrick J. Wong
2020-02-21 16:28       ` Christoph Hellwig
2020-02-20  1:45 ` [PATCH 06/14] xfs: make xfs_buf_read_map " Darrick J. Wong
2020-02-20  1:45 ` [PATCH 07/14] xfs: make xfs_buf_get " Darrick J. Wong
2020-02-20  1:45 ` [PATCH 08/14] xfs: make xfs_buf_get_uncached " Darrick J. Wong
2020-02-20  1:45 ` [PATCH 09/14] xfs: make xfs_buf_read " Darrick J. Wong
2020-02-20  1:45 ` [PATCH 10/14] xfs: make xfs_trans_get_buf_map " Darrick J. Wong
2020-02-20  1:45 ` [PATCH 11/14] xfs: make xfs_trans_get_buf " Darrick J. Wong
2020-02-20  1:45 ` [PATCH 12/14] xfs: remove the xfs_btree_get_buf[ls] functions Darrick J. Wong
2020-02-20  1:45 ` [PATCH 13/14] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers Darrick J. Wong
2020-02-20  1:45 ` [PATCH 14/14] xfs: remove unnecessary null pointer checks from _read_agf callers Darrick J. Wong
2020-02-20  1:53 ` [PATCH 00/14] xfsprogs: make buffer functions return error codes Darrick J. Wong

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.