Re: [PATCH 2/5] x86/ucode: Fold early_update_cache() into microcode_init_cache()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.03.2023 11:13, George Dunlap wrote:
> On Tue, Mar 28, 2023 at 4:58 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
>> On 28.03.2023 17:07, Andrew Cooper wrote:
>>> On 28/03/2023 2:51 pm, Jan Beulich wrote:
>>>> On 27.03.2023 21:41, Andrew Cooper wrote:
>>>>> It is not valid to retain a bootstrap_map() across returning back to
>>>>> __start_xen(), but various pointers get stashed across calls.
>>>> It's error prone, yes, but "not valid" isn't really true imo: As long as
>>>> nothing calls bootstrap_map(NULL) all mappings will remain as they are.
>>>
>>> And how is this code supposed to know whether it's stashed pointer is
>>> any good or not?
>>>
>>> This is precisely "not valid" means.  It doesn't mean "it definitely
>>> doesn't work", but very much does mean "can't rely on it working".
>>
>> Hmm, not my understanding of "not valid".
>>
> 
> A "valid" approach or algorithm is one which can be relied on.  If an
> approach or algorithm may sometimes work or may sometimes not work, it's
> not valid.

Interesting. Still not my understanding of "not valid", not the least
because "may work" again depends on further aspects. In the case here,
carefully placed (or avoided) bootstrap_map(NULL) means the logic, which
has served us well for years, does always work - so long as you apply
sufficient care. Over time that "sufficient care" has arguably turned
into "overly much care", and hence Andrew's rework here is definitely an
improvement. All I dislike is to call things worse than they really are.
(Another thing would be if "sufficient care" wasn't applied at some
point, and if as a result we actually had active breakage somewhere.)

> That said:
> 
> * "Not valid" is kind of vague: it tells you think it's "bad", but not
> how.  (Even "racy" or "risky" or "error-prone" are more descriptive,
> because it prompts you for the types of problems to think about.) It's
> usually better to state exactly what problems might happen if you do X,
> rather than simply saying X is "not valid".
> 
> * It's usually better to propose specific alternate wording, rather than
> arguing about whether a given wording is... er, valid or not.

Which I did (without explicitly saying that this is an alternative wording
proposal) by starting with "It's error prone, ...". Similarly (iirc) in
replies elsewhere on this series' thread.

Jan