[pulseaudio-discuss] [PATCH v2 06/12] pulsecore: memexport/memimport: Support memfd blocks

[David Henningsson]

On 2016-02-19 20:28, Ahmed S. Darwish wrote:
> On Fri, Feb 19, 2016 at 03:54:06PM +0100, David Henningsson wrote:
>>
>> On 2016-02-16 22:41, Ahmed S. Darwish wrote:
>>> Hi :-)
>>>
> ...
>>> 4. The code above is actually for the PA endpoint creating a
>>>     memfd mempool and registering it with the other side. That
>>>     is, pa_mempool_take_memfd_fd() is just used when registering
>>>     a memfd mempool over the pstream.
>>
>> Ah, so that's only used on the sending side. Maybe I was just
>> confused when I wrote the above.
>>
>> But then I have a follow-up question. In shm_attach, which is on the
>> receiving side, why do we need to save the fd in pa_shm at all? I
>> mean, we don't need to send it to anyone else (right?), so all we
>> need is the memory and mmap keeps that open for us. So, we could
>> just close the memfd in shm_attach?
>
> Yup, this is what exactly happens. No fd is ever cached on the
> receiving side at shm_attch(). Similar point was raised in patch #5
> review and a more detailed reply was posted here [*] -- in the 2nd
> reply hunk.
>
> Given that this issue is now raised twice, I'm sure the code is
> badly unclear, possibly due to the per-type stuff now removed.
>
> Anyway here's how shm_attch() now looks like in v3:
>
>    static int shm_attach(pa_shm *m, pa_mem_type_t type, unsigned
>                          id, int memfd_fd, ..) {
>        ..
>        /* In case of attaching to memfd areas, _the caller_
>         * maintains ownership of the passed fd and has the sole
>         * responsibility of closing it down.. For other types, we
>         * are the code path  which created the fd in the first
>         * place and we're thus the ones responsible for closing it
>         * down */
>        if (type != PA_MEM_TYPE_SHARED_MEMFD)
>            pa_assert_se(pa_close(fd) == 0);
>
>        m->type = type;
>        m->id = id;
>        m->size = (size_t) st.st_size;
>        m->do_unlink = false;
>        m->fd = -1;
>    }
>
> I guess this is now much more clear :-)

Okay, so it's mostly a design decision then? Whether to take ownership 
of the memfd and thus close it down when we're done (and simpler code as 
a result?), or let the caller do the same right after the call to 
shm_attach?


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic