xfs_buftarg_isolate(): "Correctly invert xfs_buftarg LRU isolation logic"

["Alex Lyakas" <alex@zadara.com>]

Hello Vratislav, Brian,

This is with regards to commit "xfs: Correctly invert xfs_buftarg LRU 
isolation logic" [1].

I am hitting this issue in kernel 4.14. However, after some debugging, I do 
not fully agree with the commit message, describing the effect of this 
defect.

In case b_lru_ref > 1, then indeed this xfs_buf will be taken off the LRU 
list, and immediately added back to it, with b_lru_ref being lesser by 1 
now.

In case b_lru_ref==1, then this xfs_buf will be similarly isolated (due to a 
bug), and xfs_buf_rele() will be called on it. But now its b_lru_ref==0. In 
this case, xfs_buf_rele() will free the buffer, rather than re-adding it 
back to the LRU. This is a problem, because we intended for this buffer to 
have another trip on the LRU. Only when b_lru_ref==0 upon entry to 
xfs_buftarg_isolate(), we want to free the buffer. So we are freeing the 
buffer one trip too early in this case.

In case b_lru_ref==0 (somehow), then due to a bug, this xfs_buf will not be 
removed off the LRU. It will remain sitting in the LRU with b_lru_ref==0. On 
next shrinker call, this xfs_buff will also remain on the LRU, due to the 
same bug. So this xfs_buf will be freed only on unmount or if 
xfs_buf_stale() is called on it.

Do you agree with the above?

If so, I think this fix should be backported to stable kernels.

Thanks,
Alex.

[1]
commit 19957a181608d25c8f4136652d0ea00b3738972d
Author: Vratislav Bendel <vbendel@redhat.com>
Date:   Tue Mar 6 17:07:44 2018 -0800

    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 <vbendel@redhat.com>
    Reviewed-by: Brian Foster <bfoster@redhat.com>
    Reviewed-by: Christoph Hellwig <hch@lst.de>
    Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
    Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>