Am 21.02.2015 um 02:51 schrieb Linus Torvalds:
> So here's my try at fixing READ_ONCE() so that it is happy with 'const' sources.
>
> It is entirely untested. Comments/testing?
>
> Christian, I guess I could just have forced a cast instead of the
> union. I'd like you to take a look at this, because right now it's
> holding up me pulling from Ingo.
Sorry for the too late for rc1 answer, but I was traveling the last 4
days.
Hmm, some autocasting feels better, but I could not come up with a proper
solution that works for all cases (e.g. I tried "__auto_type __val = x"
or "typeof(x * 0)" to make this lvalue and rvalue, but all variants failed
in one or the other way).
Unless I can come up with a better solution your union patch is probably
the best way to go and rc1 seems to work.
>
> And Ingo, I think you need to add some kind of test for "horrible new
> warnings". I think your pull request *worked*, but the tens of lines
> of new warnings it generates is unacceptable, and will just cause me
> to undo the pull if I notice in time (like I did this time).
I was getting several complaints from the linux-next buildbots about new
sparse warnings, compile warning and so on when doing this rework, e.g.
commit c5b19946eb76c675 ("kernel: Fix sparse warning for ACCESS_ONCE")
fixes two of those warnings.
So I am somewhat surprised that I never saw this as I am also following the
KVM list. turns out that arch/x86/kernel/kvm.c does not CC the kvm list
in get_maintainers.pl.
Maybe I should push something like that to Paolo/Marcelo.
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5574,6 +5574,7 @@ S: Supported
F: Documentation/*/kvm*.txt
F: Documentation/virtual/kvm/
F: arch/*/kvm/
+F: arch/x86/kernel/kvm.c
F: arch/*/include/asm/kvm*
F: include/linux/kvm*
F: include/uapi/linux/kvm*
Christian
>
> Linus
>
> On Fri, Feb 20, 2015 at 4:03 PM, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>> How does this work for you at all?
>>
>> On Fri, Feb 20, 2015 at 5:37 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>>> index 94f643484300..e354cc6446ab 100644
>>> --- a/arch/x86/kernel/kvm.c
>>> +++ b/arch/x86/kernel/kvm.c
>>> @@ -803,8 +808,8 @@ static void kvm_unlock_kick(struct arch_spinlock *lock, __ticket_t ticket)
>>> add_stats(RELEASED_SLOW, 1);
>>> for_each_cpu(cpu, &waiting_cpus) {
>>> const struct kvm_lock_waiting *w = &per_cpu(klock_waiting, cpu);
>>> - if (ACCESS_ONCE(w->lock) == lock &&
>>> - ACCESS_ONCE(w->want) == ticket) {
>>> + if (READ_ONCE(w->lock) == lock &&
>>> + READ_ONCE(w->want) == ticket) {
>>> add_stats(RELEASED_SLOW_KICKED, 1);
>>> kvm_kick_cpu(cpu);
>>> break;
>>
>> I get horrible compile warnings from this, because of how 'w' is a
>> pointer to a 'const' structure, which then causes things like
>>
>> include/linux/compiler.h:262:39: warning: passing argument 1 of
>> ‘__read_once_size’ discards ‘const’ qualifier from pointer target type
>> ({ typeof(x) __val; __read_once_size(&x, &__val, sizeof(__val)); __val; })
>>
>> which is fairly hard to avoid (looks like it might need a union)
>>
>> Linus
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
