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 ([EMAIL PROTECTED], [EMAIL PROTECTED])
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


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to