[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] rumpkernel dependencies

From: Samuel Thibault
Subject: Re: [PATCH] rumpkernel dependencies
Date: Fri, 3 Apr 2020 00:28:34 +0200
User-agent: NeoMutt/20170609 (1.8.3)

Damien Zammit, le lun. 30 mars 2020 22:22:12 +1100, a ecrit:
> I cleaned up rumpdisk and used your new api.  See patch below.

I'm amazed how very straighforward this implementation looks, rump rocks

Concerning the link line, I don't understand why hardcoding everything?

- For a start, are the _pic.a versions really needed? The .a versions
  should just work.

- Then, you don't need to define a link rule by hand, you can put the
  names of the hurd libraries to be linked in in HURDLIBS, and the -l
  flags for the system libraries to be linked in in LDLIBS. See for
  instance ext2fs/Makefile.

- For the rump+machdev piece with "whole-archive", you can stuff that in

That way, rumpdisk will benefit from all the common make bits.

About IOC macros in block-rump.c, is rump not providing a header that
defines them, instead of hardcoding them here?

Concerning translate_name, instead of malloc+snprintf, you can use
asprintf.  That being said, since it doesn't actually need to be kept
allocated, better just make this a function that takes the output
buffer, which you can allocate on the stack in device_open.

Take care of indentation, the GNU Coding Style is not

if (foo)


if (foo)

There are a couple mach_print debugging calls left.

Please add a /* FIXME: make multithreaded */ in device_write/read.  The
way it is currently shaped is completely synchronous: device_read will
be stuck inside rump_sys_read() until the data is retrieved from the
disk.  To fix this, we need to add a machdev_multithread_server function
in libmachdev, which calls ports_manage_port_operations_multithread
instead of ports_manage_port_operations_one_thread, and use that here
instead of machdev_server. That way there will be one thread per
device_read/write request, not blocking each other.  That will however
pose a problem with the lseek-then-read/write way currently used in
device_read/write: two concurrent device_read calls with call lseek
concurrently before calling read concurrently. You can already fix this
now (even before moving to ports_manage_port_operations_multithread) by
calling rump_sys_pread/pwrite instead.

That being said, on even longer run, we may probably not want
to keep one thread per device_read/write request. We'll want to
call rump_sys_aio_read/write instead and return MIG_NO_REPLY from
device_read/write, and send the mig reply once the aio request has
completed. That way, only the aio request will be kept in rumpdisk
memory instead of a whole thread structure. Again, please add this as a
FIXME note, but for much later. Our current linux glue disk driver does
not even do that currently :)

On rump_sys_read error, you need to unmap buf.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]