On Thu, Apr 28, 2022 at 12:58:59PM -0700, Stefan Roesch wrote:
>
>
> On 4/26/22 3:56 PM, Dave Chinner wrote:
> > On Tue, Apr 26, 2022 at 10:43:28AM -0700, Stefan Roesch wrote:
> >> This adds the async buffered write support to XFS. For async buffered
> >> write requests, the request will return -EAGAIN if the ilock cannot be
> >> obtained immediately.
> >>
> >> Signed-off-by: Stefan Roesch <shr@xxxxxx>
> >> ---
> >> fs/xfs/xfs_file.c | 10 ++++++----
> >> 1 file changed, 6 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> >> index 6f9da1059e8b..49d54b939502 100644
> >> --- a/fs/xfs/xfs_file.c
> >> +++ b/fs/xfs/xfs_file.c
> >> @@ -739,12 +739,14 @@ xfs_file_buffered_write(
> >> bool cleared_space = false;
> >> int iolock;
> >>
> >> - if (iocb->ki_flags & IOCB_NOWAIT)
> >> - return -EOPNOTSUPP;
> >> -
> >> write_retry:
> >> iolock = XFS_IOLOCK_EXCL;
> >> - xfs_ilock(ip, iolock);
> >> + if (iocb->ki_flags & IOCB_NOWAIT) {
> >> + if (!xfs_ilock_nowait(ip, iolock))
> >> + return -EAGAIN;
> >> + } else {
> >> + xfs_ilock(ip, iolock);
> >> + }
> >
> > xfs_ilock_iocb().
> >
>
> The helper xfs_ilock_iocb cannot be used as it hardcoded to use iocb->ki_filp to
> get a pointer to the xfs_inode.
And the problem with that is?
I mean, look at what xfs_file_buffered_write() does to get the
xfs_inode 10 lines about that change:
xfs_file_buffered_write(
struct kiocb *iocb,
struct iov_iter *from)
{
struct file *file = iocb->ki_filp;
struct address_space *mapping = file->f_mapping;
struct inode *inode = mapping->host;
struct xfs_inode *ip = XFS_I(inode);
In what cases does file_inode(iocb->ki_filp) point to a different
inode than iocb->ki_filp->f_mapping->host? The dio write path assumes
that file_inode(iocb->ki_filp) is correct, as do both the buffered
and dio read paths.
What makes the buffered write path special in that
file_inode(iocb->ki_filp) is not correctly set whilst
iocb->ki_filp->f_mapping->host is?
Regardless, if this is a problem, then just pass the XFS inode to
xfs_ilock_iocb() and this is a moot point.
> However here we need to use iocb->ki_filp->f_mapping->host.
> I'll split off new helper for this in the next version of the patch.
We don't need a new helper here, either.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
