Re: [Xen-devel] [PATCH 1/3] xen/x86: delay parsing of dom0_mem parameter until needed

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.11.18 at 16:33, <jgross@xxxxxxxx> wrote:
> On 28/11/2018 16:12, Jan Beulich wrote:
>>>>> On 22.11.18 at 17:40, <jgross@xxxxxxxx> wrote:
>>> @@ -44,14 +46,20 @@ static long __initdata dom0_max_nrpages = LONG_MAX;
>>>   *  If +ve: The specified amount is an absolute value.
>>>   *  If -ve: The specified amount is subtracted from total available memory.
>>>   */
>>> -static long __init parse_amt(const char *s, const char **ps)
>>> +static unsigned long __init parse_amt(const char *s, const char **ps,
>>> +                                      unsigned long avail)
>>>  {
>>> -    long pages = parse_size_and_unit((*s == '-') ? s+1 : s, ps) >> 
>>> PAGE_SHIFT;
>>> -    return (*s == '-') ? -pages : pages;
>>> +    unsigned int minus = (*s == '-') ? 1 : 0;
>>> +    unsigned long pages = parse_size_and_unit(s + minus, ps) >> PAGE_SHIFT;
>>> +
>>> +    /* Negative specification means "all memory - specified amount". */
>>> +    return minus ? avail - pages : pages;
>>>  }
>> 
>> I don't think any of this should be done in a patch with the given
>> title.
> 
> Going the other route will result in merging patches 1 and 2, I guess.

Well, you could easily do here what you do _besides_ the actual
topic of the patch, just then with its title changed.

>>> @@ -298,6 +306,10 @@ unsigned long __init dom0_compute_nr_pages(
>>>          (!iommu_hap_pt_share || !paging_mode_hap(d));
>>>      for ( ; ; need_paging = false )
>>>      {
>>> +        if ( dom0_mem_par[0] && parse_dom0_mem(avail) )
>>> +            printk("Invalid dom0_mem parameter value \"%s\", ignoring\n",
>>> +                   dom0_mem_par);
>> 
>> Looking at how the parsing function works I don't think the entire
>> command line would necessarily be ignored in case of error. I think
>> the log message shouldn't give a wrong impression.
> 
> The message is referencing the dom0_mem value only. And this would be
> ignored. I don't see your concern here.

The function may have updated all three of dom0_{,max_,min_}nrpages
before hitting en error condition. Hence a fair part of what was specified
may not be ignored despite the error.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel