Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: The second batch of qwz backports and fixes
To:
tech@openbsd.org
Date:
Mon, 28 Sep 2026 13:34:40 +0200

Download raw body.

Thread
On Mon, 28 Sep 2026 13:14:44 +0200,
Stefan Sperling <stsp@stsp.name> wrote:
> 
> On Mon, Sep 28, 2026 at 12:06:22PM +0200, Kirill A. Korinsky wrote:
> > From 8636d1b7d6412861f72d8c9ebfe9064b3337e5c8 Mon Sep 17 00:00:00 2001
> > From: "Kirill A. Korinsky" <kirill@korins.ky>
> > Date: Sun, 27 Sep 2026 12:57:24 +0200
> > Subject: [PATCH 05/11] sys/qwz: wait for confirmed peer release
> > 
> > Wait for both an explicit peer-unmap event and the delete response
> > before releasing a peer. This also covers a creation that timed out
> > before its mapping event arrived.
> 
> All the other diffs are OK by me.
> 
> Only this one commit looks a bit strange, and I cannot understand
> the reasoning behind it without more details.
> 
> I cannot tell what the first part of your log message is trying to say
> with "Wait for both an explicit peer-unmap event and the delete response".
> The code which unmaps and deletes a peer already handles both events.
> How is the existing code broken?
> 
> Previously, the peer_mapped flag was used to track both map and unmap events,
> for no particular reason other than that the same flag can be used for both.
> 
> Now you've added a dedicated unmapped flag to track unmap events separately,
> but still report unmapping events to code which sleeps on &sc->peer_mapped.
> Is this intentional? Is this what the following part of your log message is
> referring to? "This also covers a creation that timed out before its mapping
> event arrived."  What problem does this solve exactly? A timeout would rather
> be handled via error codes returned from tsleep, wouldn't it?
> 

Well, not only. If timeout had happened we never call this block:

	TAILQ_REMOVE(&sc->peers, peer, entry);
	qwz_node_clear_peer_id(sc, peer);
	free(peer, M_DEVBUF, sizeof(*peer));
	sc->num_peers--;

and we ignore error in qwz_stop() for example.

If I not wrong, it will leak a bit.

Am I wrong?

-- 
wbr, Kirill