From nobody Wed Jan 08 21:51:31 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 4YT1pS49tsz5k3Bq for ; Wed, 08 Jan 2025 21:51:44 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-pl1-x62e.google.com (mail-pl1-x62e.google.com [IPv6:2607:f8b0:4864:20::62e]) (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 4YT1pS2301z4bZF for ; Wed, 8 Jan 2025 21:51:44 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-pl1-x62e.google.com with SMTP id d9443c01a7336-2164b662090so3287905ad.1 for ; Wed, 08 Jan 2025 13:51:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20230601.gappssmtp.com; s=20230601; t=1736373103; x=1736977903; darn=freebsd.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=DTke1ASZBzG/vUe6AxhJXhFq/d39MXIdhiS7IuWYJXw=; b=JM6Iwpuxi+SaJG3c604ykIEn+GQpb9utZodn05HnzCAQPfpF35IuLl1k31mkXiyNaS hjdLv3k7NQgeMnDhoVMHjhEUjpdUc/tvKLoQHxvwqo9yLKB7JjbvfduL3Jbj/gCV3Sn6 1/qhIWTnf4GyDml+a/lS75n/CzGGfIuWt40W98q8NvdjMab9iMi1o0ZmLzLzMVut1x9s kQsXvN8xBQYWKYP51MOF900ujub4MVn7E2g/orynTY+vS03gv5hxyvqkQ614QDDNEclc mcNfAOMfNXpAshB7Yc6B3qBy6E+6rjD06KcGwEmgL9K3LZ4ZCSuHTo4+mrDuXHtvur1+ 8Wkg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736373103; x=1736977903; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=DTke1ASZBzG/vUe6AxhJXhFq/d39MXIdhiS7IuWYJXw=; b=EpSloPrzbvfCd1gRwoCCh1UIZ2NZ5iI+d3R8FJKRReaDHjcYq94Fs6ldjubVDcVdx7 +GYdY1G1nelGzBr6jqdkcwtvbD9vlXihCAZCRY0pAxjbEpmGNdlIKV/tkVfWB/WC6MBs As8x1y3NLhuMWeaonzzF/zheqG0xEFIU/Q1oa+wvHqDtjZez2ysUnnY/YVcE/dDleqyF Jpcf6vFg4mvupjHKfbDdacXY06rD9PCdwvWheq5sW+X3ZUgW5BGFXm2+aygMDZiXY2Ok gcpoKPD2s736OZ7SvqhAA9OMCAPiAMe4g97Zzg5haEoNAAttGjm97xDHRDAPklRs0+pP haYA== X-Gm-Message-State: AOJu0Yx9+oNLW30xW111XNix8+kFxdykHyyg/KipvfVLgTpcSQtZMNS8 YPmKE+e5tIBVBRxyqLdnWhIboX500mwsYBR2AUxGks1Isk/bFYd0uJLzUWWq1FH2j+uleP6hfhc 9CtoUmQMHawXDXGHMI/RTZLaW+RNuVN0xZkJLNAZfwNslkPcK6ddFAg== X-Gm-Gg: ASbGncu6MHcjE1MObComN/KJvkR00GL1ebOQT4NEKmfXqLWpI1OImwxfLVcNTqJ7wQU ubKl//wimF+ifqesSARUQjAqHYM699G5i0FLMRQ== X-Google-Smtp-Source: AGHT+IH1/esAq2DBsM8SUEzSv2bXY5rdjfBxKfURPsG5zQiIg8r3ZFi++72f0KYLlFbyi5LO+5zam3Kmi5LMAh1k8vc= X-Received: by 2002:a05:6a00:1942:b0:725:d956:aa6f with SMTP id d2e1a72fcca58-72d21f7f5a6mr6829915b3a.5.1736373102610; Wed, 08 Jan 2025 13:51:42 -0800 (PST) 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 References: In-Reply-To: From: Warner Losh Date: Wed, 8 Jan 2025 14:51:31 -0700 X-Gm-Features: AbW1kvZbIYwqEyCdfqkbkyZXye9Ek2kobRSu0YMM9x8gi-142cGHoR4vJzhW8iQ Message-ID: Subject: Re: widening ticks To: Mark Johnston Cc: freebsd-hackers@freebsd.org Content-Type: multipart/alternative; boundary="000000000000faa563062b38ddd8" X-Rspamd-Queue-Id: 4YT1pS2301z4bZF 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] --000000000000faa563062b38ddd8 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Jan 8, 2025 at 2:31=E2=80=AFPM Mark Johnston wr= ote: > 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? > So solution (1) is MFC-able, I think, so I like it. (2) Isn't, but is likely a better long-term solution. (3) is a non-starter since ticks is too common a name to #define. I could easily see a situation where we do (1) and then convert all current users of ticks to be ticks64. This could proceed one at a time with as much haste or caution as we need. Once we convert all of them over, we could delete ticks and then there'd be no extra work in hardclock. This too would be MFC-able. sys/net/iflib.c: uint64_t this_tick =3D ticks; sys/netinet/tcp_subr.c: < (u_int)ticks))) { look fun! We also shadow it in a lot of places. The TCP stack uses it a lot with a bunch of different variables, struct entries, etc, including RACK and BBR. The 802.11 stack uses it a bunch. As to a bunch of drivers, sometimes shadowing other times not. It would be a lot to audit all this, so I think having the new API in place might be better, and incrementally converting / removing the shadowing (even if it isn't completely in scoe, using ticks as a local variable is begging for trouble)= . Warner Also I see both jiffies and jiffies_64 defined. Does that matter at all? --000000000000faa563062b38ddd8 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Wed, Jan 8, = 2025 at 2:31=E2=80=AFPM Mark Johnston <markj@freebsd.org> wrote:
The global "ticks" variable counts hardclock= ticks, it's widely used in
the kernel for low-precision timekeeping.=C2=A0 The linuxkpi provides a ver= y
similar variable, "jiffies", but there's an incompatibility: = the former
is a signed int and the latter is an unsigned long.=C2=A0 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.=C2=A0 I=
can see a few approaches:
- Define a 64-bit ticks variable, say ticks64, and make hardclock()
=C2=A0 update both ticks and ticks64.=C2=A0 Then #define jiffies ticks64 on= 64-bit
=C2=A0 platforms.=C2=A0 This is the simplest to implement, but it adds extr= a work
=C2=A0 to hardclock() and is somewhat ugly.
- Make ticks an int64_t or a long and convert our native code
=C2=A0 accordingly.=C2=A0 This is cleaner but requires a lot of auditing to= avoid
=C2=A0 introducing bugs, though perhaps some code could be left unmodified,=
=C2=A0 implicitly truncating the value to an int.=C2=A0 For example I think=
=C2=A0 sched_pctcpu_update() is fine.=C2=A0 I've gotten an amd64 kernel= to compile
=C2=A0 and boot with this change, but it's hard to be confident in it.= =C2=A0 This
=C2=A0 approach also has the potential downside of bloating structures that=
=C2=A0 store a ticks value, and it can't be MFCed.
- Introduce a 64-bit ticks variable, ticks64, and
=C2=A0 #define ticks ((int)ticks64).=C2=A0 This requires renaming any struc= t
=C2=A0 fields and local vars named "ticks", of which there's = a decent number,
=C2=A0 but that can be done fairly mechanically.

Is there another solution which avoids these pitfalls?=C2=A0 If not, should=
we go ahead with one of these approaches?=C2=A0 If so, which one?

So solution (1) is MFC-able, I think, so I like = it.
(2) Isn't, but is likely a better long-term solution.
(3) is a non-starter since ticks is too common a name to #define.

I could easily see a situation where we do (1) and t= hen convert all current
users of ticks to be ticks64. This could = proceed one at a time with as much
haste or caution as we need. O= nce we convert all of them over, we could
delete ticks and then t= here'd be no extra work in hardclock. This too would
be MFC-a= ble.

sys/net/iflib.c: uint64_t this_tick =3D ticks= ;
sys/netinet/tcp_subr.c: =C2=A0 =C2=A0 =C2=A0 =C2=A0 < (u_int= )ticks))) {

look fun! We also shadow it in a lot o= f places. The TCP stack uses it a lot
with a bunch of different v= ariables, struct entries, etc, including RACK and BBR.
The 802.11= stack uses it a bunch.=C2=A0 As to a bunch of drivers, sometimes shadowing=
other times not.

It would be a lot to a= udit all this, so I think having the new API in place might be
be= tter, and incrementally converting / removing the shadowing (even if it isn= 't
completely in scoe, using ticks as a local variable is beg= ging for trouble).

Warner

Also I see both jiffies and jiffies_64 defined. Does that matter at all?

--000000000000faa563062b38ddd8--