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

Re: [PATCH v2 05/10] xen/x86: Use ASSERT instead of VIRTUAL_BUG_ON for phys_to_nid


  • To: Wei Chen <Wei.Chen@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 27 Apr 2022 07:56:48 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=lbKitOelmPXblbKTvzwSDY3ph0VV9YlpwIwvC8edFjc=; b=YUUMf6brsnIMHjWQbGofCQmbOazHZr7EnA64hRG4NiclWBa07YUStsQ6Z+fVbnRw8LjdaOGQAsflN/WGTsDYPnHuM8iQ23vqqNsvcjbcdt0DGHtsZVTrgApfjiPkX2zHidwANfZ+ogEnovuXFdK0d/zf+0SFKHoxrc+/5SIjQBQrsTZyZcztZpvdy5zQ1N9cqbrA9n6RV8nV1OnENmHvu1O2fHP9UbPjJcgtQ/+Z/rE3Nx+STbWMwa7nnOVwd2yxW/isUBgQNfrxpFgihOo89NJB14SOz9u6oYUCATcC3Y3FbEI6sgNKJ9AErWzwPNHABJvYYqQzGjXgJktkRtZqzQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hN+6demHQXGVCtRByGAxn9EuXC4cu8beqrw3u2ek0BjpeTeWlU78dTgiDpMFHo64T84UqYPMxv0P8idb+vQaGtj6HUOaktMDUHo12IcxpVoVCl3e1fwciEn/FdbK/i38BPNJkihf3fduppSdyEtSi9VJcyQ5Fd/URmM01BUtejDbeW8Ty44eNrttP3Yz3AyLtZ+8pLKTRFmJ8mzUs0vyVfg8+bBez93gtkFV7FJ2bIw30a8YYwEtNUuoU6405B5mCz6lDYjxBTEBLXEZOfOOUmZMiI8b14jZZBmIAVT0nnc3argUWT+vyxT9fMdQ7eX7h0+1dE9LsEs+eWA72ZmUug==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: nd <nd@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 27 Apr 2022 05:57:09 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 27.04.2022 05:52, Wei Chen wrote:
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: 2022年4月26日 22:42
>>
>> On 26.04.2022 12:59, Wei Chen wrote:
>>> On 2022/4/26 17:02, Jan Beulich wrote:
>>>> On 18.04.2022 11:07, Wei Chen wrote:
>>>>> 2. error: wrong type argument to unary exclamation mark.
>>>>>     This is because, the error-checking code contains !node_data[nid].
>>>>>     But node_data is a data structure variable, it's not a pointer.
>>>>>
>>>>> So, in this patch, we use ASSERT instead of VIRTUAL_BUG_ON to
>>>>> enable the two lines of error-checking code. And fix the left
>>>>> compilation errors by replacing !node_data[nid] to
>>>>> !node_data[nid].node_spanned_pages.
>>>>>
>>>>> Because when node_spanned_pages is 0, this node has no memory,
>>>>> numa_scan_node will print warning message for such kind of nodes:
>>>>> "Firmware Bug or mis-configured hardware?".
>>>>
>>>> This warning is bogus - nodes can have only processors. Therefore I'd
>>>> like to ask that you don't use it for justification. And indeed you
>>>
>>> Yes, you're right, node can only has CPUs! I will remove it.
>>>
>>>> don't need to: phys_to_nid() is about translating an address. The
>>>> input address can't be valid if it maps to a node with no memory.
>>>>
>>>
>>> Can I understand your comment:
>>> Any input address is invalid, when node_spanned_pages is zero, because
>>> this node has no memory?
>>
>> It's getting close, but it's not exactly equivalent I think. A node
>> with 0 bytes of memory might (at least in theory) have an entry in
>> memnodemap[]. But finding a node ID for that address would still
> 
> I have done a quick check in populate_memnodemap:
> 74          spdx = paddr_to_pdx(nodes[i].start);
> 75          epdx = paddr_to_pdx(nodes[i].end - 1) + 1;
> 76          if ( spdx >= epdx )
> 77              continue;
> 
> It seems that if node has no memory, start == end, then this function
> will not populate memnodemap entry for this node.
> 
>> not mean that at least one byte of memory at that address is present
>> on the given node, because the node covers 0 bytes.
>>
> 
> And back to this patch, can I just drop the unnecessary justification
> from the commit message?

Well, you'll want to have _some_ justification for that particular
aspect of the patch.

> And for the bogus warning message, can I update it to an INFO level
> message in part#2 series, and just keep:
> printk(KERN_INFO "SRAT: Node %u has no memory!\n", i);
> but remove "BIOS Bug or mis-configured hardware?\n", i); ?

This sounds at least plausible to do.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.