On 2016-01-02 21:04, Ahmed S. Darwish wrote:
Hi!
Hi! Long time no see :-)
On Mon, Sep 28, 2015 at 10:47:09AM +0200, David Henningsson wrote:
1)
There was some discussion about how the pa_mem struct should look like. I
think it should just look like this:
struct pa_mem {
void *ptr;
size_t size;
pa_mem_type_t type;
int fd;
unsigned id; /* both shm and memfd need an id, see below */
bool shm_do_unlink;
}
Memfd and shm can then share some methods because they're both fd backed (e
g, closing is to call munmap and then fclose for both).
Plus, it's all much simpler than anonymous unions and structs, IMO.
Indeed. After finishing doing so now in v2, a lot of code duplication
between posix SHM and memfds has been removed and the patch series is
now much smaller. Thanks a lot.
2)
The second big thing that makes me confused is how the memfd is initially
set up and sent over the pipe to the client. Am I reading your code wrong,
or do you actually send an extra memfd packet with the fd in *for every
packet* sent?
You're reading the code right. Definitely a problem to be fixed.
The per-client memfd should be set up by the server at SHM negotiation time
(and sealed, but you've said that's a future patch set). Then send a packet
with the memfd as ancil data (or extend PA_COMMAND_ENABLE_SRBCHANNEL, if
that's simpler). This packet should also have an ID that identifies the
memfd.
I'm having a problem in this part. By doing as said above, aren't we
limiting the pstream connection to send memblocks only from a _single_
memfd-backed pool?
Imagine the following:
// PA node 1
pa_pstream_send_memblock(p, memblock1); // memblock1 is inside pool1
pa_pstream_send_memblock(p, memblock2); // memblock2 is inside pool2
If pool1 and pool2 are backed by different memfd regions, how would the
above suggestion handle that case?
Hmm, to ask a counter question; why would you put them in different
pools in the first place? Why would you need more than one pool per pstream?
Every memfd and shm now have a unique ID, so you can just put that ID when
you do the memexport, so on the sender side you don't need to distinguish
between the two types.
And on the memimport side you'd look this ID up, and see if it matches a
previously received memfd, if not, it's a posix-shm ID that you need to call
pa_shm_attach on.
Does that make sense to you?
Ah, definitely agree. This is much simpler.
If my claim above is valid though, I might just implement in a slightly
different way:
``Instead of sending the memfd file descriptor at SHM negotiation
time, do it in pstream.c::prepare_next_write_item().
To avoid the problem of sending the memfd fd for every packet sent,
track the memfd-backed blocks earlier sent using their randomly
generated IDs.
If this is the first time we encounter this ID, send the memfd file
descriptor as ancil data with the packet. If this is not the first
encouter, just send the ID without any ancil data. The other end
is already tracking this ID and have a memfd socket opened for it
from earlier communication''
What do you think?
Thanks again, and happy holidays :)
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
