Re: [PATCH v2 4/7] s390/physmem_info: query diag500(STORAGE LIMIT) to support QEMU/KVM memory devices

[David Hildenbrand <david@redhat.com>]

On 17.10.24 11:53, Alexander Gordeev wrote:
>>> Why search_mem_end() is not tried in case sclp_early_get_memsize() failed?
>>
>> Patch #3 documents that:
>>
>> +    The storage limit does not indicate currently usable storage, it may
>> +    include holes, standby storage and areas reserved for other means, such
>> +    as memory hotplug or virtio-mem devices. Other interfaces for detecting
>> +    actually usable storage, such as SCLP, must be used in conjunction with
>> +    this subfunction.
> 
> Yes, I read this and that exactly what causes my confusion. In this wording it
> sounds like SCLP *or* other methods are fine to use. But then you use SCLP or
> DIAGNOSE 260, but not memory scanning. So I am still confused ;)

Well, DIAGNOSE 260 is z/VM only and DIAG 500 is KVM only. So there are 
currently not really any other reasonable ways besides SCLP.

> 
>> If SCLP would fail, something would be seriously wrong and we should just crash
>> instead of trying to fallback to the legacy way of scanning.
> 
> But what is wrong with the legacy way of scanning?

Missing to detect holes and starting to use them, detecting and using 
device memory without negotiating with the device ... it all falls to 
pieces.

> 
>>>> +	case MEM_DETECT_DIAG500_STOR_LIMIT:
>>>> +		return "diag500 storage limit";
>>>
>>> AFAIU you want to always override MEM_DETECT_DIAG500_STOR_LIMIT method
>>> with an online memory detection method. In that case this code is dead.
>>
>> Not in the above case, pathological case above where something went wrong
>> during sclp_early_get_memsize(). In that scenario, die_oom() would indicate
>> that there are no memory ranges but that "diag500 storage limit" worked.
>>
>> Does that make sense?
> 
> Yes, I get your approach.

Thanks, please let me know if I should make it clearer in the 
description, of if you think we can improve the code.

-- 
Cheers,

David / dhildenb