NetBSD Problem Report #58167

From www@netbsd.org  Wed Apr 17 18:46:52 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 16E541A9238
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 17 Apr 2024 18:46:52 +0000 (UTC)
Message-Id: <20240417184650.D56611A923A@mollari.NetBSD.org>
Date: Wed, 17 Apr 2024 18:46:50 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: unnecessary mutex_tryenter in tap(4)
X-Send-Pr-Version: www-1.0

>Number:         58167
>Category:       kern
>Synopsis:       unnecessary mutex_tryenter in tap(4)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Apr 17 18:50:01 +0000 2024
>Closed-Date:    Fri Oct 11 19:00:19 +0000 2024
>Last-Modified:  Fri Oct 11 19:00:19 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, 8, ...
>Organization:
The TryTAP Foundation
>Environment:
>Description:
    875 	/* In the TAP_NBIO case, we have to make sure we won't be sleeping */
    876 	if ((sc->sc_flags & TAP_NBIO) != 0) {
    877 		if (!mutex_tryenter(&sc->sc_lock))
    878 			return EWOULDBLOCK;
    879 	} else
    880 		mutex_enter(&sc->sc_lock);

https://nxr.netbsd.org/xref/src/sys/net/if_tap.c?r=1.128#875

This lock is only used to serialize access to the packet queue, not to serialize operations that sleep for I/O or anything -- it's a spin lock anyway (though I'm not sure there's any reason for it to be).  So there is no need to make tap_dev_read flakier and harder to understand by using mutex_tryenter.
>How-To-Repeat:
code inspection
>Fix:
just use mutex_enter unconditionally

>Release-Note:

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58167 CVS commit: src/sys/net
Date: Wed, 17 Apr 2024 18:52:55 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Wed Apr 17 18:52:54 UTC 2024

 Modified Files:
 	src/sys/net: if_tap.c

 Log Message:
 tap(4): Just use mutex_enter.

 PR kern/58167


 To generate a diff of this commit:
 cvs rdiff -u -r1.131 -r1.132 src/sys/net/if_tap.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Tue, 23 Jul 2024 21:57:12 +0000
State-Changed-Why:
fixed in HEAD, needs pullup-10 and pullup-9


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58167 CVS commit: [netbsd-10] src/sys/net
Date: Wed, 11 Sep 2024 16:12:50 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Wed Sep 11 16:12:50 UTC 2024

 Modified Files:
 	src/sys/net [netbsd-10]: if_tap.c

 Log Message:
 Pull up following revision(s) (requested by rin in ticket #825):

 	sys/net/if_tap.c: revision 1.129
 	sys/net/if_tap.c: revision 1.130
 	sys/net/if_tap.c: revision 1.131
 	sys/net/if_tap.c: revision 1.132

 tap(4): Prune dead branches around tap_dev_close.
 No functional change intended.
 PR kern/58166

 tap(4): Prune dead branches around tap_dev_destroyer.
 No functional change intended.
 PR kern/58166

 tap(4): Use DETACH_FORCE with config_detach.
 It doesn't make a difference here, because tap_detach never fails,
 but let's make it more obvious at the call site that failure is
 forbidden here.

 No functional change intended.
 PR kern/58166

 tap(4): Just use mutex_enter.
 PR kern/58167


 To generate a diff of this commit:
 cvs rdiff -u -r1.127.4.1 -r1.127.4.2 src/sys/net/if_tap.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: needs-pullups->closed
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Fri, 11 Oct 2024 19:00:19 +0000
State-Changed-Why:
ixed in HEAD, pulled up to 10, not gonna work on this in 9


>Unformatted:

Home	PR Database Search