From nobody Thu Jan 19 16:49:12 2023 X-Original-To: dev-commits-src-main@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 4NyT9k3K3vz2v68g; Thu, 19 Jan 2023 16:49:14 +0000 (UTC) (envelope-from mhorne@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 4NyT9k2q7xz3rhm; Thu, 19 Jan 2023 16:49:14 +0000 (UTC) (envelope-from mhorne@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1674146954; 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=9It0UWgx15zW1E2apS7Ke/XSQeYt8uCUU9/OsyoO33I=; b=ZJwzY4fZd/dl150fvX4jUrAJkNsCHcCV4QW5V7FVIIH4F9BMuAWfg+zs/BmT8bgvnCnNVp /FniRK+wyETe+vI5ZqBKcXjlapOR2V12T4ZCHLuQIxSGmYuJo0dHnurBffBe1mT3WWvDQl nG1Vs0cAwnVISE6o1a3oTE3QPf+XtKchsZwUDthS8J4h2fDeF/Br8gQ2uwUqT5Vq+AIPKY FsqzQc3wMbJnto4NiKMXxjSMC/OeZ+EvX0VgRyCDxE2QlgVtr/tdasHptD3wMJwlBH+SFy aEHRS0p44GkOvHXplSJISA457VtyMTKv0xD6FurHZ8cVZqpJp9yru4hePiA62A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1674146954; 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=9It0UWgx15zW1E2apS7Ke/XSQeYt8uCUU9/OsyoO33I=; b=C0jRVhcAjuQlqIOgLrWPFn2zgXSIr0Fdc3w0TubyoX31doJr2mYYbyjXWFOaMsNkRFGomc eYzCbys2L3AocSOq5VvvpdOPqZxAjtJMiyNJ+BQM0R9KWepqI1cw4p8I/X0hAsLk4N7VFv 3zsHAbeVhTksFp9Mq/9vm3MIZUgiujaH+JhUkB4YGYLSWeTVhLPUJ4UBLMfd75bIfFMLlS /oAX1iTxP3GFBPBr990v6Lo1hBARcMhcAawljTWm+kFh/wSdK/YzS+LTd8CL8zmQauP8NP MyFtuxemteizYkRuPKxVsZM8hx4Ha7PneLwiyMhW+/XwCIWlJ92cOxMLccmb/w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1674146954; a=rsa-sha256; cv=none; b=vxCfDkvVmp+v9q/+tVDGEX1t+TxrQ1rdsdJSz+DOC+eSJPPXB7W8F9Zasqb/LeCfe5eJUf h1bz+BREWBL0eCUjQJk/hwXFZi2VZk5tW8E8fOYeUIJyOyP5mfA6ot2bahhu45chNdueMf eKRGN0aOFT/b+X+nMeZvX1Mbj+Dul8Lr5TYph6WJlz9bbcnKd4Cof4VNN74k9Q5xt6uPZJ uCfusdGSfw3mtPFJFDDfCoJ/nGOE84SWJZoZypYq3iPOxtmemEbrblu+0PB0fT3hhZ5aI/ s8o84KWp7uGriellZWtOgHNmGVtcXHW01rJI/BsRLMKpNSQS96zcz9jFWZl27Q== Received: from [192.168.1.151] (host-173-212-76-127.public.eastlink.ca [173.212.76.127]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: mhorne) by smtp.freebsd.org (Postfix) with ESMTPSA id 4NyT9j6p3gznk8; Thu, 19 Jan 2023 16:49:13 +0000 (UTC) (envelope-from mhorne@freebsd.org) Message-ID: Date: Thu, 19 Jan 2023 12:49:12 -0400 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: git: b75062f23431 - main - riscv: Fix thread0.td_kstack_pages init To: Brooks Davis , Warner Losh Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org References: <202301171638.30HGcP3C091184@gitrepo.freebsd.org> Content-Language: en-CA From: Mitchell Horne In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ThisMailContainsUnwantedMimeParts: N On 1/18/23 20:57, Brooks Davis wrote: > On Wed, Jan 18, 2023 at 04:53:51PM -0700, Warner Losh wrote: >> On Wed, Jan 18, 2023 at 3:07 PM Mitchell Horne wrote: >> >>> >>> >>> On 1/17/23 12:38, Brooks Davis wrote: >>>> The branch main has been updated by brooks: >>>> >>>> URL: >>> https://cgit.FreeBSD.org/src/commit/?id=b75062f23431fbabef1e7d665cae270b144f71b1 >>>> >>>> commit b75062f23431fbabef1e7d665cae270b144f71b1 >>>> Author: Brooks Davis >>>> AuthorDate: 2023-01-17 16:36:15 +0000 >>>> Commit: Brooks Davis >>>> CommitDate: 2023-01-17 16:37:42 +0000 >>>> >>>> riscv: Fix thread0.td_kstack_pages init >>>> >>>> Commit 0ef3ca7ae37c70e9dc83475dc2e68e98e1c2a418 initialized >>>> thread0.td_kstack_pages to KSTACK_PAGES. Due to the lack of an >>>> include of opt_kstack_pages.h it used the fallback value of 4 from >>>> machine/param.h. >>> >>> Does this mean that we could/should include opt_kstack_pages.h within >>> machine/param.h (under #ifdef _KERNEL)? This header is both a consumer >>> and provider of the KSTACK_PAGES definition, by virtue of the #ifndef. I >>> think the hidden dependency should be avoided, if possible. >>> >> >> No. Including opt_XXXX.h is never OK in our .h files. They are used in too >> many places, some of which "cheat" and define _KERNEL becuse, well, they >> need to get to the kernel bits.... That will break... > Riiiiiight, I was forgetting the _KERNEL liars always ruin the fun. You are right, and the "never include opt_ headers in a header" rule is a good one. > We could potentially use the __has_include extension. I don't think we > care about building the kernel with a compiler that isn't clang or gcc > and the usage pattern defined by gcc is safe for compilers that don't > define it. We could do something like: > > #ifdef _KERNEL > #ifndef KSTACK_PAGES > #ifdef __has_include > #if __has_include("opt_kstack_pages.h") > #include "opt_kstack_pages.h" > #endif > #endif > #endif > #endif > > Yeah this would work, but I think we can agree it's one step more complicated than necessary for this edge-case which is unlikely to show up again often, if at all. I say let's just add a comment to each machine/param.h explaining the situation and that will be enough. Mitchell >> >> I do agree, however, that the current interface is less than ideal... >> >> >>> Of course, the problem at hand has been fixed and we want to keep direct >>> consumers of KSTACK_PAGES to a minimum, but I think the point still stands. >>> >> >> I think it's a good point, but the current way is likely the least-bad way >> to accomplish things. >> >> It would be much better if we could remove it from machine/param.h and >> opt_XXX.h always defines it, even the default value when it's not otherwise >> specified. However, we don't (currently) have a way to set default values >> in config(8). We could add it, since the efforts at config++ have thus far >> fallen flat.... > > I think this is probably the better direction to move. There aren't any > in-tree uses of KSTACK_PAGES so removing the definition should be fine. > > -- Brooks