This might seem in the weeds for a lot of folks, but the density of (currently always-expanded) ceph_assert() calls in fast paths seems really significant, to me.
Matt
On Mon, Oct 2, 2023 at 3:01 PM Casey Bodley <cbodley@xxxxxxxxxx> wrote:
Matt recently raised the issue of ceph assertions in production code,
which reminded me of Sage's 2016 pr
https://github.com/ceph/ceph/pull/9969 that added a
ceph_assert_always(). the idea was to eventually make ceph_assert()
conditional on NDEBUG to match the behavior of libc's assert(),
leaving ceph_assert_always() as the marked unconditional case
i would love to see this finally happen, but there are some potential risks:
* ceph_assert()s with side effects won't behave as expected in release
builds. assert() documents this same issue at
https://www.man7.org/linux/man-pages/man3/assert.3.html#BUGS. if we
could at least identify these cases, we can switch them to
ceph_assert_always()
* in teuthology, we test the same release builds that we ship to
users. that means teuthology won't catch the code paths that trigger
debug assertions. if those lead to crashes, they could be much harder
to debug without the assertions and backtraces
* conversely, merging pull requests after a successful teuthology run
may introduce new assertions in debug builds. it could be annoying for
developers to track down and fix new assertions after pulling the
latest main or stable release branch
* unused variable warnings in release builds where ceph_assert() was
the only reference. at least the compiler will catch all of these for
us, and [[maybe_unused]] annotations can clear them up
in general, do folks agree that this is a change worth making? if so,
what can we do to mitigate the risks?
if not, how should we handle the use of ceph_assert() vs raw assert()
in new code? should there be some guidance in CodingStyle?
as a half-measure, we might introduce a new ceph_assert_debug() as an
alternative to raw assert(), then convert some existing uses of
ceph_assert() on a case-by-case basis
--
Matt Benjamin
Red Hat, Inc.
315 West Huron Street, Suite 140A
Ann Arbor, Michigan 48103
http://www.redhat.com/en/technologies/storage
tel. 734-821-5101
fax. 734-769-8938
cel. 734-216-5309
_______________________________________________ Dev mailing list -- dev@xxxxxxx To unsubscribe send an email to dev-leave@xxxxxxx
