On 21.01.19 09:58, David Hildenbrand wrote:
> On 18.01.19 12:42, Janosch Frank wrote:
[...]
>> +void sclp_handle_ext(void)
>> +{
>> + ctl_clear_bit(0, 9);
>> + spin_lock(&sclp_lock);
>> + sclp_busy = false;
>> + spin_unlock(&sclp_lock);
>
> With multiple CPUs, this is problematic. Just when the interrupt comes
> in, SCLP is marked as !busy and the SCCB block can immediately be reused
> by other callers waiting in sclp_mark_busy().
>
> Wasn't the idea to also protect the whole global SCCB?
>
> We should maybe stop using a gloval page and instead get one per caller
> via the page allocator. Or let the caller of sclp_mark_busy() set SCLP
> as !busy once he's done handling the block.
Yes, I need to rethink all of that properly instead of fixing around it.
We could for now use a two stage lock/indication:
mark_busy()
On sclp call sclp code sets processed = false;
svc interrupt sets processed = true
Caller loops on processed and unlocks busy after processing.
Later on we either use pcpu pages and locks or stay with it, as it's
less complicated.
That's from the top of my head, so -ENOCOFFEE
>
>> +}
>> +
>> +void sclp_wait_busy(void)
>> +{
>> + while (sclp_busy)
>> + mb();
>> +}
>> +
>> +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);
>> +}
>> +
>> 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++) {
>> + sclp_mark_busy();
>> memset(&ri->h, 0, sizeof(ri->h));
>> ri->h.length = length;
>>
>> - if (sclp_service_call(commands[i], ri))
>> + cc = sclp_service_call(commands[i], ri);
>> + if (cc)
>> break;
>> if (ri->h.response_code == SCLP_RC_NORMAL_READ_COMPLETION)
>> return;
>> @@ -57,12 +105,14 @@ int sclp_service_call(unsigned int command, void *sccb)
>> {
>> int cc;
>>
>> + sclp_setup_int();
>> asm volatile(
>> " .insn rre,0xb2200000,%1,%2\n" /* servc %1,%2 */
>> " ipm %0\n"
>> " srl %0,28"
>> : "=&d" (cc) : "d" (command), "a" (__pa(sccb))
>> : "cc", "memory");
>> + sclp_wait_busy();
>> if (cc == 3)
>> return -1;
>> if (cc == 2)
>> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
>> index 583c4e5..63cf609 100644
>> --- a/lib/s390x/sclp.h
>> +++ b/lib/s390x/sclp.h
>> @@ -213,6 +213,9 @@ typedef struct ReadEventData {
>> } __attribute__((packed)) ReadEventData;
>>
>> extern char _sccb[];
>> +void sclp_handle_ext(void);
>> +void sclp_wait_busy(void);
>> +void sclp_mark_busy(void);
>> void sclp_console_setup(void);
>> void sclp_print(const char *str);
>> int sclp_service_call(unsigned int command, void *sccb);
>>
>
>
Attachment:
signature.asc
Description: OpenPGP digital signature
