Re: [libvirt] [PATCH v5 4/6] udev: Convert udevEventHandleThread to an actual thread routine
On Sun, Oct 15, 2017 at 10:23:56AM -0400, John Ferlan wrote:
On 10/11/2017 10:52 AM, Erik Skultety wrote:
> Adjust udevEventHandleThread to be a proper thread routine running in an
> infinite loop handling devices. The handler thread pulls all available
> data from the udev monitor and only then waits until a wakeup signal for
> new incoming data has been emitted by udevEventHandleCallback.
>
This doing a bit more by removing the driver locks from initialization
too. Perhaps those locks should be kept on Initialization for now and
then in a followup patch remove them since @privateData (or whatever
name it becomes) is then totally self locking.
I don't think we'd run into any deadlocks since Initialization and
Cleanup would still be the only consumers.
Like I said, patch will be added.
[...]
> static bool
> @@ -1625,15 +1639,25 @@ udevPCITranslateDeinit(void)
> static int
> nodeStateCleanup(void)
> {
> + udevEventDataPtr priv = NULL;
> +
> if (!driver)
> return -1;
>
> + priv = driver->privateData;
> +
Although perhaps impossible, better safe than sorry
Here it's actually quite possible, when allocation of the private data in
Initialize fails, cleanup is performed, so this would get a SIGSEGV, thanks.
Erik
Perhaps we should keep it just for this patch and remove in the
"next"
patch leaving the goto unlock intact, but modifying it to be :
unlock:
if (priv)
virObjectUnlock(priv);
nodeDeviceUnlock();
Then the next patch alters the gots's and unlock: label to what this
patch has. It also removes nodeDeviceLock from Cleanup
Yeah, I'll handle the locking prior to adding this thread-related stuff.