Re: [PATCH] btrfs: add support for uncached writes

[Jens Axboe <axboe@kernel.dk>]

On 2/10/25 10:26 AM, David Sterba wrote:
> On Mon, Feb 03, 2025 at 09:35:42AM -0700, Jens Axboe wrote:
>> The read side is already covered as btrfs uses the generic filemap
>> helpers. For writes, just pass in FGP_DONTCACHE if uncached IO is being
>> done, then the folios created should be marked appropriately.
>>
>> For IO completion, ensure that writing back folios that are uncached
>> gets punted to one of the btrfs workers, as task context is needed for
>> that. Add an 'uncached_io' member to struct btrfs_bio to manage that.
>>
>> With that, add FOP_DONTCACHE to the btrfs file_operations fop_flags
>> structure, enabling use of RWF_DONTCACHE.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>
>> ---
>>
>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>> index bc2555c44a12..fd51dbc16176 100644
>> --- a/fs/btrfs/bio.c
>> +++ b/fs/btrfs/bio.c
>> @@ -337,7 +337,7 @@ static void btrfs_end_bio_work(struct work_struct *work)
>>  	struct btrfs_bio *bbio = container_of(work, struct btrfs_bio, end_io_work);
>>  
>>  	/* Metadata reads are checked and repaired by the submitter. */
>> -	if (is_data_bbio(bbio))
>> +	if (bio_op(&bbio->bio) == REQ_OP_READ && is_data_bbio(bbio))
>>  		btrfs_check_read_bio(bbio, bbio->bio.bi_private);
>>  	else
>>  		btrfs_bio_end_io(bbio, bbio->bio.bi_status);
>> @@ -354,7 +354,7 @@ static void btrfs_simple_end_io(struct bio *bio)
>>  	if (bio->bi_status)
>>  		btrfs_log_dev_io_error(bio, dev);
>>  
>> -	if (bio_op(bio) == REQ_OP_READ) {
>> +	if (bio_op(bio) == REQ_OP_READ || bbio->dropbehind_io) {
>>  		INIT_WORK(&bbio->end_io_work, btrfs_end_bio_work);
>>  		queue_work(btrfs_end_io_wq(fs_info, bio), &bbio->end_io_work);
>>  	} else {
>> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
>> index e2fe16074ad6..b41c5236c93d 100644
>> --- a/fs/btrfs/bio.h
>> +++ b/fs/btrfs/bio.h
>> @@ -82,6 +82,8 @@ struct btrfs_bio {
>>  	/* Save the first error status of split bio. */
>>  	blk_status_t status;
>>  
>> +	bool dropbehind_io;
> 
> Changelog says this should be 'uncached_io', which is more
> understandable than 'dropbehind_io' (as in the folio helper)
> As it's just a rename I will fix it but would like to know if this is ok
> for you.

Yep that's fine with me - and honestly I think it's a better name, but
some renaming happened with the external changes.

> Otherwise the patch looks ok, thanks.

Thanks!

-- 
Jens Axboe