On Sun, Aug 26, 2012 at 10:42:44AM +0300, Michael S. Tsirkin wrote: > > Reading two atomics and doing math? Result can even be negative. > I did not look at use closely but it looks suspicious. Doc on atomic_read says: " The read is atomic in that the return value is guaranteed to be one of the values initialized or modified with the interface operations if a proper implicit or explicit memory barrier is used after possible runtime initialization by any other thread and the value is modified only with the interface operations. "
There's no runtime init by other thread than balloon's itself at device register, and the operations (inc, dec) are made by the proper interface operations only when protected by the spinlock pages_lock. It does not look suspicious, IMHO. I'm failing to see how it could become a negative on that case, since you cannot isolate more pages than what was previoulsy inflated to balloon's list. > It's already the case everywhere except __wait_on_isolated_pages, > so just fix that, and then we can keep using int instead of atomics. > Sorry, I quite didn't get you here. fix what? > That's 1K on stack - and can become more if we increase > VIRTIO_BALLOON_ARRAY_PFNS_MAX. Probably too much - this is the reason > we use vb->pfns. > If we want to use vb->pfns we'll have to make leak_balloon mutual exclusive with page migration (as it was before), but that will inevictably bring us back to the discussion on breaking the loop when isolated pages make leak_balloon find less pages than it wants to release at each leak round. _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
