Re: [PATCH] xen/balloon: use a kernel thread instead a workqueue

[Juergen Gross <jgross@xxxxxxxx>]

On 08.09.21 16:47, Boris Ostrovsky wrote:
> On 8/27/21 8:32 AM, Juergen Gross wrote:
> > +static bool balloon_thread_cond(enum bp_state state, long credit)
> > +{
> > +       if (state != BP_EAGAIN)
> > +               credit = 0;
> > +
> > +       return current_credit() != credit || kthread_should_stop();
> > +}
> > +
> > +/*
> > + * As this is a kthread it is guaranteed to run as a single instance only.
> >    * We may of course race updates of the target counts (which are protected
> >    * by the balloon lock), or with changes to the Xen hard limit, but we will
> >    * recover from these in time.
> >    */
> > -static void balloon_process(struct work_struct *work)
> > +static int balloon_thread(void *unused)
> >   {
> >         enum bp_state state = BP_DONE;
> >         long credit;
> > +       unsigned long timeout;
> > +
> > +       set_freezable();
> > +       for (;;) {
> > +               if (state == BP_EAGAIN)
> > +                       timeout = balloon_stats.schedule_delay * HZ;
> > +               else
> > +                       timeout = 3600 * HZ;
> > +               credit = current_credit();
> >   
> > +		wait_event_interruptible_timeout(balloon_thread_wq,
> > +                                balloon_thread_cond(state, credit), timeout);
>
> Given that wait_event_interruptible_timeout() is a bunch of nested macros do we 
> need to worry here about overly aggressive compiler optimizing out 'credit = 
> current_credit()'?
I don't think so. current_credit() is reading from balloon_stats, which
is a global variable. So the compiler shouldn't assume the contents
won't change.

But I can add a barrier() after 'credit = current_credit()' in case
you'd feel uneasy without it.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature