From nobody Tue Mar 19 00:57:28 2024 X-Original-To: hackers@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4TzCxW4Dlcz5FN09 for ; Tue, 19 Mar 2024 00:57:35 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4TzCxW3Mkrz4rf1; Tue, 19 Mar 2024 00:57:35 +0000 (UTC) (envelope-from kp@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1710809855; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jQQwF3+CQglorcr88qrJYeo3BrfMNHu30qOFviLKgvA=; b=DBeMHoN1RHCUAmmVy9IMsKEx0l0hCXHmUX6EL6g+k0IiugkO5PflFMUydNeunp7iQmVbeR ugfVihrnbjwHHgfEdsN2Sl6rHP7SRVKxLatDRjHeo4OkFtoxhfbcVqhB3uh5gJH3TXqZr3 7qiUllZZQOzv7uH6A8adMX7gKuvrCGJMmVDDw9C65buJf6g8a7IQrZFLtWd5ipdpb/TlOB yE3UbZBXeEiHlxjTlTWsdf078PGqc45L3+I27dqDBNS4qnlGgvnjJGEAHiewAUsc3gQOF7 BYGnXYJkHmUPqVK5RrLC0kFR7e5cdoNzsgb0zR9B2J0YgoWEVtkvIK7r88/ISQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1710809855; a=rsa-sha256; cv=none; b=Z+AOen4ELrojWNYwtjmx6L4lHj+USrO13yAVeED6H/G5Tfjx8M9fmFN4wTnx7pqJ9eKfEK aQo8hUaxIOQnqCP8QmDeD2iIZRatEQnNsb2clwnrGtTMCzNSKewtSgWKQ8A1Bh1StQ7j1+ Qx4XgM2NcxulPG3McAXMhk1n+3wgyVNxlCnuccyfXNceMwetA1t5DgD6HQ4AfGHpYl1h+x D1Tr9KdI0YTgrWtL3fcf8RtgFq8An1MApqPu5jTJyz+b1hvLAlq8btlqYbwqiTalysHjdn b7f35h6ZEGcDRZQVfDYCoUoa25KguyAbAHkhgVOe2N9qFuWtWZT63auOfSMIVg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1710809855; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jQQwF3+CQglorcr88qrJYeo3BrfMNHu30qOFviLKgvA=; b=flF7Kj4JThBG1El21z+g7YFqzTYvqtUbz3PW6o3AucEcfFUVMbYIK8KFHUrasftyitHmX2 BtmZKpGJJk0Qq1loSaPAGYwkIVNdjhKTWF2Xz4y30ul75q1oBORfVQVH5tO4DAg9Q+2M4p bDgy1rZWaoTMvzl6+JO6/cefzOzPxixn7HD+PzvIWlFkIOl88SuNgs9gbcVSI2g40eeBgY Qwd1yFJUr4gYjsbP1uxybtcTSvtdcmO0mcM7uW7y0WX9lK6HJLHO4/hIeQQHFNhsIWVIUJ THQyQ+9bTLKqvgcVeGvGvKf402jiJKKS9TDvqYKqkatwjX0Erk6o5aYiKsN9Ig== Received: from venus.codepro.be (venus.codepro.be [5.9.86.228]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "mx1.codepro.be", Issuer "R3" (verified OK)) (Authenticated sender: kp) by smtp.freebsd.org (Postfix) with ESMTPSA id 4TzCxW2FCCzVHp; Tue, 19 Mar 2024 00:57:35 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: by venus.codepro.be (Postfix, authenticated sender kp) id 7B6834A8D5; Tue, 19 Mar 2024 01:57:32 +0100 (CET) From: Kristof Provost To: "Bjoern A. Zeeb" Cc: hackers@freebsd.org Subject: Re: Why I dislike MPASS... Date: Tue, 19 Mar 2024 08:57:28 +0800 X-Mailer: MailMate (1.14r5937) Message-ID: In-Reply-To: <57o4rnnq-013s-3nsn-59n5-4ssn1pq81s94@yvfgf.mnoonqbm.arg> References: <57o4rnnq-013s-3nsn-59n5-4ssn1pq81s94@yvfgf.mnoonqbm.arg> List-Id: Technical discussions relating to FreeBSD List-Archive: https://lists.freebsd.org/archives/freebsd-hackers List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-hackers@freebsd.org MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="=_MailMate_FA3B4B96-0ECB-4EA5-937A-ECEE45C6FE33_=" Content-Transfer-Encoding: 8bit --=_MailMate_FA3B4B96-0ECB-4EA5-937A-ECEE45C6FE33_= Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 19 Mar 2024, at 8:46, Bjoern A. Zeeb wrote: > Hi, > > needless to say, this needed a 2nd kernel, a reproducer (when no gdb > and > no crash dump was avail). > > XXX-BZ KABOOM 45000 > 0 && 45000 <= 360 && 45000 < 1536 > panic: Assertion (key.to) > 0 && (key.to) <= w_max_used_index && > (key.to) < witness_count failed at > /usr/src/sys/kern/subr_witness.c:3118 > > This is why I dislike MPASS; if it hits you are often no wiser as to > why, especially if people added more than a single condition. > > Why do we make our life harder ourselves by not having informative > messages (I mean that is what the comment above MPASS claims)? > > I still have no idea how I get to that assertion but at least key.to > looks dodgy enough so that I can go look where-as before I really > didn't > know if I had hit a limit or which one... > Presumably you’d prefer something that’d log the actual compared values? E.g. something like `ASSERT_GT(key.to, 0); ASSERT_LTE(key.to, w_max_used_index); ASSERT_LT(key.to, witness_count);`? I vaguely recall seeing discussion of similar constructs in the ZFS code. I’d enthusiastically support adding, and encouraging the use of, macros like that. I’d object to removing or discouraging MPASS(), because I fear that will reduce the number of assertions developers add to the code, and we’d be objectively worse off in a world where the assertion was ` ` rather than `MPASS((key.to) > 0 && (key.to) <= w_max_used_index && (key.to) < witness_count);`. We should probably encourage people to avoid complex expressions in MPASS(), but I’d still much rather have the complex expression than nothing at all. Best regards, Kristof --=_MailMate_FA3B4B96-0ECB-4EA5-937A-ECEE45C6FE33_= Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

On 19 Mar 2024, at 8:46, Bjoern A. Zeeb wrote:

Hi,

needless to say, this needed a 2nd kernel, a reproducer (= when no gdb and
no crash dump was avail).

XXX-BZ KABOOM 45000 > 0 && 45000 <=3D 360 &= amp;& 45000 < 1536
panic: Assertion (key.to) > 0 && (key.to) <=3D w_max_used_i= ndex && (key.to) < witness_count failed at /usr/src/sys/kern/s= ubr_witness.c:3118

This is why I dislike MPASS; if it hits you are often no = wiser as to
why, especially if people added more than a single condition.

Why do we make our life harder ourselves by not having in= formative
messages (I mean that is what the comment above MPASS claims)?

I still have no idea how I get to that assertion but at l= east key.to
looks dodgy enough so that I can go look where-as before I really didn't
know if I had hit a limit or which one...


Presumably you=E2=80=99d prefer something that=E2=80=99d = log the actual compared values?
E.g. something like ASSERT_GT(key.to, 0); ASSERT_LTE(key.to, w_max_used_index); ASSE= RT_LT(key.to, witness_count);?
I vaguely recall seeing discussion of similar constructs in the ZFS code.=

I=E2=80=99d enthusiastically support adding, and encourag= ing the use of, macros like that.

I=E2=80=99d object to removing or discouraging MPASS(), b= ecause I fear that will reduce the number of assertions developers add to= the code, and we=E2=80=99d be objectively worse off in a world where the= assertion was rather than MPASS((key.to) > 0 && (key.to) <=3D w_max_use= d_index && (key.to) < witness_count);.

We should probably encourage people to avoid complex expr= essions in MPASS(), but I=E2=80=99d still much rather have the complex ex= pression than nothing at all.

Best regards,
Kristof

--=_MailMate_FA3B4B96-0ECB-4EA5-937A-ECEE45C6FE33_=--