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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9d1edc4e-b2b5-c620-17c6-31d238c2c417@suse.com>
Date:   Wed, 8 Sep 2021 17:11:36 +0200
From:   Juergen Gross <jgross@...e.com>
To:     Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org
Cc:     Stefano Stabellini <sstabellini@...nel.org>,
        Jan Beulich <jbeulich@...e.com>
Subject: Re: [PATCH] xen/balloon: use a kernel thread instead a workqueue

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

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3092 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)