| Message ID | 20230825135431.1317785-25-hao.xu@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | io_uring getdents | expand |
On Fri, Aug 25, 2023 at 09:54:26PM +0800, Hao Xu wrote: > From: Hao Xu <howeyxu@tencent.com> > > This causes xfstests generic/232 hung in umount process, waiting for ail > push, so I comment it for now, need some hints from xfs folks. > Not a real patch. > > Signed-off-by: Hao Xu <howeyxu@tencent.com> > --- > fs/xfs/xfs_buf.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c > index cdad80e1ae25..284962a9f31a 100644 > --- a/fs/xfs/xfs_buf.c > +++ b/fs/xfs/xfs_buf.c > @@ -828,6 +828,13 @@ xfs_buf_read_map( > trace_xfs_buf_read(bp, flags, _RET_IP_); > > if (!(bp->b_flags & XBF_DONE)) { > +// /* > +// * Let's bypass the _xfs_buf_read() for now > +// */ > +// if (flags & XBF_NOWAIT) { > +// xfs_buf_relse(bp); > +// return -EAGAIN; > +// } This is *fundamentally broken*, and apart from anything else breaks readahead. IF we asked for a read, we cannot instantiate the buffer and then *not issue any IO on it* and release it. That leaves an uninitialised buffer in memory, and there's every chance that something then trips over it and bad things happen. A buffer like this *must* be errored out and marked stale so that the next access to it will then re-initialise the buffer state and trigger any preparatory work that needs to be done for the new operation. This comes back to my first comments that XBF_TRYLOCK cannot simpy be replaced with XBF_NOWAIT semantics... -Dave.
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index cdad80e1ae25..284962a9f31a 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -828,6 +828,13 @@ xfs_buf_read_map( trace_xfs_buf_read(bp, flags, _RET_IP_); if (!(bp->b_flags & XBF_DONE)) { +// /* +// * Let's bypass the _xfs_buf_read() for now +// */ +// if (flags & XBF_NOWAIT) { +// xfs_buf_relse(bp); +// return -EAGAIN; +// } /* Initiate the buffer read and wait. */ XFS_STATS_INC(target->bt_mount, xb_get_read); bp->b_ops = ops;