Re: [PATCH 3/3] iomap: use filemap_range_needs_writeback() for O_DIRECT reads

From: Jens Axboe <axboe@kernel.dk>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	akpm@linux-foundation.org
Subject: Re: [PATCH 3/3] iomap: use filemap_range_needs_writeback() for O_DIRECT reads
Date: Tue, 9 Feb 2021 07:29:37 -0700	[thread overview]
Message-ID: <9fb1a43c-3d5f-be43-952e-b0462ed47b4c@kernel.dk> (raw)
In-Reply-To: <20210209075119.GC1696555@infradead.org>

On 2/9/21 12:51 AM, Christoph Hellwig wrote:
> On Mon, Feb 08, 2021 at 07:30:08PM -0700, Jens Axboe wrote:
>> +		if (iocb->ki_flags & IOCB_NOWAIT) {
>> +			if (filemap_range_needs_writeback(mapping, pos, end)) {
>> +				ret = -EAGAIN;
>> +				goto out_free_dio;
>> +			}
>> +			flags |= IOMAP_NOWAIT;
>> +		}
>>  		if (iter_is_iovec(iter))
>>  			dio->flags |= IOMAP_DIO_DIRTY;
>>  	} else {
>> +		if (iocb->ki_flags & IOCB_NOWAIT) {
>> +			if (filemap_range_has_page(mapping, pos, end)) {
>> +				ret = -EAGAIN;
>> +				goto out_free_dio;
>> +			}
>> +			flags |= IOMAP_NOWAIT;
>> +		}
>> +
>>  		flags |= IOMAP_WRITE;
>>  		dio->flags |= IOMAP_DIO_WRITE;
>>  
>> @@ -478,14 +493,6 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>>  			dio->flags |= IOMAP_DIO_WRITE_FUA;
>>  	}
>>  
>> -	if (iocb->ki_flags & IOCB_NOWAIT) {
>> -		if (filemap_range_has_page(mapping, pos, end)) {
>> -			ret = -EAGAIN;
>> -			goto out_free_dio;
>> -		}
>> -		flags |= IOMAP_NOWAIT;
>> -	}
> 
> looking at this I really hate the scheme with the potential racyness
> and duplicated page looksups.

Me too

> Why can't we pass a nonblock flag to filemap_write_and_wait_range
> and invalidate_inode_pages2_range that makes them return -EAGAIN
> when they would block to clean this whole mess up?

We could, but that's a _lot_ of surgery. I'd rather live with the
slight race for now instead of teaching writepages, page laundering,
etc about IOCB_NOWAIT.

I do think that's a worthy long term goal, but we dio read situation
is bad enough that it warrants a quicker fix.

-- 
Jens Axboe