From nobody Tue Jan 31 18:38:15 2023 X-Original-To: dev-commits-src-all@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 4P5v2024GPz3bxy5; Tue, 31 Jan 2023 18:38:16 +0000 (UTC) (envelope-from glebius@freebsd.org) Received: from glebi.us (glebi.us [162.251.186.162]) by mx1.freebsd.org (Postfix) with ESMTP id 4P5v2013T2z4Jwf; Tue, 31 Jan 2023 18:38:16 +0000 (UTC) (envelope-from glebius@freebsd.org) Authentication-Results: mx1.freebsd.org; none Received: by glebi.us (Postfix, from userid 1000) id 736C3560C4; Tue, 31 Jan 2023 10:38:15 -0800 (PST) Date: Tue, 31 Jan 2023 10:38:15 -0800 From: Gleb Smirnoff To: John Baldwin Cc: Konstantin Belousov , Brooks Davis , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: fa1d803c0f65 - main - epoch: replace hand coded assertion Message-ID: References: <202301201805.30KI5Ht0099187@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 4P5v2013T2z4Jwf X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:27348, ipnet:162.251.186.0/24, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N On Tue, Jan 31, 2023 at 10:30:49AM -0800, John Baldwin wrote: J> >> @@ -420,7 +420,8 @@ _epoch_enter_preempt(epoch_t epoch, epoch_tracker_t et EPOCH_FILE_LINE) J> >> MPASS(cold || epoch != NULL); J> >> MPASS(epoch->e_flags & EPOCH_PREEMPT); J> >> td = curthread; J> >> - MPASS((vm_offset_t)et >= td->td_kstack && J> >> + MPASS((vm_offset_t)et + sizeof(struct epoch_tracker) >= J> >> + (vm_offset_t)__builtin_frame_address(0) && J> >> (vm_offset_t)et + sizeof(struct epoch_tracker) <= J> >> td->td_kstack + td->td_kstack_pages * PAGE_SIZE); J> >> J> >> What do you guys think on legitimacy of the improved assertion that uses J> >> compiler built to get the current top of stack and makes the assertion even J> >> stricter? J> >> J> >> I think in FreeBSD we should at least add the MPASS(kstack_contains(td, ...) J> >> assertion to the epoch exit. J> > J> > The __builtin_frame_address() is somewhat weird thing, e.g. it assumes J> > that the local function frame is contiguous, and static. Also I do not J> > see a guarantee that any local variable address is higher that the value J> > returned from the builtin. J> J> Though in these cases the pointer value (et) is not a local variable on J> this frame, but should be in an earlier frame (usually the callers). Yes, this is exactly what my patch does. Limit the presence of epoch_tracker to the already used part of the stack. We've been running with this patch for couple years. -- Gleb Smirnoff