Date: Sun, 8 Aug 2010 16:15:03 -0700 From: Garrett Cooper <gcooper@FreeBSD.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: des@des.no, emaste@freebsd.org, ivoras@freebsd.org, freebsd-hackers@freebsd.org Subject: Re: Why is TUNABLE_INT discouraged? Message-ID: <AANLkTi=AEXiM-VWqihUMOTU%2BHo0nbPFzsNBOO=b553n6@mail.gmail.com> In-Reply-To: <20100808.121450.752311254854610266.imp@bsdimp.com> References: <864of680wv.fsf@ds4.des.no> <AANLkTinraF50O%2Bcp_h1m6TODnoz_7R3WXfjTanh-86mn@mail.gmail.com> <20100808130624.GB40928@sandvine.com> <20100808.121450.752311254854610266.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--0016e6deddeae50ad7048d5811f8
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
On Sun, Aug 8, 2010 at 11:14 AM, M. Warner Losh <imp@bsdimp.com> wrote:
> In message: <20100808130624.GB40928@sandvine.com>
> =A0 =A0 =A0 =A0 =A0 =A0Ed Maste <emaste@FreeBSD.org> writes:
> : On Sun, Aug 08, 2010 at 01:30:19AM +0200, Ivan Voras wrote:
> :
> : > 2010/8/8 Dag-Erling Sm??rgrav <des@des.no>:
> : > > Garrett Cooper <gcooper@FreeBSD.org> writes:
> : > >> Dag-Erling Sm??rgrav <des@des.no> writes:
> : > >> > Perhaps. ??I don't remember all the details; I can't find a disc=
ussion in
> : > >> > the list archives (other than me announcing the change in respon=
se to a
> : > >> > bug report), but there must have been one, either on IRC or in K=
arlsruhe.
> : > >> > In any case, I never removed TUNABLE_INT(), so...
> : > >> It does matter for integers on 64-bit vs 32-bit architectures thou=
gh,
> : > >> right
> : > >
> : > > Not sure what you mean. ??The original issue was that someone had u=
sed
> : > > TUNABLE_INT() for something that was actually a memory address. ??I
> : > > changed it to TUNABLE_ULONG(). ??Of course, if your tunable is a bo=
olean
> : > > value or something like maxprocs, an int is fine - but so is a long=
.
> : >
> : > Semantically valid but using TUNABLE_INT to hold pointers is a
> : > developer bug, not the fault of the API, and there's nothing wrong
> : > with "int" as a data type in this context.
> :
> : I agree with Ivan here. =A0If we can't find an actual reason to
> : universally prefer long I'd like to remove this comment.
> :
> : As a counterpoint to the preference for long I can offer a reason to
> : prefer int: the same variable is often exposed by both a tunable and a
> : sysctls, and a sysctl using long can have 32-bit compat issues. =A0(Tha=
t
> : is, a 32-bit app using sysctlbyname will try to use 4 bytes for a long.=
)
>
> There's absolutely no reason to not use TUNABLE_INT for simple
> integers. =A0Back in the deep, dark, dim past, there was no
> TUNABLE_*LONG. =A0TUNABLE_INT was introduce in r77900 by peter. =A0DES
> added TUNABLE_LONG in r137099, as well as adding the comment.
>
> The comment is bogus, or at least not quite specific enough. =A0It has
> been routinely ignored since it was added.
>
> There's absolutely nothing wrong with TUNABLE_INT.
>
> We really should have a TUNABLE_ADDRESS for this case, although
> there's at least three types of address that we need to worry about:
> Physical Addresses, Virtual Addresses and Bus Addresses. =A0You don't
> know if ULONG or LONG is the right type for an address, or if you need
> a quad (for example, 32-bit MIPS can address, through PTE entries,
> addresses beyond the 32-bit barrier, so you'd need a QUAD).
>
> I'm in favor of changing the comment to:
>
> /*
> =A0* Please do not use for addresses or pointers
> =A0*/
Then comes my next request: could someone please review this patch
and commit it if it's ok? This adds comments to TUNABLE_INT
(recommended by Warner) and also adds functionality for TUNABLE_UINT
as well. I have a simple module and test script attached for it.
Adding this functionality would make things more consistent with
the TUNABLE KPIs, make my experimenting with sound(4) easier, and
could help improve the sound(4) subsystem's code (I'll have to discuss
some things with ariff@, because I'm not sure whether or not some
quantities need to be signed or not).
Thanks!
-Garrett
--0016e6deddeae50ad7048d5811f8
Content-Type: application/octet-stream; name="uint_tunable.diff"
Content-Disposition: attachment; filename="uint_tunable.diff"
Content-Transfer-Encoding: base64
X-Attachment-Id: f_gcmibyy41
SW5kZXg6IGtlcm4va2Vybl9lbnZpcm9ubWVudC5jCj09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIGtlcm4va2Vybl9l
bnZpcm9ubWVudC5jCShyZXZpc2lvbiAyMTA4MjQpCisrKyBrZXJuL2tlcm5fZW52aXJvbm1lbnQu
Ywkod29ya2luZyBjb3B5KQpAQCAtNTgzLDYgKzU4MywxNCBAQAogfQogCiB2b2lkCit0dW5hYmxl
X3VpbnRfaW5pdCh2b2lkICpkYXRhKQoreworCXN0cnVjdCB0dW5hYmxlX3VpbnQgKmQgPSAoc3Ry
dWN0IHR1bmFibGVfdWludCAqKWRhdGE7CisKKwlUVU5BQkxFX1VJTlRfRkVUQ0goZC0+cGF0aCwg
ZC0+dmFyKTsKK30KKwordm9pZAogdHVuYWJsZV9sb25nX2luaXQodm9pZCAqZGF0YSkKIHsKIAlz
dHJ1Y3QgdHVuYWJsZV9sb25nICpkID0gKHN0cnVjdCB0dW5hYmxlX2xvbmcgKilkYXRhOwpJbmRl
eDogc3lzL2tlcm5lbC5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHN5cy9rZXJuZWwuaAkocmV2aXNpb24gMjEw
ODI0KQorKysgc3lzL2tlcm5lbC5oCSh3b3JraW5nIGNvcHkpCkBAIC0yNzUsNyArMjc1LDcgQEAK
IAogLyoKICAqIGludAotICogcGxlYXNlIGF2b2lkIHVzaW5nIGZvciBuZXcgdHVuYWJsZXMhCisg
KiBQbGVhc2UgZG8gbm90IHVzZSBmb3IgYWRkcmVzc2VzIG9yIHBvaW50ZXJzLgogICovCiBleHRl
cm4gdm9pZCB0dW5hYmxlX2ludF9pbml0KHZvaWQgKik7CiBzdHJ1Y3QgdHVuYWJsZV9pbnQgewpA
QCAtMjk0LDYgKzI5NCwyNiBAQAogI2RlZmluZQlUVU5BQkxFX0lOVF9GRVRDSChwYXRoLCB2YXIp
CWdldGVudl9pbnQoKHBhdGgpLCAodmFyKSkKIAogLyoKKyAqIHVuc2lnbmVkIGludAorICogUGxl
YXNlIGRvIG5vdCB1c2UgZm9yIGFkZHJlc3NlcyBvciBwb2ludGVycy4KKyAqLworZXh0ZXJuIHZv
aWQgdHVuYWJsZV91aW50X2luaXQodm9pZCAqKTsKK3N0cnVjdCB0dW5hYmxlX3VpbnQgeworCWNv
bnN0IGNoYXIgKnBhdGg7CisJdW5zaWduZWQgaW50ICp2YXI7Cit9OworI2RlZmluZQlUVU5BQkxF
X1VJTlQocGF0aCwgdmFyKQkJCQkJXAorCXN0YXRpYyBzdHJ1Y3QgdHVuYWJsZV91aW50IF9fQ09O
Q0FUKF9fdHVuYWJsZV91aW50XywgX19MSU5FX18pID0geyBcCisJCShwYXRoKSwJCQkJCQlcCisJ
CSh2YXIpLAkJCQkJCVwKKwl9OwkJCQkJCQlcCisJU1lTSU5JVChfX0NPTkNBVChfX1R1bmFibGVf
aW5pdF8sIF9fTElORV9fKSwJCVwKKwkgICAgU0lfU1VCX1RVTkFCTEVTLCBTSV9PUkRFUl9NSURE
TEUsIHR1bmFibGVfdWludF9pbml0LCBcCisJICAgICZfX0NPTkNBVChfX3R1bmFibGVfdWludF8s
IF9fTElORV9fKSkKKworI2RlZmluZQlUVU5BQkxFX1VJTlRfRkVUQ0gocGF0aCwgdmFyKQlnZXRl
bnZfdWludCgocGF0aCksICh2YXIpKQorCisvKgogICogbG9uZwogICovCiBleHRlcm4gdm9pZCB0
dW5hYmxlX2xvbmdfaW5pdCh2b2lkICopOwo=
--0016e6deddeae50ad7048d5811f8--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTi=AEXiM-VWqihUMOTU%2BHo0nbPFzsNBOO=b553n6>