Date: Tue, 15 Jan 2013 14:33:25 -0800 From: Alfred Perlstein <bright@mu.org> To: John Baldwin <jhb@freebsd.org> Cc: svn-src-projects@freebsd.org, Alfred Perlstein <alfred@freebsd.org>, src-committers@freebsd.org Subject: Re: svn commit: r245259 - projects/utrace2 Message-ID: <50F5D935.2010309@mu.org> In-Reply-To: <201301151443.25121.jhb@freebsd.org> References: <201301101758.r0AHw6m7078896@svn.freebsd.org> <201301141532.53327.jhb@freebsd.org> <50F47A0B.9070401@mu.org> <201301151443.25121.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 1/15/13 11:43 AM, John Baldwin wrote:
> On Monday, January 14, 2013 4:35:07 pm Alfred Perlstein wrote:
>> On 1/14/13 3:32 PM, John Baldwin wrote:
>>> On Monday, January 14, 2013 2:58:23 pm Alfred Perlstein wrote:
>>>> On 1/14/13 2:01 PM, John Baldwin wrote:
>>>>> On Monday, January 14, 2013 1:44:28 pm Alfred Perlstein wrote:
>>>>>> I think we are basically in agreement, however we differ on the following two points, whereas now I think we only differ on a single point.
>>>>>>
>>>>>> 1) belief that a 4 character string signature is superior to a protocol/version tuple.
>>>>>>
>>>>>> After looking at the code and thinking about this quite a bit, I agree with you that string based namespace is nicer, however I think we
>>> need the
>>>>> following changes:
>>>>>> a) define the system namespace to have "_" preceding the trace name. so RTLD -> _RTL
>>>>>> b) or maybe we need another few characters? 6 or 8 so that it can still be nice. so "_RTL" -> "_RTLD\0\0\0", "_MALLOC\0"
>>>>>> c) we add a version field after the character string.
>>>>>> d) we create a mechanism for requesting a utrace allocation namespace somewhere (/usr/share/utrace/allocations.txt) where vendors can
>>> allocate
>>>>> strings.
>>>>>> 2) you believe that filtering this all through utrace(2) is OK. I would prefer that we leave utrace(2) alone and move forward with
> utrace2(2)
>>> to
>>>>> leave behind all the unformatted data we used to have. I would like to leave utrace(2) in the system and add utrace2(2) for new consumers.
>>>>>> What do you think?
>>>>>>
>>>>>> My end goal is to make this something that more users can grab and use for a quick and handy debug tool and to actually build on this
> somewhat,
>>>>> (libutrace) what we have now (unstructured globs of whatever) does not work.
>>>>>
>>>>> I disagree with this last assertion. On what basis do you claim that what we
>>>>> have now does not work? Do you have any specific examples besides
>>>>> hypothetical cases? I fail to see how utrace() in its current form is not
>>>>> already useful, and I've yet to see a convincing argument from you that it is
>>>>> not.
>>>>>
>>>> #include <stdio.h>
>>>> #include <stdlib.h>
>>>>
>>>> int
>>>> main(void)
>>>> {
>>>> void *ptr = 0x52544c44;
>>>>
>>>> realloc(ptr, 200);
>>>> }
>>> That is fair, though you could easily fix that by changing malloc to use a
>>> signature going forward. :) That is a far simpler change than adding an
>>> entirely new system call. You also ignored the point that by making an API
>>> change you _are_ forcing all the current hypothetical utrace() users you are
>>> so worried about to make a code change.
>> Well, wouldn't the hypothetical users just continue to use the old code
>> until they are ready to move forward? Having both utrace and utrace2
>> would not impact force anyone to move.
> Presumably if you really want to deprecate it you need to remove it from the
> header so it is truly hidden? (At least in 11 if not in 10?)
I don't really want to deprecate it. I just want to provide a new
subsystem that's more regular.
I only offered to deprecate it because I thought you were hinting at that.
I'd rather keep both old and new. I do NOT want to break people.
>
>>> There is one thing, however, that would be nice about forcing all user data to
>>> be tagged with an explicit type which is to be able to have kdump filter user
>>> requests (so I could choose to only see RTLD events even if malloc events were
>>> logged).
>> Yes, very much so!
>>
>>> The 'utrace2' name is horrible. I'm not sure I have a better one off hand,
>>> though if we claim that utrace is not an important API to preserve we could
>>> just call the new one 'utrace' and call the old one 'freebsd9_utrace'. In
>>> userland you could have utrace@FBSD_1.0 call freebsd9_utrace() and the new
>>> utrace() call the new system call. That would preserve the ABI, but force
>>> users to update to the new API. This is similar to what we do when changing
>>> ABIs (see freebsd6_lseek() for an exampe). The difference in those cases is
>>> that the API doesn't change, but we still intended to obsolete
>>> freebsd6_lseek() and have it not used for future binaries, etc.
>>>
>> I don't see how this would work, they are not API compatible.
> Yes, but that comes back to "if we claim that utrace is not an important API
> to preserve" then we can just break the API and force anyone compiling on 10
> to adopt the new API. Take kdb_enter() growing a new argument in 9 (or 8?).
> Because it's a bit of an internal API we felt it was ok to change in a major
> release. I still think there are only a tiny number of utrace() users and
> that if they have found utrace() and figured out how to use it they are more
> than capable of adapting to the new API in 10. I wouldn't make this case for
> a commonly used routine or POSIX API, etc., but I think it does apply to
> utrace(). Does that make sense?
>
It does make sense, however it's not been my experience. I usually wind
up cussing up a storm each time I get bitten by such a change.
Heck, want to see something funny? Go look at ddb manpage and look how
"call doadump" doesn't really work anymore but the docs reference it in
so many places and so many mailing lists.
I'd much rather just leave old utrace(2) alone and add utrace2(2) and be
done with it.
-Alfred
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50F5D935.2010309>