Re: [PATCH 11/13] iomap: make buffered writes work with RWF_UNCACHED

[Jens Axboe <axboe@kernel.dk>]

On 11/8/24 1:07 PM, Matthew Wilcox wrote:
> On Fri, Nov 08, 2024 at 12:49:58PM -0700, Jens Axboe wrote:
>> On 11/8/24 12:26 PM, Jens Axboe wrote:
>>> On 11/8/24 11:46 AM, Matthew Wilcox wrote:
>>>> On Fri, Nov 08, 2024 at 10:43:34AM -0700, Jens Axboe wrote:
>>>>> +++ b/fs/iomap/buffered-io.c
>>>>> @@ -959,6 +959,8 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
>>>>>  		}
>>>>>  		if (iter->iomap.flags & IOMAP_F_STALE)
>>>>>  			break;
>>>>> +		if (iter->flags & IOMAP_UNCACHED)
>>>>> +			folio_set_uncached(folio);
>>>>
>>>> This seems like it'd convert an existing page cache folio into being
>>>> uncached?  Is this just leftover from a previous version or is that a
>>>> design decision you made?
>>>
>>> I'll see if we can improve that. Currently both the read and write side
>>> do drop whatever it touches. We could feasibly just have it drop
>>> newly instantiated pages - iow, uncached just won't create new persistent
>>> folios, but it'll happily use the ones that are there already.
>>
>> Well that was nonsense on the read side, it deliberately only prunes
>> entries that has uncached set. For the write side, this is a bit
>> trickier. We'd essentially need to know if the folio populated by
>> write_begin was found in the page cache, or create from new. Any way we
>> can do that? One way is to change ->write_begin() so it takes a kiocb
>> rather than a file, but that's an amount of churn I'd rather avoid!
>> Maybe there's a way I'm just not seeing?
> 
> Umm.  We can solve it for iomap with a new FGP_UNCACHED flag and
> checking IOMAP_UNCACHED in iomap_get_folio().  Not sure how we solve it
> for other filesystems though.  Any filesystem which uses FGP_NOWAIT has
> _a_ solution, but eg btrfs will need to plumb through a third boolean
> flag (or, more efficiently, just start passing FGP flags to
> prepare_one_folio()).

Yeah that's true, forgot we already have the IOMAP_UNCACHED flag there
and it's available in creation. Thanks, I'll start with that.

-- 
Jens Axboe