Date: Sat, 18 Jan 2003 14:22:15 -0700 From: Scott Long <scott_long@btc.adaptec.com> To: Joe Marcus Clarke <marcus@marcuscom.com> Cc: Hiten Pandya <hiten@unixdaemons.com>, Craig Rodrigues <rodrigc@attbi.com>, Will Andrews <will@csociety.org>, freebsd-current@freebsd.org, dillon@freebsd.org Subject: Re: VM_METER no longer defined? Message-ID: <3E29C587.9030409@btc.adaptec.com> In-Reply-To: <1042923720.7820.25.camel@shumai.marcuscom.com> References: <1042923720.7820.25.camel@shumai.marcuscom.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Joe Marcus Clarke wrote:
> On Sat, 2003-01-18 at 15:20, Scott Long wrote:
>
> >Hiten Pandya wrote:
> >
> >
> >>On Fri, Jan 17, 2003 at 04:39:42PM -0700, Scott Long wrote the words
>
>
> >>in effect of:
> >>
> >>
> >>>Craig Rodrigues wrote:
> >>>
> >>>
> >>>
> >>>>On Fri, Jan 17, 2003 at 10:26:10AM -0800, Will Andrews wrote:
> >>>>
> >>>>
> >>>>
> >>>>>Of course, these things can be fixed. But I consider this change
> >>>>>gratuitous and it breaks standard compatability rules: deprecate
> >>>>>for one major version and remove in the second. I haven't seen
> >>>>>any reason why this couldn't be added to vm/vm_param.h:
> >>>>>
> >>>>>#define VM_METER VM_TOTAL
> >>>>>
> >>>>>for compatability purposes. This change is way too sudden in an
> >>>>>external API (if it's supposed to be internal, then protect it
> >>>>>with an #ifdef _KERNEL already!).
> >>>>
> >>>>
> >>>>How about this then:
> >>>>
> >>>>
> >>>>Index: vm_param.h
> >>
> >>===================================================================
> >>
> >>>>RCS file: /home/ncvs/src/sys/vm/vm_param.h,v
> >>>>retrieving revision 1.16
> >>>>diff -u -r1.16 vm_param.h
> >>>>--- vm_param.h 2003/01/11 07:29:46 1.16
> >>>>+++ vm_param.h 2003/01/17 23:25:52
> >>>>@@ -89,6 +89,8 @@
> >>>>#define VM_SWAPPING_ENABLED 11 /* swapping enabled */
> >>>>#define VM_MAXID 12 /* number of valid vm
>
> ids */
>
> >>>>+#define VM_METER VM_TOTAL /* backwards compatibility, struct
>
> vmmeter
>
> >>>>*/
> >>>>+
> >>>>#define CTL_VM_NAMES { \
> >>>> { 0, 0 }, \
> >>>> { "vmtotal", CTLTYPE_STRUCT }, \
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>The only place where VM_METER is used in this directory was in
> >>
> >>vm_meter.c:
> >>
> >>>> 240 SYSCTL_PROC(_vm, VM_METER, vmmeter,
>
> CTLTYPE_OPAQUE|CTLFLAG_RD,
>
> >>>> 241 0, sizeof(struct vmtotal), vmtotal, "S,vmtotal",
> >>>> 242 "System virtual memory statistics");
> >>>>
> >>>>This changed to:
> >>>>
> >>>> 240 SYSCTL_PROC(_vm, VM_TOTAL, vmtotal,
>
> CTLTYPE_OPAQUE|CTLFLAG_RD,
>
> >>>> 241 0, sizeof(struct vmtotal), vmtotal, "S,vmtotal",
> >>>> 242 "System virtual memory statistics");
> >>>>
> >>>>
> >>>>
> >>>
> >>>This is ugly and only further perpetuates what appears to be a
> >>>gratuitous API
> >>>change. Let's wait to hear from the submitter (Hiten) and
>
> committer
>
> >>>(Matt) to
> >>>see why this was needed in the first place.
> >>>
> >>>Hiten? Matt?
> >>
> >>
> >>The change was made, because VM_METER was a bogus name for what it
>
> did.
>
> >>It returned struct vmtotal, but we named it VM_METER. Infact, I
>
> tried
>
> >>to push this change some long time ago, but there were complications
> >>(people busy etc...).
> >>
> >>I think applicatins to should be changed to use VM_TOTAL, instead of
> >>VM_METER, because that's the correct name. This is the same issue
>
> with
>
> >>the KMEM_METER define, which will be resolved once I get around to
>
> it.
>
> >>I sent this change to Matt first, to check if it was right, since he
>
> is
>
> >>the VM guru and whatnot. Also, the change was made quite a while
>
> ago.
>
> >>Before we entered the freeze, IIRC. IMHO, backing it out will just
>
> make
>
> >>more and more apps use it, and it will be totally sad -- but hey, I
>
> am
>
> >>not Release Engineer, so final decision is up to you.
> >>
> >>Cheers.
> >>
> >>P.S. Apologies for taking long to reply, I was out party-ing. :^)
> >>
> >>--
> >>Hiten Pandya (hiten@unixdaemons.com, hiten@uk.FreeBSD.org)
> >>http://www.unixdaemons.com/~hiten/
> >
> >
> >Ok, I agree that the naming could cause confusion since there is a
> >vmmeter struct and a vmtotal struct. However, the Release Engineering
> >policy that was set out at the start of RELENG_5_0 is that public API
> >changes need review. I'm not saying that those reviews won't be
> >approved, but we want to keep the pain involved in 5.0->5.1 as low
> >as possible. This is causing pain with several high-profile ports.
> >
> >In my opinion, the inconsistency in VM_METER was annoying, but
> >not enough to justify breaking an interface that has been existence
> >since _1994_. Dude, that is _9_ years. If you'd like the name to
> >change, lets hold of for RELENG_5 to happen. Please back out the
> >name change.
>
>
> Scott, the API change does not exist in RELENG_5_0. This change only
> went into HEAD.
>
> Joe
>
>
> >Scott
I'm fully aware of that. As I state in the previous email, I'd like to see
5.0->5.1 happen as smoothly as possible. For background, go re-read
my email to developers@ when the RELENG_5_0 branch happened.
Also, it's common practice to deprecate public interfaces for a period
of time
before removing them. Hiten's change doesn't do that either.
Scott
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3E29C587.9030409>