| Message ID | 20220621224839.76007-1-krisman@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [ubdsrv] tgt_null: Return number of sectors read/written | expand |
On Tue, Jun 21, 2022 at 06:48:39PM -0400, Gabriel Krisman Bertazi wrote: > Hi Ming, > > I wrote this against your devel-v3 branch. I'm wondering if you plan to > send a new version of the kernel patch soon? From the latest Yeah, that is on my todo list: https://github.com/ming1/linux/commits/my_for-5.19-ubd-devel_v3 there has lots cleanup & improvement. > discussions, I don't think there were major issues found on review. :) One problem is the driver name, and Christoph thought we have 'arch/um/drivers/ubd*.c'. Not thought of one good candidate yet. > > I hope people don't mind I cc'd linux-block about this userspace code. > Please, let me know if I shouldn't do that. > > -- >8 -- > > The number of sectors read/written is used to verify forward progress of > the request inside the kernel. If we return 0 here, the kernel > understands that as an IO failure (see first check in ubd_complete_rq), > and will reissue the request, causing an infinite loop of unfullfilled > requests. This can be reproduced with: > > ubdsrv/ubd add -t null -n0 -q1 -d1 > dd if=/dev/vda of=/dev/ubdb0 count=1 bs=4k > > The approach minics nullblk, which returns the total IO size. > > Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com> > --- > tgt_null.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tgt_null.c b/tgt_null.c > index 85636c405f0c..61850a2cd046 100644 > --- a/tgt_null.c > +++ b/tgt_null.c > @@ -20,7 +20,9 @@ static int null_init_tgt(struct ubdsrv_tgt_info *tgt, int type, int argc, > static int null_handle_io_async(struct ubdsrv_queue *q, struct ubd_io *io, > int tag) > { > - ubdsrv_mark_io_done(io, 0); > + const struct ubdsrv_io_desc *iod = ubdsrv_get_iod(q, tag); > + > + ubdsrv_mark_io_done(io, iod->nr_sectors << 9); This issue was actually fixed in master branch, and there are actually lots of change in master: - switch to liburing - switch to c++, and use c++20 coroutine for ->handle_io_async() - all kinds of cleanup I am actually working on ubd-qcow2 with above, but that may take a while and the code isn't posted yet. The motivation is that the whole framework can get verified well enough with one complicated target implementation. Thanks, Ming
Ming Lei <ming.lei@redhat.com> writes: > On Tue, Jun 21, 2022 at 06:48:39PM -0400, Gabriel Krisman Bertazi wrote: >> Hi Ming, >> >> I wrote this against your devel-v3 branch. I'm wondering if you plan to >> send a new version of the kernel patch soon? From the latest > > Yeah, that is on my todo list: > > https://github.com/ming1/linux/commits/my_for-5.19-ubd-devel_v3 > > there has lots cleanup & improvement. > >> discussions, I don't think there were major issues found on review. :) > > One problem is the driver name, and Christoph thought we have > 'arch/um/drivers/ubd*.c'. Not thought of one good candidate yet. Hi Ming, Thanks for the info, and sorry for not noticing the fix merged on Jun, 3rd on the master branch. I will follow that branch when testing and submit fixes I find along the way. I guess you have considered a lot of names, but I'd suggest any of: * blkuser, * ublk * BUSE (as in Block FUSE, though there is another non-upstream project with that name), * UBIO (as in UIO, but for Block IO) * B2U (Block IO Backed by userspace) :-P TBH, my favorite is ublk. Thank you,
On Wed, Jun 22, 2022 at 12:23:42PM -0400, Gabriel Krisman Bertazi wrote: > Ming Lei <ming.lei@redhat.com> writes: > > > On Tue, Jun 21, 2022 at 06:48:39PM -0400, Gabriel Krisman Bertazi wrote: > >> Hi Ming, > >> > >> I wrote this against your devel-v3 branch. I'm wondering if you plan to > >> send a new version of the kernel patch soon? From the latest > > > > Yeah, that is on my todo list: > > > > https://github.com/ming1/linux/commits/my_for-5.19-ubd-devel_v3 > > > > there has lots cleanup & improvement. > > > >> discussions, I don't think there were major issues found on review. :) > > > > One problem is the driver name, and Christoph thought we have > > 'arch/um/drivers/ubd*.c'. Not thought of one good candidate yet. > > Hi Ming, > > Thanks for the info, and sorry for not noticing the fix merged on Jun, 3rd > on the master branch. I will follow that branch when testing and submit > fixes I find along the way. That is great! > > I guess you have considered a lot of names, but I'd suggest any of: > > * blkuser, > * ublk > * BUSE (as in Block FUSE, though there is another non-upstream > project with that name), > * UBIO (as in UIO, but for Block IO) > * B2U (Block IO Backed by userspace) :-P > > TBH, my favorite is ublk. Me too, will change to it in v3 if no one objects. Thanks, Ming
diff --git a/tgt_null.c b/tgt_null.c index 85636c405f0c..61850a2cd046 100644 --- a/tgt_null.c +++ b/tgt_null.c @@ -20,7 +20,9 @@ static int null_init_tgt(struct ubdsrv_tgt_info *tgt, int type, int argc, static int null_handle_io_async(struct ubdsrv_queue *q, struct ubd_io *io, int tag) { - ubdsrv_mark_io_done(io, 0); + const struct ubdsrv_io_desc *iod = ubdsrv_get_iod(q, tag); + + ubdsrv_mark_io_done(io, iod->nr_sectors << 9); return 0; }
Hi Ming, I wrote this against your devel-v3 branch. I'm wondering if you plan to send a new version of the kernel patch soon? From the latest discussions, I don't think there were major issues found on review. :) I hope people don't mind I cc'd linux-block about this userspace code. Please, let me know if I shouldn't do that. -- >8 -- The number of sectors read/written is used to verify forward progress of the request inside the kernel. If we return 0 here, the kernel understands that as an IO failure (see first check in ubd_complete_rq), and will reissue the request, causing an infinite loop of unfullfilled requests. This can be reproduced with: ubdsrv/ubd add -t null -n0 -q1 -d1 dd if=/dev/vda of=/dev/ubdb0 count=1 bs=4k The approach minics nullblk, which returns the total IO size. Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com> --- tgt_null.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)