public inbox for [email protected]
 help / color / mirror / Atom feed
From: Bernd Schubert <[email protected]>
To: Jens Axboe <[email protected]>, Bernd Schubert <[email protected]>,
	Miklos Szeredi <[email protected]>,
	Pavel Begunkov <[email protected]>,
	[email protected]
Cc: [email protected], [email protected],
	Joanne Koong <[email protected]>,
	Josef Bacik <[email protected]>,
	Amir Goldstein <[email protected]>
Subject: Re: [PATCH RFC v3 17/17] fuse: {uring} Pin the user buffer
Date: Wed, 4 Sep 2024 18:08:06 +0200	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

Hi Jens,

thanks for your help.

On 9/4/24 17:47, Jens Axboe wrote:
> On 9/1/24 7:37 AM, Bernd Schubert wrote:
>> This is to allow copying into the buffer from the application
>> without the need to copy in ring context (and with that,
>> the need that the ring task is active in kernel space).
>>
>> Also absolutely needed for now to avoid this teardown issue
> 
> I'm fine using these helpers, but they are absolutely not needed to
> avoid that teardown issue - well they may help because it's already
> mapped, but it's really the fault of your handler from attempting to map
> in user pages from when it's teardown/fallback task_work. If invoked and
> the ring is dying or not in the right task (as per the patch from
> Pavel), then just cleanup and return -ECANCELED.

As I had posted on Friday/Saturday, it didn't work. I had added a 
debug pr_info into Pavels patch, somehow it didn't trigger on PF_EXITING 
and I didn't further debug it yet as I was working on the pin anyway.
And since Monday occupied with other work...

For this series it is needed to avoid kernel crashes. If we can can fix 
patch 15 and 16, the better. Although we will still later on need it as
optimization.



> 
>> +/*
>> + * Copy from memmap.c, should be exported
>> + */
>> +static void io_pages_free(struct page ***pages, int npages)
>> +{
>> +	struct page **page_array = *pages;
>> +
>> +	if (!page_array)
>> +		return;
>> +
>> +	unpin_user_pages(page_array, npages);
>> +	kvfree(page_array);
>> +	*pages = NULL;
>> +}
> 
> I noticed this and the mapping helper being copied before seeing the
> comments - just export them from memmap.c and use those rather than
> copying in the code. Add that as a prep patch.

No issue to do that either. The hard part is then to get it through
different branches. I had removed the big optimization of 
__wake_up_on_current_cpu in this series, because it needs another
export.


> 
>> @@ -417,6 +437,7 @@ static int fuse_uring_out_header_has_err(struct fuse_out_header *oh,
>>  		goto seterr;
>>  	}
>>  
>> +	/* FIXME copied from dev.c, check what 512 means  */
>>  	if (oh->error <= -512 || oh->error > 0) {
>>  		err = -EINVAL;
>>  		goto seterr;
> 
> -512 is -ERESTARTSYS
> 

Ah thank you! I'm going to add separate patch for dev.c, as I wrote, this was
just a copy-and-paste.

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 592d0d96a106..779b23fa01c2 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2028,7 +2028,7 @@ static ssize_t fuse_dev_do_write(struct fuse_dev *fud,
        }
 
        err = -EINVAL;
-       if (oh.error <= -512 || oh.error > 0)
+       if (oh.error <= -ERESTARTSYS || oh.error > 0)
                goto copy_finish;
 
        spin_lock(&fpq->lock);


Thanks,
Bernd

  reply	other threads:[~2024-09-04 16:08 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-01 13:36 [PATCH RFC v3 00/17] fuse: fuse-over-io-uring Bernd Schubert
2024-09-01 13:36 ` [PATCH RFC v3 01/17] fuse: rename to fuse_dev_end_requests and make non-static Bernd Schubert
2024-09-01 13:36 ` [PATCH RFC v3 02/17] fuse: Move fuse_get_dev to header file Bernd Schubert
2024-09-01 13:36 ` [PATCH RFC v3 03/17] fuse: Move request bits Bernd Schubert
2024-09-01 13:36 ` [PATCH RFC v3 04/17] fuse: Add fuse-io-uring design documentation Bernd Schubert
2024-09-01 13:36 ` [PATCH RFC v3 05/17] fuse: Add a uring config ioctl Bernd Schubert
2024-09-04  0:43   ` Joanne Koong
2024-09-04 22:24     ` Bernd Schubert
2024-09-06 19:23       ` Joanne Koong
2024-09-01 13:37 ` [PATCH RFC v3 06/17] fuse: Add the queue configuration ioctl Bernd Schubert
2024-09-04 22:23   ` Joanne Koong
2024-09-04 22:38     ` Bernd Schubert
2024-09-04 22:42       ` Joanne Koong
2024-09-01 13:37 ` [PATCH RFC v3 07/17] fuse: {uring} Add a dev_release exception for fuse-over-io-uring Bernd Schubert
2024-09-01 13:37 ` [PATCH RFC v3 08/17] fuse: {uring} Handle SQEs - register commands Bernd Schubert
2024-09-04 15:40   ` Jens Axboe
2024-09-01 13:37 ` [PATCH RFC v3 09/17] fuse: Make fuse_copy non static Bernd Schubert
2024-09-01 13:37 ` [PATCH RFC v3 10/17] fuse: Add buffer offset for uring into fuse_copy_state Bernd Schubert
2024-09-01 13:37 ` [PATCH RFC v3 11/17] fuse: {uring} Add uring sqe commit and fetch support Bernd Schubert
2024-09-01 13:37 ` [PATCH RFC v3 12/17] fuse: {uring} Handle teardown of ring entries Bernd Schubert
2024-09-01 13:37 ` [PATCH RFC v3 13/17] fuse: {uring} Add a ring queue and send method Bernd Schubert
2024-09-01 13:37 ` [PATCH RFC v3 14/17] fuse: {uring} Allow to queue to the ring Bernd Schubert
2024-09-01 13:37 ` [PATCH RFC v3 15/17] ate: 2024-08-30 15:43:32 +0100 Bernd Schubert
2024-09-04 15:43   ` Jens Axboe
2024-09-04 15:54     ` Bernd Schubert
2024-09-01 13:37 ` [PATCH RFC v3 16/17] fuse: {uring} Handle IO_URING_F_TASK_DEAD Bernd Schubert
2024-09-01 13:37 ` [PATCH RFC v3 17/17] fuse: {uring} Pin the user buffer Bernd Schubert
2024-09-04 15:47   ` Jens Axboe
2024-09-04 16:08     ` Bernd Schubert [this message]
2024-09-04 16:16       ` Jens Axboe
2024-09-04 19:25         ` Bernd Schubert
2024-09-04 19:40           ` Jens Axboe
2024-09-05 21:04             ` Bernd Schubert
2024-09-04 18:59   ` Jens Axboe
2024-09-04 16:42 ` [PATCH RFC v3 00/17] fuse: fuse-over-io-uring Jens Axboe
2024-09-04 19:37   ` Bernd Schubert
2024-09-04 19:41     ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox