[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] rumpkernel dependencies
From: |
Damien Zammit |
Subject: |
Re: [PATCH] rumpkernel dependencies |
Date: |
Fri, 3 Apr 2020 12:23:56 +1100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 |
On 3/4/20 9:28 am, Samuel Thibault wrote:
> 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.
I think the _pic.a versions are required. I could not get rump to work with .a
That is why I made rumpkernel package install them.
> About IOC macros in block-rump.c, is rump not providing a header that
> defines them, instead of hardcoding them here?
I can put them in my own header, I could not find them exposed in rump.
> 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.
OK I will fix this.
> Take care of indentation, the GNU Coding Style is...
OK I will fix this.
> There are a couple mach_print debugging calls left.
I think they should remain, they only occur in severe error.
> 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.
ACK
> 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 :)
ACK
> On rump_sys_read error, you need to unmap buf.
ACK
Damien
- Re: [PATCH] rumpkernel dependencies, Damien Zammit, 2020/04/01
- Re: [PATCH] rumpkernel dependencies, Samuel Thibault, 2020/04/02
- Re: [PATCH] rumpkernel dependencies,
Damien Zammit <=
- Re: [PATCH] rumpkernel dependencies, Samuel Thibault, 2020/04/03
- Re: [PATCH] rumpkernel dependencies, Damien Zammit, 2020/04/03
- Re: [PATCH] rumpkernel dependencies, Samuel Thibault, 2020/04/03
- Re: [PATCH] rumpkernel dependencies, Damien Zammit, 2020/04/10
- Re: [PATCH] rumpkernel dependencies, Samuel Thibault, 2020/04/11