Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <0b1837f9-57ef-8deb-df78-cb7841d93d68@oracle.com>
Date:   Thu, 28 Oct 2021 15:08:51 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.2.1
Subject: Re: [PATCH V2 3/4] xen/unpopulated-alloc: Add mechanism to use Xen
 resource
Content-Language: en-US
To:     Oleksandr Tyshchenko <olekstysh@gmail.com>,
        xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org
Cc:     Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
        Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Julien Grall <julien@xen.org>
References: <1635264312-3796-1-git-send-email-olekstysh@gmail.com>
 <1635264312-3796-4-git-send-email-olekstysh@gmail.com>
From:   Boris Ostrovsky <boris.ostrovsky@oracle.com>
In-Reply-To: <1635264312-3796-4-git-send-email-olekstysh@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NU5jZG5KSktGNzBuZXdOaWlsK095VDVYOFppb0Uzck1HelZtY2t2a0p6bk80?=
 =?utf-8?B?b0p1aUJwSXY2cG5zc3BaanViMENjVVJtSVpCMHVHZzZFZUxNL3J2anNsMDZL?=
 =?utf-8?B?eEV4cHE0TU9HMWIxamxBbktHaU45a2lhMEdhU1BhNjhLY0NJUk9TaW9keExO?=
 =?utf-8?B?NkFYTnE0NzNsdjd3U2VpeEFjYy9sMWd0VWZtK2x1VmVQKzdORGxubXVQR0tI?=
 =?utf-8?B?bmJaZUNWNFBnVURtWUJocXhnWE5xQUZJdnowU0Y1c2pFWFFvV3V4Z3czczVo?=
 =?utf-8?B?U3FJcWI2bFUwOVNtcjc5cEtRV3JhQkxaZzhDN0JuOVo4OTErb1haL0FhYkVX?=
 =?utf-8?B?clNVUzhuaHphcmhYTFFMazUweVpLVU12UUpyaVVGaEgrOUZ3U3pwdW0zeG43?=
 =?utf-8?B?bUp5MkJ2OXRzdzFkeXBnVWZubFRUa0czeG5UTmFvRnFDdXlkRWlhalRtN0JU?=
 =?utf-8?B?YXcxTlA3QnJ2aVk5ZW5MMHhTbU5WOXo5V25aMEs2Z09nL1FQajU4ZlVDVDJS?=
 =?utf-8?B?YmRQc3gzS2hQdVNoL2hOUWtBdWhCRWV2M2lIbXZiR00xL3FhbTMrWmFSZjND?=
 =?utf-8?B?MTczTTg4MExlNzRBNDduVnBud2ljTTl1OGtFZ0lzM0VvME5TWnhxL0UwZEYv?=
 =?utf-8?B?aVFJYVpuUDZZY000MWNDS0pLcjZCRTNhTlF2dXp2MXlESDVZKzdEaTF5WE4z?=
 =?utf-8?B?cUxjOWlWa2xjQzh5d3NnOGVlTzNPYUxQZXpMYUQ3M2pyMG9WQnk0UytoSnpV?=
 =?utf-8?B?MWNlUUpMVExjMTA4YkJWVkU4RzJnQjVzZHdFSzM5Q1Y0N2xSa0VNQlJzeHU3?=
 =?utf-8?B?SnVuNUl6VHRUcXI2VzZ3bU1jQ0RNc0pTa2p0U2dPQkNwbEtZUkJ6TWsrblA0?=
 =?utf-8?B?RGhWTnZCaEs1eVlUc0NocU1kYWpmMzJkUWdSNmdPTGxoanBjdkZ6bHNuTVhm?=
 =?utf-8?B?cktlcU40ZmVLRlQyZkt4YVBtU08zMGVKbWVJUGNpejZzZ3AzOUJDMGZseUZ3?=
 =?utf-8?B?YlVobnBYLzNING52MDdOckRZeWp0Ny9aTW5CT1NJazlvVXBsYjFtL3hQMlN5?=
 =?utf-8?B?U0YzUGg3YjNiaVpMNmgyYzhTUGYwNlF6ejVOUUU0dGVPODlqZXk4ZEdYNGJI?=
 =?utf-8?B?ckFCVFdRYmZ2TEVKVzl1d0U2ZlQza0V3ZHJDdXpsbUdydUpKcURrbURqNTNx?=
 =?utf-8?B?Vzk2NHc4QlBMK3EvRk8zV21NWmNSdCtBK0o3bXRpcEpRZERldjh3SndQN1hp?=
 =?utf-8?B?SjZ2WkxJRGJRMll3RFVQZ284dmFrMFlkMkJoRjBOYXhpRlhWbGRDUFZkTjhw?=
 =?utf-8?B?MThkL2k4ME9IU3libTUranhuakdwNUhjSHdlNDZmTC9tTFRLNDZ1Z3RmTEpa?=
 =?utf-8?B?aXlpSnBodEVpMzFSZW9zaVlHM2daSE51UlNwaG53ajhSa1UvUEF2VEdRT25V?=
 =?utf-8?B?c2VvUy9TaDMvOWpTRzZLVVdrRExucUowanVQYUt2eHR6UWtZL01MM0VqUG9Q?=
 =?utf-8?B?V1RlZGcxQ0RBeEc4T2ROek9STUVhYzBwQzhxV2Q0ZGhtK3lJM1RmQm1xTXFR?=
 =?utf-8?B?U2s5dTFkT3NTVU9IdGdpdmFMZHNsbXdUMGNrcHc4dlk0SmVsMi9tSWUxbmdO?=
 =?utf-8?B?aGpucU9zVkFNQ05ZZVhHOWttMnZxdi9QRHBidzIxN0pVQ3VxdTlyU0dIZ0Nv?=
 =?utf-8?B?VENwZk9nc0ovYmg5Rk5CNWFqbWpSVE9EdFNhcnVLd3ZrQ0tYODErS21BODE1?=
 =?utf-8?B?QWxSdElDZDd5OE8yK296YzFiV3NNa1lKR1ZmQitHa3FYbmJHTER2a3AweS95?=
 =?utf-8?B?Wll6M21aN3VGTUNtTnY3Z2tSWE5oRmgwUUI5Y1g1dW1meUc3enZ1UGgyaUZ5?=
 =?utf-8?B?MUJUZi9vdTBWSmM2aTZyNkVTVWNhYjVzWlBhbUR6aTQ1cjZzWGRvdGZ4aUlF?=
 =?utf-8?B?N1NHcGV4Y0M1VW9QSXl3NytLVUxGWmEzWDdUaVRZbks5Yy9WdmQrVU0zakM2?=
 =?utf-8?B?NU9LOU0rVkt5d3lIVnVpbHNJelk4Wmp5YklrUXV1YVdaWm9UaXkveEM0enU5?=
 =?utf-8?B?Vlk3WU92SkpYaTFTSnVFeXdOUUR4dzYrbmpkcWtLd1RoRjFNVm9EWTIxZmk2?=
 =?utf-8?B?L1pxMGw3a3krWXNNNTh6UWx0cnF2RlQ3MjlycXlEdzJDczNRQmNRZVBnYnZq?=
 =?utf-8?B?RURocUZNR1liQmk1VktKaUZqUHJhdk9iY0R6eE5QVWFLQWJKamhuSjNZS3dC?=
 =?utf-8?Q?6phB7HTmS99wQBboBnLDhg0xceDPaKyJqz9EoC/vG8=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 88bb9532-1987-45ec-6265-08d99a466389
