On 25/10/2019 19:57, Jens Axboe wrote: > On 10/25/19 10:55 AM, Pavel Begunkov wrote: >> On 25/10/2019 19:44, Jens Axboe wrote: >>> On 10/25/19 10:40 AM, Pavel Begunkov wrote: >>>> On 25/10/2019 19:32, Jens Axboe wrote: >>>>> On 10/25/19 10:27 AM, Jens Axboe wrote: >>>>>> On 10/25/19 10:21 AM, Pavel Begunkov wrote: >>>>>>> On 25/10/2019 19:03, Jens Axboe wrote: >>>>>>>> On 10/25/19 3:55 AM, Pavel Begunkov wrote: >>>>>>>>> I found 2 problems with __io_sequence_defer(). >>>>>>>>> >>>>>>>>> 1. it uses @sq_dropped, but doesn't consider @cq_overflow >>>>>>>>> 2. @sq_dropped and @cq_overflow are write-shared with userspace, so >>>>>>>>> it can be maliciously changed. >>>>>>>>> >>>>>>>>> see sent liburing test (test/defer *_hung()), which left an unkillable >>>>>>>>> process for me >>>>>>>> >>>>>>>> OK, how about the below. I'll split this in two, as it's really two >>>>>>>> separate fixes. >>>>>>> cached_sq_dropped is good, but I was concerned about cached_cq_overflow. >>>>>>> io_cqring_fill_event() can be called in async, so shouldn't we do some >>>>>>> synchronisation then? >>>>>> >>>>>> We should probably make it an atomic just to be on the safe side, I'll >>>>>> update the series. >>>>> >>>>> Here we go, patch 1: >>>>> >>>>> http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus&id=f2a241f596ed9e12b7c8f960e79ccda8053ea294 >>>>> >>>>> patch 2: >>>>> >>>>> http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus&id=b7d0297d2df5bfa0d1ecf9d6c66d23676751ef6a >>>>> >>>> 1. submit rqs (not yet completed) >>>> 2. poll_list is empty, inflight = 0 >>>> 3. async completed and placed into poll_list >>>> >>>> So, poll_list is not empty, but we won't get to polling again. >>>> At least until someone submitted something. >>> >>> But if they are issued, the will sit in ->poll_list as well. That list >>> holds both "submitted, but pending" and completed entries. >>> >> Missed it, then should work. Thanks! > > Glad we agree :-) > >>> + ret = iters = 0; >> A small suggestion, could we just initialise it in declaration >> to be a bit more concise? >> e.g. int ret = 0, iters = 0; >> >> Reviewed-by: Pavel Begunkov <asml.silence@xxxxxxxxx> >> And let me test it as both patches are ready. > > Sure, I'll make that change and add your reviewed-by. Thanks! > Stress tested, works well! Tested-by: Pavel Begunkov <asml.silence@xxxxxxxxx> -- Yours sincerely, Pavel Begunkov
Attachment:
signature.asc
Description: OpenPGP digital signature
