xfs
[Top] [All Lists]

Re: [PATCH 4/7] xfs: split direct IO write path from xfs_file_aio_write

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 4/7] xfs: split direct IO write path from xfs_file_aio_write
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 16 Dec 2010 07:06:29 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1292376208-16282-5-git-send-email-david@xxxxxxxxxxxxx>
References: <1292376208-16282-1-git-send-email-david@xxxxxxxxxxxxx> <1292376208-16282-5-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
> +STATIC ssize_t
> +xfs_file_dio_aio_write(
> +     struct kiocb            *iocb,
> +     const struct iovec      *iovp,
> +     unsigned long           nr_segs,
> +     loff_t                  pos,
> +     size_t                  ocount,
> +     int                     *need_i_mutex,
> +     int                     *iolock)

need_i_mutex == 1 is equivalent to iolock == XFS_IOLOCK_EXCL, I think
it's a good idea to throw in a patch to remove the need_i_mutex variable
before this patch.

Maybe that patch can even add xfs_rw_lock or similar helpers to
manipulate the i_mutex, the iolock, and the iolock variable together?

Speaking of that, shouldn't xfs_file_aio_read also take the iolock
exclusive during the page invalidation and then demote it, just like
the write case?  The above helpers would enforce that nicely.

> +     if ((pos & target->bt_smask) || (count & target->bt_smask))
> +             return XFS_ERROR(-EINVAL);

For the error traps to work this needs to be

        -XFS_ERROR(EINVAL);

<Prev in Thread] Current Thread [Next in Thread>