From nobody Sat Jan 11 22:35:36 2025 X-Original-To: freebsd-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 4YVtdm52R6z5jvwQ for ; Sat, 11 Jan 2025 22:35:40 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-qk1-x733.google.com (mail-qk1-x733.google.com [IPv6:2607:f8b0:4864:20::733]) (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-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "WR4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4YVtdm2mnWz4SKs for ; Sat, 11 Jan 2025 22:35:40 +0000 (UTC) (envelope-from markjdb@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-qk1-x733.google.com with SMTP id af79cd13be357-7b6ea711805so406691985a.1 for ; Sat, 11 Jan 2025 14:35:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736634939; x=1737239739; darn=freebsd.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:from:to:cc:subject:date:message-id :reply-to; bh=DuhN1+n0bEvdZs6w2zz67/7rBmd3eTi7XetjHIUon0w=; b=R+YU98ERA49oBxEPachMbjO8Th4oPgbQncl/UQt9iCxtYkuNKVgcGzpY674J323Wc0 9aJD3DLGWWdcSH7k7wI6SaGBMEOv8VglSCWd+Ku2e4RFJErZ2W3vv7nMR758xbJhTBHk 8Y2H5Jj8NLuaC0sVJYHyTia8CgGf2rC++NeXVh10b29JGWQBNCGfcemmoX5p4pfOUNx5 cRhypOJTFxMg7LaN/6olZ/yqmYebW/jU+uSpolm+VV2Iz4pn9OKhYoLj+xI230nZ2H3H tNTY7Gu7g9UmP19chOPbsOW5zemUayLk3s71vO+hCh6+uj1vAJvQu05ds7q0RKWgVUOd d2OQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736634939; x=1737239739; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=DuhN1+n0bEvdZs6w2zz67/7rBmd3eTi7XetjHIUon0w=; b=mnCylZnQ5LI/OAAkJu2weq0AFEB0X29moU+DAegUnpW+fYcYE69KNrvG1ZnBF8dqQv NSC4utHrDW75Evs9RRQxxKqjyH/mMKy3dHDfAL2m7hlp8940NCcfhtqTpmDL/Wn/bcEo 145nATjXtC7pxm09cR+5GzMxJjj0tV5OnhWCyTubHZ3QEg5iUrHHZecMWvmL2A68oQqm oBDCalc41GIzTjxEcHjD/zHXOEKCa9velDhI56GudBxW5SnY4QodDLUQwQJsdDQZvLnw 6v9k5g2Im6nZsEQEBNMECE2deAoKpVJC5IZ7G3Hqe7//BZLxYNRcEAJEoTp80V9pKTaw KPHA== X-Forwarded-Encrypted: i=1; AJvYcCUNbdUmSSI1tJC+KurVusT6B4qqOMJS/DRwo8q9eoOnCQoflPds7XTXVOmW5f4tGlvtprbR6sTBAlE+aBEaqD4=@freebsd.org X-Gm-Message-State: AOJu0Yym3wk0UIiXd5wWKR4pFymanX1YvQX3Nvzx2y8vHjdJSKoETNwX e3UFsrpnLhBp6qSTKau5Vre120s+qGKFkEHc8ChxH+I3m46imeN92x2UoA== X-Gm-Gg: ASbGncsIT/Cdlz9WQEE0cw882wCbutZ6X/im1lDMvEEmlctnSY+96ueGZQkgyJf6tkO YaMnfDU8T0PPXdR9thl3wSNzDXe15qvZjXZH4040dCWLFo9hrrDs5hK7LaqGWCG+YTyu7h+IVEu EhhII0fEY6M0nKBF56dI5g4o+TLpVyz7bNGn1cZijWrWTInFusV8UlowVWWdHNtSA3hFZfdy5PT EXJGT92WfQjvnIKQ4i/1MO/+GftZ14mLRGpd7Il+Sxmi3rs2ux94ywJTEPxZY+/JhzMcyI= X-Google-Smtp-Source: AGHT+IG95Tqas1k5YDfriekZNn4d4uiIPiq1raff+o0QzdG0rUNkBMTC/DIG8CX8tw4hxkoBgs5Ngw== X-Received: by 2002:a05:620a:2951:b0:7b6:67df:51e3 with SMTP id af79cd13be357-7bcd973892amr2365429785a.16.1736634939396; Sat, 11 Jan 2025 14:35:39 -0800 (PST) Received: from nuc (192-0-220-237.cpe.teksavvy.com. [192.0.220.237]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7bce324828esm323535085a.45.2025.01.11.14.35.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 11 Jan 2025 14:35:39 -0800 (PST) Date: Sat, 11 Jan 2025 17:35:36 -0500 From: Mark Johnston To: Tomoaki AOKI Cc: Konstantin Belousov , freebsd-hackers@freebsd.org Subject: Re: widening ticks Message-ID: References: <20250111131106.4d2657de20eeed7eef5c0b15@dec.sakura.ne.jp> <20250112043543.86b303419f954b2b287d39d1@dec.sakura.ne.jp> 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: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250112043543.86b303419f954b2b287d39d1@dec.sakura.ne.jp> X-Rspamd-Queue-Id: 4YVtdm2mnWz4SKs X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US] On Sun, Jan 12, 2025 at 04:35:43AM +0900, Tomoaki AOKI wrote: > On Sat, 11 Jan 2025 11:34:06 -0500 > Mark Johnston wrote: > > > On Sat, Jan 11, 2025 at 01:11:06PM +0900, Tomoaki AOKI wrote: > > > On Wed, 8 Jan 2025 18:07:47 -0500 > > > Mark Johnston wrote: > > > > > > > On Thu, Jan 09, 2025 at 12:18:48AM +0200, Konstantin Belousov wrote: > > > > > On Wed, Jan 08, 2025 at 04:31:16PM -0500, Mark Johnston wrote: > > > > > > The global "ticks" variable counts hardclock ticks, it's widely used in > > > > > > the kernel for low-precision timekeeping. The linuxkpi provides a very > > > > > > similar variable, "jiffies", but there's an incompatibility: the former > > > > > > is a signed int and the latter is an unsigned long. It's not > > > > > > particularly easy to paper over this difference, which has been > > > > > > responsible for some nasty bugs, and modifying drivers to store the > > > > > > jiffies value in a signed int is error-prone and a maintenance burden > > > > > > that the linuxkpi is supposed to avoid. > > > > > > > > > > > > It would be nice to provide a compatible implementation of jiffies. I > > > > > > can see a few approaches: > > > > > > - Define a 64-bit ticks variable, say ticks64, and make hardclock() > > > > > > update both ticks and ticks64. Then #define jiffies ticks64 on 64-bit > > > > > > platforms. This is the simplest to implement, but it adds extra work > > > > > > to hardclock() and is somewhat ugly. > > > > > > - Make ticks an int64_t or a long and convert our native code > > > > > > accordingly. This is cleaner but requires a lot of auditing to avoid > > > > > > introducing bugs, though perhaps some code could be left unmodified, > > > > > > implicitly truncating the value to an int. For example I think > > > > > > sched_pctcpu_update() is fine. I've gotten an amd64 kernel to compile > > > > > > and boot with this change, but it's hard to be confident in it. This > > > > > > approach also has the potential downside of bloating structures that > > > > > > store a ticks value, and it can't be MFCed. > > > > > > - Introduce a 64-bit ticks variable, ticks64, and > > > > > > #define ticks ((int)ticks64). This requires renaming any struct > > > > > > fields and local vars named "ticks", of which there's a decent number, > > > > > > but that can be done fairly mechanically. > > > > > > > > > > > > Is there another solution which avoids these pitfalls? If not, should > > > > > > we go ahead with one of these approaches? If so, which one? > > > > > > > > > > You cannot do this in C, but can in asm: > > > > > .data > > > > > .globl ticksl, ticks > > > > > .type ticksl, @object > > > > > .type ticks, @object > > > > > ticksl: .quad > > > > > .size ticksl, 8 > > > > > ticks =ticksl /* for little-endian */ > > > > > /* ticks =ticksl + 4 for big-endian */ > > > > > .size ticks, 4 > > > > > > > > > > > > > > > Then update only ticksl in the hardclock(). > > > > > > > > I implemented your suggestion here: https://reviews.freebsd.org/D48383 > > > > > > As this is already committed to main, commenting here instead of review > > > D48383. > > > > > > Maybe I'm too paranoid and overlooking something, but... > > > > > > *If "jiffies" in LinuxKPI is really unsigned, isn't there any > > > possibilities that relies on its value to be larger than > > > 0x7fffffffffffffff as a threshold? > > > (Yes, it should be silly and non-realistic, but theoretically > > > possible.) > > > > Ideally we would have > > > > #define jiffies ((unsigned long)ticksl) > > > > in the linuxkpi, but some Linux code uses "jiffies" as a struct field or > > local variable name, so this doesn't quite work. > > > > In practice, the value is usually assigned to an unsigned long or used > > as an operand where it would be implicitly promoted to an unsigned type, > > so we don't see any incompatibilities. > > > > When jiffies is an int, code like the following can misbehave: > > > > unsigned long remain, timeout = jiffies + const; > > ... > > remain = timeout - jiffies; > > if ((long)remain < 0) > > /* timed out */ > > > > If (int)timeout and jiffies have different signs, as might happen close > > to a rollover, the comparison won't work as expected. > > > > Linux has some macros (time_after() etc.) which are supposed to be used > > instead of direct comparisons, but they're not always used. > > So ticksl should better be unsigned long if there's no reason to keep > it signed, isn't it? Well, I kept it signed since it's meant to be similar in usage to ticks. With a signed counter, you can check test whether a value has passed by looking at the sign of the difference between ticks(l) and that value (modulo rollover). With an unsigned counter, you need some casting, as in the example above. > > > *Is anywhere checking carry (sign) bit for int on LP32? > > > Maybe it would be the reason if "jiffies" in LinuxKPI is really > > > unsigned. > > > > Could you provide an example of what you mean? > > Not an example of code, but for example, when ticksl is at > 0x7fffffffffffffff (positive value), ticks shoule be 0xffffffff > (negative value), if I read the diff correctly. > The same thing starts happening ticksl is at 0x0000000080000000 throug > 0x00000000ffffffff and values alike. So signs (carry bits, usually the > leftmost bit of each) should be checked separately for ticksl and ticks. That's true, but I can't see why any code would care about this? > Am I (hopefully) overlooking something? > > -- > Tomoaki AOKI