This is a note to let you know that I've just added the patch titled
xfs: bulkstat chunk formatting cursor is broken
to the 3.17-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
xfs-bulkstat-chunk-formatting-cursor-is-broken.patch
and it can be found in the queue-3.17 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From bf4a5af20d25ecc8876978ad34b8db83b4235f3c Mon Sep 17 00:00:00 2001
From: Dave Chinner <dchinner@xxxxxxxxxx>
Date: Fri, 7 Nov 2014 08:30:30 +1100
Subject: xfs: bulkstat chunk formatting cursor is broken
From: Dave Chinner <dchinner@xxxxxxxxxx>
commit bf4a5af20d25ecc8876978ad34b8db83b4235f3c upstream.
The xfs_bulkstat_agichunk formatting cursor takes buffer values from
the main loop and passes them via the structure to the chunk
formatter, and the writes the changed values back into the main loop
local variables. Unfortunately, this complex dance is full of corner
cases that aren't handled correctly.
The biggest problem is that it is double handling the information in
both the main loop and the chunk formatting function, leading to
inconsistent updates and endless loops where progress is not made.
To fix this, push the struct xfs_bulkstat_agichunk outwards to be
the primary holder of user buffer information. this removes the
double handling in the main loop.
Also, pass the last inode processed by the chunk formatter as a
separate parameter as it purely an output variable and is not
related to the user buffer consumption cursor.
Finally, the chunk formatting code is not shared by anyone, so make
it local to xfs_itable.c.
Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
Signed-off-by: Dave Chinner <david@xxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
fs/xfs/xfs_itable.c | 59 ++++++++++++++++++++++++----------------------------
fs/xfs/xfs_itable.h | 16 --------------
2 files changed, 28 insertions(+), 47 deletions(-)
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -262,20 +262,26 @@ xfs_bulkstat_grab_ichunk(
#define XFS_BULKSTAT_UBLEFT(ubleft) ((ubleft) >= statstruct_size)
+struct xfs_bulkstat_agichunk {
+ char __user **ac_ubuffer;/* pointer into user's buffer */
+ int ac_ubleft; /* bytes left in user's buffer */
+ int ac_ubelem; /* spaces used in user's buffer */
+};
+
/*
* Process inodes in chunk with a pointer to a formatter function
* that will iget the inode and fill in the appropriate structure.
*/
-int
+static int
xfs_bulkstat_ag_ichunk(
struct xfs_mount *mp,
xfs_agnumber_t agno,
struct xfs_inobt_rec_incore *irbp,
bulkstat_one_pf formatter,
size_t statstruct_size,
- struct xfs_bulkstat_agichunk *acp)
+ struct xfs_bulkstat_agichunk *acp,
+ xfs_ino_t *lastino)
{
- xfs_ino_t lastino = acp->ac_lastino;
char __user **ubufp = acp->ac_ubuffer;
int ubleft = acp->ac_ubleft;
int ubelem = acp->ac_ubelem;
@@ -295,7 +301,7 @@ xfs_bulkstat_ag_ichunk(
/* Skip if this inode is free */
if (XFS_INOBT_MASK(chunkidx) & irbp->ir_free) {
- lastino = ino;
+ *lastino = ino;
continue;
}
@@ -313,7 +319,7 @@ xfs_bulkstat_ag_ichunk(
ubleft = 0;
break;
}
- lastino = ino;
+ *lastino = ino;
continue;
}
if (fmterror == BULKSTAT_RV_GIVEUP) {
@@ -325,10 +331,9 @@ xfs_bulkstat_ag_ichunk(
*ubufp += ubused;
ubleft -= ubused;
ubelem++;
- lastino = ino;
+ *lastino = ino;
}
- acp->ac_lastino = lastino;
acp->ac_ubleft = ubleft;
acp->ac_ubelem = ubelem;
@@ -355,7 +360,6 @@ xfs_bulkstat(
xfs_btree_cur_t *cur; /* btree cursor for ialloc btree */
int end_of_ag; /* set if we've seen the ag end */
int error; /* error code */
- int fmterror;/* bulkstat formatter result */
int icount; /* count of inodes good in irbuf */
size_t irbsize; /* size of irec buffer in bytes */
xfs_ino_t ino; /* inode number (filesystem) */
@@ -366,10 +370,8 @@ xfs_bulkstat(
int nirbuf; /* size of irbuf */
int rval; /* return value error code */
int ubcount; /* size of user's buffer */
- int ubleft; /* bytes left in user's buffer */
- char __user *ubufp; /* pointer into user's buffer */
- int ubelem; /* spaces used in user's buffer */
int stat;
+ struct xfs_bulkstat_agichunk ac;
/*
* Get the last inode value, see if there's nothing to do.
@@ -386,11 +388,13 @@ xfs_bulkstat(
}
ubcount = *ubcountp; /* statstruct's */
- ubleft = ubcount * statstruct_size; /* bytes */
- *ubcountp = ubelem = 0;
+ ac.ac_ubuffer = &ubuffer;
+ ac.ac_ubleft = ubcount * statstruct_size; /* bytes */;
+ ac.ac_ubelem = 0;
+
+ *ubcountp = 0;
*done = 0;
- fmterror = 0;
- ubufp = ubuffer;
+
irbuf = kmem_zalloc_greedy(&irbsize, PAGE_SIZE, PAGE_SIZE * 4);
if (!irbuf)
return -ENOMEM;
@@ -402,7 +406,7 @@ xfs_bulkstat(
* inode returned; 0 means start of the allocation group.
*/
rval = 0;
- while (XFS_BULKSTAT_UBLEFT(ubleft) && agno < mp->m_sb.sb_agcount) {
+ while (XFS_BULKSTAT_UBLEFT(ac.ac_ubleft) && agno < mp->m_sb.sb_agcount) {
cond_resched();
error = xfs_ialloc_read_agi(mp, NULL, agno, &agbp);
if (error)
@@ -497,28 +501,21 @@ del_cursor:
*/
irbufend = irbp;
for (irbp = irbuf;
- irbp < irbufend && XFS_BULKSTAT_UBLEFT(ubleft); irbp++) {
- struct xfs_bulkstat_agichunk ac;
-
- ac.ac_lastino = lastino;
- ac.ac_ubuffer = &ubuffer;
- ac.ac_ubleft = ubleft;
- ac.ac_ubelem = ubelem;
+ irbp < irbufend && XFS_BULKSTAT_UBLEFT(ac.ac_ubleft);
+ irbp++) {
error = xfs_bulkstat_ag_ichunk(mp, agno, irbp,
- formatter, statstruct_size, &ac);
+ formatter, statstruct_size, &ac,
+ &lastino);
if (error)
rval = error;
- lastino = ac.ac_lastino;
- ubleft = ac.ac_ubleft;
- ubelem = ac.ac_ubelem;
-
cond_resched();
}
+
/*
* Set up for the next loop iteration.
*/
- if (XFS_BULKSTAT_UBLEFT(ubleft)) {
+ if (XFS_BULKSTAT_UBLEFT(ac.ac_ubleft)) {
if (end_of_ag) {
agno++;
agino = 0;
@@ -531,11 +528,11 @@ del_cursor:
* Done, we're either out of filesystem or space to put the data.
*/
kmem_free(irbuf);
- *ubcountp = ubelem;
+ *ubcountp = ac.ac_ubelem;
/*
* Found some inodes, return them now and return the error next time.
*/
- if (ubelem)
+ if (ac.ac_ubelem)
rval = 0;
if (agno >= mp->m_sb.sb_agcount) {
/*
--- a/fs/xfs/xfs_itable.h
+++ b/fs/xfs/xfs_itable.h
@@ -30,22 +30,6 @@ typedef int (*bulkstat_one_pf)(struct xf
int *ubused,
int *stat);
-struct xfs_bulkstat_agichunk {
- xfs_ino_t ac_lastino; /* last inode returned */
- char __user **ac_ubuffer;/* pointer into user's buffer */
- int ac_ubleft; /* bytes left in user's buffer */
- int ac_ubelem; /* spaces used in user's buffer */
-};
-
-int
-xfs_bulkstat_ag_ichunk(
- struct xfs_mount *mp,
- xfs_agnumber_t agno,
- struct xfs_inobt_rec_incore *irbp,
- bulkstat_one_pf formatter,
- size_t statstruct_size,
- struct xfs_bulkstat_agichunk *acp);
-
/*
* Values for stat return value.
*/
Patches currently in stable-queue which might be from dchinner@xxxxxxxxxx are
queue-3.17/xfs-bulkstat-main-loop-logic-is-a-mess.patch
queue-3.17/xfs-bulkstat-doesn-t-release-agi-buffer-on-error.patch
queue-3.17/xfs-bulkstat-chunk-formatter-has-issues.patch
queue-3.17/xfs-track-bulkstat-progress-by-agino.patch
queue-3.17/mm-remove-false-warn_on-from-pagecache_isize_extended.patch
queue-3.17/xfs-bulkstat-chunk-formatting-cursor-is-broken.patch
queue-3.17/xfs-bulkstat-error-handling-is-broken.patch
queue-3.17/xfs-bulkstat-btree-walk-doesn-t-terminate.patch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html