Re: xenstored memory leak

[Wei Liu <wei.liu2@citrix.com>]

On Wed, Jul 13, 2016 at 04:09:26PM +0200, Juergen Gross wrote:
> On 13/07/16 15:52, Wei Liu wrote:
> > On Wed, Jul 13, 2016 at 03:25:45PM +0200, Juergen Gross wrote:
> >> On 13/07/16 15:07, Wei Liu wrote:
> >>> On Wed, Jul 13, 2016 at 02:21:38PM +0200, Juergen Gross wrote:
> >>>> On 06/07/16 09:31, Juergen Gross wrote:
> >>>>> While testing some patches for support of ballooning in Mini-OS by using
> >>>>> the xenstore domain I realized that each xl create/destroy pair would
> >>>>> increase memory consumption in Mini-OS by about 5kB. Wondering whether
> >>>>> this is a xenstore domain only effect I did the same test with xenstored
> >>>>> and oxenstored daemons.
> >>>>>
> >>>>> xenstored showed the same behavior, the "referenced" size showed by the
> >>>>> pmap command grew by about 5kB for each create/destroy pair.
> >>>>>
> >>>>> oxenstored seemed to be even worse in the beginning (about 6kB for each
> >>>>> pair), but after about 100 create/destroys the value seemed to be
> >>>>> rather stable.
> >>>>>
> >>>>> Did anyone notice this memory leak before?
> >>>>
> >>>> I think I've found the problem:
> >>>>
> >>>> qemu as the device model is setting up a xenstore watch for each backend
> >>>> type it is supporting. Unfortunately those watches are never removed
> >>>> again. This sums up to the observed memory leak.
> >>>>
> >>>> I'm not sure how oxenstored is avoiding the problem, may be by testing
> >>>> socket connections to be still alive and so detecting qemu has gone.
> >>>> OTOH this won't help for oxenstored running in another domain than the
> >>>> device model (either due to oxenstore-stubdom, or a driver domain with
> >>>> a qemu based device model).
> >>>>
> >>>
> >>> How unfortunate.
> >>>
> >>> My gut feeling is that xenstored shouldn't have the knowledge to
> >>> associate a watch with a "process". The concept of a process is only
> >>> meaningful to OS, which wouldn't work on cross-domain xenstored setup.
> >>
> >> Right.
> >>
> >>> Maybe the OS xenbus driver should reap all watches on behalf the dead
> >>> process. This would also avoid a crashed QEMU leaking resources.
> >>>
> >>> And xenstored should have proper quota support so that a domain can't
> >>> set up excessive numbers of watches.
> >>
> >> This would be dom0 unless you arrange the device model to be accounted
> >> as the domid it is running for. But this is problematic with a xenstore
> >> domain again.
> >>
> > 
> > The quota could be based on "connection" (ring or socket) and
> > counted as per-connection? Just throwing ideas around, not necessarily
> > saying this is the way to go.
> 
> Sure. But with xenstore domain all xenstore access of dom0 is via one
> ring. And how would you want to apply any quota here solving our
> problem with one qemu process in dom0 creating stale watches? You

That's a job for the kernel driver.

The quota inside xenstored is to protect itself from abuse from one
single connection and punish the bad actors.

> could open a new connection for the device model, of course, but this
> would require some xenbus rework.
> 

I wouldn't go down that route unless absolutely necessary  because that
seems to require xenbus protocol extension.

Wei.

> 
> Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel