Re: [kvm-unit-tests PATCH 1/6] s390x: Use interrupts in SCLP and add locking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03.09.19 09:53, Janosch Frank wrote:
> On 8/30/19 2:21 PM, David Hildenbrand wrote:
>> On 29.08.19 14:14, Janosch Frank wrote:
>>> We need to properly implement interrupt handling for SCLP, because on
>>> z/VM and LPAR SCLP calls are not synchronous!
>>>
>>> Also with smp CPUs have to compete for sclp. Let's add some locking,
>>> so they execute sclp calls in an orderly fashion and don't compete for
>>> the data buffer.
> 
> [...]
> 
>>> +
>>> +void sclp_mark_busy(void)
>>> +{
>>> +	/*
>>> +	 * With multiple CPUs we might need to wait for another CPU's
>>> +	 * request before grabbing the busy indication.
>>> +	 */
>>> +retry_wait:
>>> +	sclp_wait_busy();
>>> +	spin_lock(&sclp_lock);
>>> +	if (sclp_busy) {
>>> +		spin_unlock(&sclp_lock);
>>> +		goto retry_wait;
>>> +	}
>>> +	sclp_busy = true;
>>> +	spin_unlock(&sclp_lock);
>>
>> while (true) {
>> 	sclp_wait_busy();
>> 	spin_lock(&sclp_lock);
>> 	if (!sclp_busy) {
>> 		sclp_busy = true
>> 		spin_unlock(&sclp_lock);
>> 		break;
>> 	}
>> 	spin_unlock(&sclp_lock);
>> }
>>
>> Or can we simply switch to an atomic_t for sclp_busy and implement
>> cmpxchg using __sync_bool_compare_and_swap/ __sync_val_compare_and_swap ?
>>
>> I guess then we can drop the lock. But maybe I am missing something :)
> 
> If you want to add it and it works, I'm open for it :)
> Until then I'm taking your suggestion.
> 
>>
>>> +}
> [...]
>>>  extern char _sccb[];
>>> +void sclp_handle_ext(void);
>>> +void sclp_wait_busy(void);
>>> +void sclp_mark_busy(void);
>>
>> I wonder if we can find better names ...
>>
>> sclp_prepare()
>> sclp_finalize()
>>
>> or sth like that.
> 
> Hmm, IMHO my names are a bit better, since they clearly state, that we
> wait until the other user has finished.
> 
> 

I don't like "sclp_wait_busy()" as it is not clear from the name that we
are waiting until we can mark it busy :) It is rather a
"sclp_wait_not_busy()" + marking it busy.

I'm not able to come up with a better name right now :)

-- 

Thanks,

David / dhildenb



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux