On 9/4/24 1:25 PM, Bernd Schubert wrote: > > > On 9/4/24 18:16, Jens Axboe wrote: >> On 9/4/24 10:08 AM, Bernd Schubert wrote: >>> 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... >> >> Then there's something wrong with that patch, as it definitely should >> work. How did you reproduce the teardown crash? I'll take a look here. > > Thank you! In this specific case > > 1) Run passthrough_hp with --debug-fuse > > 2) dd if=/dev/zero of=/scratch/test/testfile bs=1M count=1 > > Then on the console that has passthrough_hp output and runs slow with my > ASAN/etc kernel: ctrl-z and kill -9 % > I guess a pkill -9 passthrough_hp should also work Eerily similar to what I tried, but I managed to get it to trigger. Should work what's in there, but I think checking for task != current is better and not race prone like PF_EXITING is. So maybe? Try with the below incremental. diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c index 55bdcb4b63b3..fa5a0f724a84 100644 --- a/io_uring/uring_cmd.c +++ b/io_uring/uring_cmd.c @@ -121,7 +121,8 @@ static void io_uring_cmd_work(struct io_kiocb *req, struct io_tw_state *ts) struct io_uring_cmd *ioucmd = io_kiocb_to_cmd(req, struct io_uring_cmd); unsigned flags = IO_URING_F_COMPLETE_DEFER; - if (req->task->flags & PF_EXITING) + /* Different task should only happen if the original is going away */ + if (req->task != current) flags |= IO_URING_F_TASK_DEAD; /* task_work executor checks the deffered list completion */ -- Jens Axboe
