Skip to contentSkip to conversation

xfs: Correctly invert xfs_buftarg LRU isolation logic

PATCH In review Vratislav Bendel 3 participants 3 messages Mar 6, 2018 – Mar 8, 2018 XFS filesystem

lore
Share & tools
lore

Vratislav Bendel

author
#0 lore raw

PATCH 1/1 xfs: Correctly invert xfs_buftarg LRU isolation logic

Due to an inverted logic mistake in xfs_buftarg_isolate() the xfs_buffers with zero b_lru_ref will take another trip around LRU, while isolating buffers with non-zero b_lru_ref.

Additionally those isolated buffers end up right back on the LRU once they are released, because b_lru_ref remains elevated.

Fix that circuitous route by leaving them on the LRU as originally intended.

Signed-off-by Vratislav Bendel <[email protected]>
Reviewed-by Brian Foster <[email protected]>
Reviewed-by Darrick J. Wong <[email protected]>
---
 fs/xfs/xfs_buf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_buf.c +1 -1
index d1da2ee9e6db..ac669a10c62f 100644
@@ -1708,7 +1708,7 @@ xfs_buftarg_isolate(
 	 * zero. If the value is already zero, we need to reclaim the
 	 * buffer, otherwise it gets another trip through the LRU.
 	 */
-	if (!atomic_add_unless(&bp->b_lru_ref, -1, 0)) {
+	if (atomic_add_unless(&bp->b_lru_ref, -1, 0)) {
 		spin_unlock(&bp->b_lock);
 		return LRU_ROTATE;
 	}
Signature · 1 line
2.14.3
Reviewed-by on v1 1/1
#1 lore raw
Quoted 7 lines · Vratislav Bendel

Due to an inverted logic mistake in xfs_buftarg_isolate() the xfs_buffers with zero b_lru_ref will take another trip around LRU, while isolating buffers with non-zero b_lru_ref.

Additionally those isolated buffers end up right back on the LRU once they are released, because b_lru_ref remains elevated.

Quoted 6 lines

Fix that circuitous route by leaving them on the LRU as originally intended.

Signed-off-by Vratislav Bendel <[email protected]>
Reviewed-by Brian Foster <[email protected]>
Reviewed-by Darrick J. Wong <[email protected]>

Looks ok, tests ok...

Reviewed-by Darrick J. Wong <[email protected]>

--D

Quoted 24 lines
---
 fs/xfs/xfs_buf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_buf.c +1 -1
index d1da2ee9e6db..ac669a10c62f 100644
@@ -1708,7 +1708,7 @@ xfs_buftarg_isolate(
 	 * zero. If the value is already zero, we need to reclaim the
 	 * buffer, otherwise it gets another trip through the LRU.
 	 */
-	if (!atomic_add_unless(&bp->b_lru_ref, -1, 0)) {
+	if (atomic_add_unless(&bp->b_lru_ref, -1, 0)) {
 		spin_unlock(&bp->b_lock);
 		return LRU_ROTATE;
 	}
Signature · 6 lines
2.14.3

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