Re: [libvirt PATCH 1/5] nwfilter: stop using recursive mutex for IP learning
On Mon, Feb 28, 2022 at 11:05:09AM -0500, Laine Stump wrote: > On 2/25/22 10:30 AM, Daniel P. Berrangé wrote: > > The virNWFilterLockIface method is only called from one place, > > the learnIPAddressThread method. > > nwfilter_gentech_driver.c:virNWFilterDoInstantiate() and > _virNWFilterTeardownFilter() also call virNWFilterLockIface. Sigh, yes. I didn't properly consider that stuff outside of learnip.c would be using its mutex. We should move the iface locking into the gentech driver to make it clear is is shared functionality. Consider this patch dropped. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 1/5] nwfilter: stop using recursive mutex for IP learning
On 2/25/22 10:30 AM, Daniel P. Berrangé wrote: The virNWFilterLockIface method is only called from one place, the learnIPAddressThread method. nwfilter_gentech_driver.c:virNWFilterDoInstantiate() and _virNWFilterTeardownFilter() also call virNWFilterLockIface. And there is this call chain from learnIPAddressThread() that ends up at one or the other of those: learnIPAddressThread() virNWFilterInstantiateFilterLate() virNWFilterInstantiateFilterUpdate() virNWFilterDoInstantiate() < _virNWFilterTeardownFilter() < But of course just prior to calling virNWFilterINstantiateFilterLate(), learnIPAddressThread() calls: virNWFilterUnlockIface(req->binding->portdevname); which matches the previous virNWFilterLockIface(), so there shouldn't be a problem for the learnIPAddressThread at least. I'm not sure about the threads of other calls to virNWFilterDoInstantiate/TeardownFilter though, haven't tried to decipher them yet. This is the entry point for the learning thread on the interface in question. As such there is never any recursive locking on this mutex from the same thread. This appears to have been the case since the code was first introduced. Thus a plain mutex is sufficient. Signed-off-by: Daniel P. Berrangé --- src/nwfilter/nwfilter_learnipaddr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index fb552bd1e6..4840b0539f 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -151,7 +151,7 @@ virNWFilterLockIface(const char *ifname) if (!ifaceLock) { ifaceLock = g_new0(virNWFilterIfaceLock, 1); -if (virMutexInitRecursive(>lock) < 0) { +if (virMutexInit(>lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("mutex initialization failed")); g_free(ifaceLock);
[libvirt PATCH 1/5] nwfilter: stop using recursive mutex for IP learning
The virNWFilterLockIface method is only called from one place, the learnIPAddressThread method. This is the entry point for the learning thread on the interface in question. As such there is never any recursive locking on this mutex from the same thread. This appears to have been the case since the code was first introduced. Thus a plain mutex is sufficient. Signed-off-by: Daniel P. Berrangé --- src/nwfilter/nwfilter_learnipaddr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index fb552bd1e6..4840b0539f 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -151,7 +151,7 @@ virNWFilterLockIface(const char *ifname) if (!ifaceLock) { ifaceLock = g_new0(virNWFilterIfaceLock, 1); -if (virMutexInitRecursive(>lock) < 0) { +if (virMutexInit(>lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("mutex initialization failed")); g_free(ifaceLock); -- 2.35.1