On 22/03/2020 22:51, Jens Axboe wrote:
> commit f1d96a8fcbbbb22d4fbc1d69eaaa678bbb0ff6e2
> Author: Pavel Begunkov <asml.silence@gmail.com>
> Date:   Fri Mar 13 22:29:14 2020 +0300
> 
>     io_uring: NULL-deref for IOSQE_{ASYNC,DRAIN}
> 
> which is what I ran into as well last week...

I picked it before testing

> The extra memory isn't a bit deal, it's very minor. My main concern
> would be fairness, since we'd then be grabbing non-contig hashed chunks,
> before we did not. May not be a concern as long as we ensure the
> non-hasned (and differently hashed) work can proceed in parallel. For my
> end, I deliberately added:

Don't think it's really a problem, all ordering/scheduling is up to users (i.e.
io_uring), and it can't infinitely postpone a work, because it's processing
spliced requests without taking more, even if new ones hash to the same bit.

> +	/* already have hashed work, let new worker get this */
> +	if (ret) {
> +		struct io_wqe_acct *acct;
> +
> +		/* get new worker for unhashed, if none now */
> +		acct = io_work_get_acct(wqe, work);
> +		if (!atomic_read(&acct->nr_running))
> +			io_wqe_wake_worker(wqe, acct);
> +		break;
> +	}
> 
> to try and improve that.

Is there performance problems with your patch without this chunk? I may see
another problem with yours, I need to think it through.

> 
> I'll run a quick test with yours.

-- 
Pavel Begunkov