Re: [kvm-unit-tests PATCH v4 07/13] s390x: Use interrupts in SCLP

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

 



On 03.01.19 14:37, Thomas Huth wrote:
> On 2019-01-03 14:33, Thomas Huth wrote:
>> On 2019-01-03 14:23, Janosch Frank wrote:
>>> On 03.01.19 14:15, Thomas Huth wrote:
>>>> On 2019-01-03 14:13, Janosch Frank wrote:
>>>>> On 03.01.19 13:58, Thomas Huth wrote:
>>>>>> On 2019-01-03 11:08, Janosch Frank wrote:
>>>>>>> We need to properly implement interrupt handling for SCLP, because on
>>>>>>> z/VM and LPAR SCLP calls are not synchronous!
>>>>>>>
>>>>>>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>>>> [...]
>>>>>>> +
>>>>>>>  static void sclp_read_scp_info(ReadInfo *ri, int length)
>>>>>>>  {
>>>>>>>  	unsigned int commands[] = { SCLP_CMDW_READ_SCP_INFO_FORCED,
>>>>>>>  				    SCLP_CMDW_READ_SCP_INFO };
>>>>>>> -	int i;
>>>>>>> +	int i, cc;
>>>>>>>  
>>>>>>>  	for (i = 0; i < ARRAY_SIZE(commands); i++) {
>>>>>>>  		memset(&ri->h, 0, sizeof(ri->h));
>>>>>>>  		ri->h.length = length;
>>>>>>>  
>>>>>>> -		if (sclp_service_call(commands[i], ri))
>>>>>>> +		sclp_mark_busy();
>>>>>>> +		cc = sclp_service_call(commands[i], ri);
>>>>>>> +		sclp_wait_busy();
>>>>>>
>>>>>> You already do the sclp_wait_busy() in sclp_service_call now, so I think
>>>>>> you don't need the sclp_wait_busy() here anymore?
>>>>>
>>>>> Yeah, that has to go.
>>>>>
>>>>>>
>>>>>> Also, what about moving the sclp_mark_busy() calls to the beginning of
>>>>>> sclp_service_call() instead?
>>>>>
>>>>> Wouldn't that create a race on the data of __sccb and we could end with
>>>>> garbled scb commands?
>>>>
>>>> Since there is a sclp_wait_busy in sclp_service_call already, you can be
>>>> sure that the previous command already finished, can't you?
>>>
>>> I mean before calling sclp_service_call.
>>> That's only a problem for smp, but before calling sclp, we prepare the
>>> data in the shared __sccb page and that is currently protected by the
>>> busy mark. If it's not, then two threads could write to __sccb at the
>>> same time and the first that succeeds to call sclp will get a mix of
>>> data of both threads.
>>
>> But in that case, you also need to do a sclp_wait_busy() before calling
>> sclp_mark_busy() in all locations - otherwise the code is also not
>> thread-safe in its current shape, is it?
> 
> Ah, never mind, now I had a look at patch 11/13, and now it makes sense.
> So maybe fold patch 11 into this one, to make this clear right from the
> start?
> 
>  Thomas
> 

Already done :)


Attachment: signature.asc
Description: OpenPGP digital signature


[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