Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 29 Mar 2023 17:00:31 +0800
From:   Ming Lei <ming.lei@redhat.com>
To:     Ziyang Zhang <ZiyangZhang@linux.alibaba.com>
Cc:     linux-kernel@vger.kernel.org, Miklos Szeredi <mszeredi@redhat.com>,
        Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>,
        Bernd Schubert <bschubert@ddn.com>,
        Pavel Begunkov <asml.silence@gmail.com>,
        io-uring@vger.kernel.org, Stefan Hajnoczi <stefanha@redhat.com>,
        linux-block@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
        Dan Williams <dan.j.williams@intel.com>, ming.lei@redhat.com
Subject: Re: [PATCH V5 16/16] block: ublk_drv: apply io_uring FUSED_CMD for
 supporting zero copy
Message-ID: <ZCP+L0ADCxHo5vSg@ovpn-8-26.pek2.redhat.com>
References: <20230328150958.1253547-1-ming.lei@redhat.com>
 <20230328150958.1253547-17-ming.lei@redhat.com>
 <2e3c20e0-a0be-eaf3-b288-c3c8fa31d1fa@linux.alibaba.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2e3c20e0-a0be-eaf3-b288-c3c8fa31d1fa@linux.alibaba.com>
Precedence: bulk

On Wed, Mar 29, 2023 at 10:57:53AM +0800, Ziyang Zhang wrote:
> On 2023/3/28 23:09, Ming Lei wrote:
> > Apply io_uring fused command for supporting zero copy:
> > 
> 
> [...]
> 
> >  
> > @@ -1374,7 +1533,12 @@ static int ublk_ch_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
> >  	if (!ubq || ub_cmd->q_id != ubq->q_id)
> >  		goto out;
> >  
> > -	if (ubq->ubq_daemon && ubq->ubq_daemon != current)
> > +	/*
> > +	 * The fused command reads the io buffer data structure only, so it
> > +	 * is fine to be issued from other context.
> > +	 */
> > +	if ((ubq->ubq_daemon && ubq->ubq_daemon != current) &&
> > +			(cmd_op != UBLK_IO_FUSED_SUBMIT_IO))
> >  		goto out;
> >  
> 
> Hi Ming,
> 
> What is your use case that fused io_uring cmd is issued from another thread?
> I think it is good practice to operate one io_uring instance in one thread
> only.

So far we limit io command has to be issued from the queue context,
which is still not friendly from userspace viewpoint, the reason is
that we can't get io_uring exit notification and ublk's use case is
very special since the queued io command may not be completed forever,
see:

https://lore.kernel.org/linux-fsdevel/ZBxTdCj60+s1aZqA@ovpn-8-16.pek2.redhat.com/

I remember that people raised concern about this implementation.

But for normal IO, it could be issued from io wq simply because of
link(dependency) or whatever, and userspace is still allowed to submit
io from another pthread via same io_uring ctx.


thanks,
Ming