On 04/06/2017 05:54 PM, Darrick J. Wong wrote:
> On Mon, Apr 03, 2017 at 11:52:11PM -0700, Christoph Hellwig wrote:
>>> + if (unaligned_io) {
>>> + /* If we are going to wait for other DIO to finish, bail */
>>> + if ((iocb->ki_flags & IOCB_NOWAIT) &&
>>> + atomic_read(&inode->i_dio_count))
>>> + return -EAGAIN;
>>> inode_dio_wait(inode);
>>
>> This checks i_dio_count twice in the nowait case, I think it should be:
>>
>> if (iocb->ki_flags & IOCB_NOWAIT) {
>> if (atomic_read(&inode->i_dio_count))
>> return -EAGAIN;
>> } else {
>> inode_dio_wait(inode);
>> }
>>
>>> if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
>>> if (flags & IOMAP_DIRECT) {
>>> + /* A reflinked inode will result in CoW alloc */
>>> + if (flags & IOMAP_NOWAIT) {
>>> + error = -EAGAIN;
>>> + goto out_unlock;
>>> + }
>>
>> This is a bit pessimistic - just because the inode has any shared
>> extents we could still write into unshared ones. For now I think this
>> pessimistic check is fine, but the comment should be corrected.
>
> Consider what happens in both _reflink_{allocate,reserve}_cow. If there
> is already an existing reservation in the CoW fork then we'll have to
> CoW and therefore can't satisfy the NOWAIT flag. If there isn't already
> anything in the CoW fork, then we have to see if there are shared blocks
> by calling _reflink_trim_around_shared. That performs a refcountbt
> lookup, which involves locking the AGF, so we also can't satisfy NOWAIT.
>
> IOWs, I think this hunk has to move outside the IOMAP_DIRECT check to
> cover both write-to-reflinked-file cases.
>
IOMAP_NOWAIT is set only with IOMAP_DIRECT since the nowait feature is
for direct-IO only. This is checked early on, when we are checking for
user-passed flags, and if not, -EINVAL is returned.
--
Goldwyn
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
