[pulseaudio-discuss] [PATCH v3 00/11] Introduce memfd support

[Ahmed S. Darwish]

Hi :-)

[ Adding Sjoerd Simons ]

[ Sorry for the late replies ]

On Thu, Mar 31, 2016 at 10:57:27AM +0200, David Henningsson wrote:
> 
> On 2016-03-22 01:56, Ahmed S. Darwish wrote:
> > On Mon, Mar 21, 2016 at 10:01:14AM +0100, David Henningsson wrote:
> > >
> > > On 2016-03-12 23:35, Ahmed S. Darwish wrote:
> > > > Hello!
> > > >
> > > > The simplified memfd patch series ;-)
> > >
> > > Hi!
> > >
> > > Good work!
> > >
> > > I've read through the patch series and think they look pretty
> > > much okay, and your amount of testing is exemplary.
> > >
> >
> > Thanks a lot .. couldn't do it without everyone's help here :-)
> >
> > Also, regarding the testing, I really did not want to increase
> > anyone's Launchpad bugfixing workload ;-)
> >
> > > Surely there are nitpicks here and there but at this point I
> > > think maybe we're better off merging the patch series as it
> > > is to get wider testing from developers using it in practice.
> > >
> >
> > Great, I'll be around if there's anything in need of a fix.
> >
> > > My only concern or thought is about the global mempool. By
> > > turning that into a memfd by default, we would potentially
> > > slow down clients which support posix shm but not memfd. I'm
> > > not sure if this is a problem in practice though - even an
> > > old closed-source client would (hopefully?) bind to a new
> > > libpulse library and thus gain memfd support that way, and I
> > > hope nobody links libpulse statically. Thoughts?
> >
> > Alexander mentioned the SteamOS case, where "they don't link
> > statically, but have a 'known-good' copy of the library
> > packaged and put into LD_LIBRARY_PATH, and it may be next to
> > impossible to explain to the users how to upgrade it."
> >
> > I'm sure though that even if they have a 'known-good' copy,
> > they keep the daemon version in-sync with the library version?
> > I can't think of any sane reason not to do so..
> >
> > Anyway, if that case is indeed problematic, maybe we can lobby
> > them a bit to do a proper distribution of PA? If anyone has
> > some contacts, I'll be glad to open a communication channel..
> 
> Aaaand...there we stalled. I could use a second opinion on:
> 
>  1) Whether to go ahead and merge, despite not promising to be
>  around much to test it
> 
>  2) If that merge should include all 11 patches or not.
> 
> Given the fact that the possible performance regression only
> affects a) input data and b) only old copies of libpulse, I
> think the chances are fairly small.
> 
> One option could be to merge all patches as-is but on top of
> that set "enable-memfd" to default to false for one release cycle;
> to give Steam and others a chance to update. That will also give
> pioneers some extra time for testing, and report bugs if there
> are any.
>

There's a nice path that can satisfy all use-cases, including the
ancient libpulse one..

Patch #1 to #10 won't hurt anyone's performance, including ancient
clients, so let's merge those now.

Patch #11 transforms the srbchannel mempool and the global mempool
to memfds. I'll submit a modified version where:

- The srbchannel mempool will be kept to use memfds by default.
  Patch #3 already transformed that pool to the per-client model,
  so if the client declares support only for posix SHM but not
  memfds (e.g. libpulse < 9.0), posix SHM will be used nicely. No
  performance hit here too.

- The global mempool, since it's global by nature and cannot cater
  to each client's needs (thus the reverse to always-supported
  plain old data copy if memfds are non-existent on the client),
  will be kept to use posix SHM by default.

The next patch series, which will then transforms the global
mempool to a per-client model, will be able to personalize that
mempool's SHM type according to different clients needs. Thus
memfds can be used for it while not hurting older clients.

Sounds good?

regards,

-- 
Darwish
http://darwish.chasingpointers.com