Re: [PATCH] xen: Fix XenStore initialisation for XS_LOCAL

From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: "David Woodhouse" <dwmw2@infradead.org>,
	"Jürgen Groß" <jgross@suse.com>,
	x86@kernel.org
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Paul Durrant <pdurrant@amazon.com>,
	jgrall@amazon.com, karahmed@amazon.de,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] xen: Fix XenStore initialisation for XS_LOCAL
Date: Tue, 26 Jan 2021 16:36:41 -0500	[thread overview]
Message-ID: <da79f47d-de72-1c27-71e4-394b17577682@oracle.com> (raw)
In-Reply-To: <4c9af052a6e0f6485d1de43f2c38b1461996db99.camel@infradead.org>


On 1/26/21 12:01 PM, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> In commit 3499ba8198ca ("xen: Fix event channel callback via INTX/GSI")
> I reworked the triggering of xenbus_probe().
> 
> I tried to simplify things by taking out the workqueue based startup
> triggered from wake_waiting(); the somewhat poorly named xenbus IRQ
> handler.
> 
> I missed the fact that in the XS_LOCAL case (Dom0 starting its own
> xenstored or xenstore-stubdom, which happens after the kernel is booted
> completely), that IRQ-based trigger is still actually needed.
> 
> So... put it back, except more cleanly. By just spawning a xenbus_probe
> thread which waits on xb_waitq and runs the probe the first time it
> gets woken, just as the workqueue-based hack did.
> 
> This is actually a nicer approach for *all* the back ends with different
> interrupt methods, and we can switch them all over to that without the
> complex conditions for when to trigger it. But not in -rc6. This is
> the minimal fix for the regression, although it's a step in the right
> direction instead of doing a partial revert and actually putting the
> workqueue back. It's also simpler than the workqueue.


Wouldn't the minimal fix be to restore wake_waiting() to its previous 

        if (unlikely(xenstored_ready == 0)) {
                xenstored_ready = 1;
                schedule_work(&probe_work);
        }

(And to avoid changing xenbus_probe()'s signature just create a wrapper)

-boris