[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0b1837f9-57ef-8deb-df78-cb7841d93d68@oracle.com>
Date: Thu, 28 Oct 2021 15:08:51 -0400
From: Boris Ostrovsky <boris.ostrovsky@...cle.com>
To: Oleksandr Tyshchenko <olekstysh@...il.com>,
xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org
Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
Juergen Gross <jgross@...e.com>,
Stefano Stabellini <sstabellini@...nel.org>,
Julien Grall <julien@....org>
Subject: Re: [PATCH V2 3/4] xen/unpopulated-alloc: Add mechanism to use Xen
resource
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
Powered by blists - more mailing lists