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:
(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.