Re: [PATCH v6 5/6] fuse: add zero-copy over io-uring

On 8/13/26 20:09, Joanne Koong wrote:
> On Thu, Aug 13, 2026 at 10:01 AM Bernd Schubert <[email protected]> wrote:
>>
>>
>>
>> On 8/13/26 01:19, Joanne Koong wrote:
>>> On Wed, Aug 12, 2026 at 3:30 PM Bernd Schubert <[email protected]> wrote:
>>>>
>>>>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>>>>> index d50792162d8b..3d1910a9db52 100644
>>>>> --- a/fs/fuse/dev.c
>>>>> +++ b/fs/fuse/dev.c
>>>>> @@ -1247,11 +1247,20 @@ int fuse_copy_folio(struct fuse_copy_state *cs, struct folio **foliop,
>>>>>
>>>>> @@ -746,6 +786,62 @@ static int fuse_uring_copy_from_ring(struct fuse_req *req,
>>>>>       return err;
>>>>>  }
>>>>>
>>>>> +static void fuse_zero_copy_release(void *priv)
>>>>> +{
>>>>> +     struct fuse_zero_copy_bvs *zc_bvs = priv;
>>>>> +     unsigned int i;
>>>>> +
>>>>> +     for (i = 0; i < zc_bvs->nr_bvs; i++)
>>>>> +             folio_put(page_folio(zc_bvs->bvs[i].bv_page));
>>>>> +
>>>>> +     kfree(zc_bvs);
>>>>
>>>> kvfree (see below)?
>>>>
>>>>> +}
>>>>> +
>>>>> +static int fuse_uring_set_up_zero_copy(struct fuse_ring_ent *ent,
>>>>> +                                    struct fuse_req *req,
>>>>> +                                    unsigned int issue_flags)
>>>>> +{
>>>>> +     struct fuse_args_pages *ap;
>>>>> +     int err, i, ddir = 0;
>>>>> +     struct fuse_zero_copy_bvs *zc_bvs;
>>>>> +     struct bio_vec *bvs;
>>>>> +
>>>>> +     /* out_pages indicates a read, in_pages indicates a write */
>>>>> +     if (req->args->out_pages)
>>>>> +             ddir |= IO_BUF_DEST;
>>>>> +     if (req->args->in_pages)
>>>>> +             ddir |= IO_BUF_SOURCE;
>>>>> +
>>>>> +     ap = container_of(req->args, typeof(*ap), args);
>>>>> +
>>>>> +     zc_bvs = kmalloc(struct_size(zc_bvs, bvs, ap->num_folios),
>>>>> +                      GFP_KERNEL_ACCOUNT);
>>>>
>>>> Assuming one folio per page and 1MB max_nr_pages:
>>>>
>>>> struct_size(zc_bvs, bvs, n) == 8 + 16 * 256 = 4104
>>>>
>>>> Shouldn't this be kvmalloc()?
>>>
>>> I'm not sure what the upper limit is for a kmalloc, but I'll change
>>> this to kvmalloc().
>>
>>
>> As far as I know kmalloc will allocate continuos pages and might fail
>> under fragmentation if more than a page is needed. I'm not entirely sure
>> if kvmalloc for such a small allocation will actually do the job.
>> How about the attached patch? And with that maybe we should limit
>> max_nr_pages to 256 for ZC?
> 
> I think kvmalloc should be fine. For size > PAGE_SIZE it tries kmalloc
> first and has a fallback to vmalloc. For small allocations, kvmalloc
> is just plain kmalloc.

I think the patch I had attached is more elegant, but we can do that any
time later. I just found kvmalloc_flex, I guess that would be most suitable.

Thanks,
Bernd