* [PATCH] xfs: fix race while discarding buffers
@ 2012-08-06 15:26 Carlos Maiolino
2012-08-06 15:48 ` Carlos Maiolino
0 siblings, 1 reply; 2+ messages in thread
From: Carlos Maiolino @ 2012-08-06 15:26 UTC (permalink / raw)
To: xfs; +Cc: Carlos Maiolino
While xfs_buftarg_shrink() is freeing buffers from the dispose list (filled with
buffers from lru list), there is a possibility to have xfs_buf_stale() racing
with it, and removing buffers from dispose list before xfs_buftarg_shrink() does
it.
This happens because xfs_buftarg_shrink() handle the dispose list without
locking and since the test condition in xfs_buf_stale()
-- if (!list_empty(&bp->b_lru)) -- checks for the buffer being in *any* list, if
the buffer happens to be on dispose list, this causes the buffer counter of the
lru list (btp->bt_lru_nr) to be decremented twice (once in xfs_buftarg_shrink()
and another in xfs_buf_stale()) causing a wrong account usage of the lru list,
which may cause xfs_buftarg_shrink() to return a wrong value to the memory
shrinker function (shrink_slab()), such account error may also cause a
underflowed value to be returned; since the counter is lower than the current
number of items in the lru list, a decrement may happen when the counter is 0,
causing an underflow since the counter is an unsigned value.
The fix uses a new flag field (and a new buffer flag) to serialize buffer
handling during the shrink process. The new flag fied has been designed to use
btp->bt_lru_lock/unlock instead of xfs_buf_lock/unlock mechanism.
Thanks to dchinner, sandeen and aris for the help and hints on this
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
fs/xfs/xfs_buf.c | 7 ++++++-
fs/xfs/xfs_buf.h | 7 ++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index d7a9dd7..52b27c4 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -96,6 +96,8 @@ xfs_buf_lru_add(
atomic_inc(&bp->b_hold);
list_add_tail(&bp->b_lru, &btp->bt_lru);
btp->bt_lru_nr++;
+ if (bp->b_lru_flags & XBF_LRU_DISPOSE)
+ bp->b_lru_flags &= ~XBF_LRU_DISPOSE;
}
spin_unlock(&btp->bt_lru_lock);
}
@@ -154,7 +156,8 @@ xfs_buf_stale(
struct xfs_buftarg *btp = bp->b_target;
spin_lock(&btp->bt_lru_lock);
- if (!list_empty(&bp->b_lru)) {
+ if (!list_empty(&bp->b_lru) &&
+ !(bp->b_lru_flags & XBF_LRU_DISPOSE)) {
list_del_init(&bp->b_lru);
btp->bt_lru_nr--;
atomic_dec(&bp->b_hold);
@@ -228,6 +231,7 @@ _xfs_buf_alloc(
XB_SET_OWNER(bp);
bp->b_target = target;
bp->b_flags = flags;
+ bp->b_lru_flags = 0;
/*
* Set length and io_length to the same value initially.
@@ -1500,6 +1504,7 @@ xfs_buftarg_shrink(
* lock round trip inside xfs_buf_rele().
*/
list_move(&bp->b_lru, &dispose);
+ bp->b_lru_flags |= XBF_LRU_DISPOSE;
btp->bt_lru_nr--;
}
spin_unlock(&btp->bt_lru_lock);
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index d03b73b..ec0a17e 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -60,6 +60,9 @@ typedef enum {
#define _XBF_DELWRI_Q (1 << 22)/* buffer on a delwri queue */
#define _XBF_COMPOUND (1 << 23)/* compound buffer */
+/* flags used to handle lru items */
+#define XBF_LRU_DISPOSE (1 << 24) /* buffer being discarded */
+
typedef unsigned int xfs_buf_flags_t;
#define XFS_BUF_FLAGS \
@@ -77,7 +80,8 @@ typedef unsigned int xfs_buf_flags_t;
{ _XBF_PAGES, "PAGES" }, \
{ _XBF_KMEM, "KMEM" }, \
{ _XBF_DELWRI_Q, "DELWRI_Q" }, \
- { _XBF_COMPOUND, "COMPOUND" }
+ { _XBF_COMPOUND, "COMPOUND" }, \
+ { XBF_LRU_DISPOSE, "LRU_DISPOSE" }
typedef struct xfs_buftarg {
dev_t bt_dev;
@@ -122,6 +126,7 @@ typedef struct xfs_buf {
atomic_t b_hold; /* reference count */
atomic_t b_lru_ref; /* lru reclaim ref count */
xfs_buf_flags_t b_flags; /* status flags */
+ xfs_buf_flags_t b_lru_flags; /* internal lru status flags */
struct semaphore b_sema; /* semaphore for lockables */
struct list_head b_lru; /* lru list */
--
1.7.11.2
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] xfs: fix race while discarding buffers
2012-08-06 15:26 [PATCH] xfs: fix race while discarding buffers Carlos Maiolino
@ 2012-08-06 15:48 ` Carlos Maiolino
0 siblings, 0 replies; 2+ messages in thread
From: Carlos Maiolino @ 2012-08-06 15:48 UTC (permalink / raw)
To: xfs
Please, ignore this patch.
I've made a mistake on this patch, and I'm sending a V2 patch to fix the
error signed below:
> fs/xfs/xfs_buf.c | 7 ++++++-
> fs/xfs/xfs_buf.h | 7 ++++++-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index d7a9dd7..52b27c4 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -96,6 +96,8 @@ xfs_buf_lru_add(
> atomic_inc(&bp->b_hold);
> list_add_tail(&bp->b_lru, &btp->bt_lru);
> btp->bt_lru_nr++;
> + if (bp->b_lru_flags & XBF_LRU_DISPOSE)
^^^^^^
This should be if (!(b_lru_flags & XBF_LRU_DISPOSE))
I'm sending the V2 patch. Sorry about that.
--
--Carlos
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-08-06 15:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-06 15:26 [PATCH] xfs: fix race while discarding buffers Carlos Maiolino
2012-08-06 15:48 ` Carlos Maiolino
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.