X-MS-Exchange-CrossTenant-AuthSource: BLAPR10MB5009.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Oct 2021 19:08:55.9624
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8nnkcjxKWK0q00HGxr4/be0nFfL3PS1gE1U+k7NuJWAh5LpvdIAsXQQmgCSFUdmXvTA4IvgN1HlrtpQv1Bl3J0V1ZftqDgxRXU30hXP6B7U=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR10MB2980
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10151 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 malwarescore=0
 adultscore=0 suspectscore=0 bulkscore=0 mlxscore=0 spamscore=0
 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2110280099
X-Proofpoint-GUID: HKPxr1Rf9kkDtqz470HThbNiVPGDYUBf
X-Proofpoint-ORIG-GUID: HKPxr1Rf9kkDtqz470HThbNiVPGDYUBf
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 10/26/21 12:05 PM, Oleksandr Tyshchenko wrote:
>   
> +static void unpopulated_init(void)
> +{
> +	static bool inited = false;
> +	int ret;
> +
> +	if (inited)
> +		return;
> +
> +	/*
> +	 * Try to initialize Xen resource the first and fall back to default
> +	 * resource if arch doesn't offer one.
> +	 */
> +	ret = arch_xen_unpopulated_init(&xen_resource);
> +	if (!ret)
> +		target_resource = &xen_resource;
> +	else if (ret == -ENOSYS)
> +		target_resource = &iomem_resource;
> +	else
> +		pr_err("Cannot initialize Xen resource\n");


I'd pass target_resource as a parameter to arch_xen_unpopulated_init() instead. Default routine will assign it iomem_resource and you won't have to deal with -ENOSYS.


Also, what happens in case of error? Is it fatal? I don't think your changes in fill_list() will work.


> +
> +	inited = true;


I agree with Stefano in that it would be better to call this from an init function, and you won't have t worry about multiple calls here.


-boris