NetBSD Problem Report #47114

From campbell@mumble.net  Tue Oct 23 20:06:02 2012
Return-Path: <campbell@mumble.net>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id AABF363E398
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 23 Oct 2012 20:06:02 +0000 (UTC)
Message-Id: <20121023200523.E6A15604BF@jupiter.mumble.net>
Date: Tue, 23 Oct 2012 20:05:23 +0000 (UTC)
From: Taylor R Campbell <campbell+netbsd@mumble.net>
Reply-To: Taylor R Campbell <campbell+netbsd@mumble.net>
To: gnats-bugs@gnats.NetBSD.org
Subject: it's too easy to misspell `mutex_owned' as `mutex_owner'
X-Send-Pr-Version: 3.95

>Number:         47114
>Category:       kern
>Synopsis:       it's too easy to misspell `mutex_owned' as `mutex_owner'
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Oct 23 20:10:00 +0000 2012
>Closed-Date:    Wed Apr 12 07:03:51 +0000 2023
>Last-Modified:  Wed Apr 12 07:03:51 +0000 2023
>Originator:     Taylor R Campbell <campbell+netbsd@mumble.net>
>Release:        NetBSD 6.99.14
>Organization:
>Environment:
Architecture: any
Machine: any
>Description:

	mutex_owned(m) tells whether curlwp owns m.  mutex_owner(m)
	asks for some thread that recently owned m, if any.  Kasserting
	mutex_owned is sensible; kasserting mutex_owner is not.  But we
	have several kasserts of mutex_owner:

sys/arch/mips/include/pmap.h:#define        PG_MD_PVLIST_LOCKED_P(md)       (mutex_owner((md)->pvh_lock) != 0)
sys/dev/ieee1394/fwohci.c:  KASSERT(mutex_owner(&sc->fc.fc_mtx));
sys/ufs/ufs/ufs_quota2.c:   KASSERT(mutex_owner(&dqlock));
sys/uvm/pmap/vmpagemd.h:    mutex_owner((mdpg)->mdpg_lock)

	(The first and last are not directly in kasserts, but rather in
	macros used only in kasserts.)

	These should probably all be mutex_owned, but I'm very nervous
	about touching these parts of the system which I have no way to
	test at the moment, especially since that would point blame at
	me for causing breakage if it turned out that there are locking
	issues there which changing mutex_owned to mutex_owner might
	reveal...

>How-To-Repeat:

	Advanced code analysis tools (sometimes known as `grep').

>Fix:

	Yes, please!  And maybe we should rename one or the other so
	that it is not as easy to confound the two.

>Release-Note:

>Audit-Trail:
From: coypu@SDF.ORG
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/47114: it's too easy to misspell `mutex_owned' as
 `mutex_owner'
Date: Sun, 20 Nov 2016 20:37:41 +0000

 Aside from renaming, another option may be to add it to linting, although
 I'm not sure how to do it myself/ if it's easy to do it.
 mutex owner.... == something would be valid,
 mutex_owned .... == invalid.

 mutex_owner sans == invalid,etc.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/47114 CVS commit: src/sys/ufs/ufs
Date: Sun, 20 Nov 2016 21:21:26 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sun Nov 20 21:21:26 UTC 2016

 Modified Files:
 	src/sys/ufs/ufs: ufs_quota2.c

 Log Message:
 KASSERT(mutex_owner(...)) ---> KASSERT(mutex_owned(...))

 Fixes part of PR kern/47114.  Tested by code inspection.


 To generate a diff of this commit:
 cvs rdiff -u -r1.40 -r1.41 src/sys/ufs/ufs/ufs_quota2.c

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

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/47114: it's too easy to misspell `mutex_owned' as
 `mutex_owner'
Date: Sun, 20 Nov 2016 22:30:52 +0000

 On Sun, Nov 20, 2016 at 08:40:01PM +0000, coypu@SDF.ORG wrote:
  >  Aside from renaming, another option may be to add it to linting, although
  >  I'm not sure how to do it myself/ if it's easy to do it.
  >  mutex owner.... == something would be valid,
  >  mutex_owned .... == invalid.
  >  
  >  mutex_owner sans == invalid,etc.

 mutex_owner() == anything_besides_curlwp isn't valid either, so
 mutex_owner should probably just go away.

 -- 
 David A. Holland
 dholland@netbsd.org

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/47114 CVS commit: src/sys/dev/ieee1394
Date: Sun, 20 Nov 2016 22:56:13 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sun Nov 20 22:56:13 UTC 2016

 Modified Files:
 	src/sys/dev/ieee1394: fwohci.c

 Log Message:
 KASSERT(mutex_owner(...)) ---> KASSERT(mutex_owned(...))

 If this is not correct, then there's something bogus in this code
 anyway, so better to fail early.

 Final part of PR kern/47114.


 To generate a diff of this commit:
 cvs rdiff -u -r1.139 -r1.140 src/sys/dev/ieee1394/fwohci.c

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

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/47114 CVS commit: src
Date: Wed, 12 Apr 2023 06:35:41 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Wed Apr 12 06:35:41 UTC 2023

 Modified Files:
 	src/external/cddl/osnet/dist/uts/common/fs/zfs: spa.c
 	src/sys/kern: kern_mutex.c
 	src/sys/rump/librump/rumpkern: locks.c locks_up.c
 	src/sys/sys: mutex.h

 Log Message:
 kern: Nix mutex_owner.

 There is no valid reason to use this except in assertions of the form

 	KASSERT(mutex_owner(lock) == curlwp),

 which is more obviously spelled as

 	KASSERT(mutex_owned(lock)).

 Exception: There's one horrible kludge in zfs that abuses this, which
 should be eliminated.

 XXX kernel revbump -- deleting symbol

 PR kern/47114


 To generate a diff of this commit:
 cvs rdiff -u -r1.11 -r1.12 \
     src/external/cddl/osnet/dist/uts/common/fs/zfs/spa.c
 cvs rdiff -u -r1.104 -r1.105 src/sys/kern/kern_mutex.c
 cvs rdiff -u -r1.83 -r1.84 src/sys/rump/librump/rumpkern/locks.c
 cvs rdiff -u -r1.11 -r1.12 src/sys/rump/librump/rumpkern/locks_up.c
 cvs rdiff -u -r1.26 -r1.27 src/sys/sys/mutex.h

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

State-Changed-From-To: open->closed
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Wed, 12 Apr 2023 07:03:51 +0000
State-Changed-Why:
mutex_owner has been deleted


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.47 2022/09/11 19:34:41 kim Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2023 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.