[PATCH 0 of 3] Add support for dtrace compatible sdt probes on GNU/Linux

[serguei.spitsyn at oracle.com]

Keith,

Thank you for reviewing at the fix!

I forgot to mention another AI:
   5. Submit a CCC request and get an approval

I'll do this as a background work.
Mark, are you Ok with that?


Thanks,
Serguei

On 5/23/12 10:49 AM, Keith McGuigan wrote:
>
> Serguei -
>
> I think that all sounds fine.
>
> -- 
> - Keith
>
> On 5/23/2012 1:25 PM, serguei.spitsyn at oracle.com wrote:
>> Keith,
>>
>> I agree that it'd be nice to follow the rules.
>> But we also can do it in two stages:
>>   - first integration to keep close to the original fix that was already
>> tested on Linux platform (as Mark tells)
>>   - separate refactoring to follow the platform separation rules (will
>> need to file another CR)
>>
>> Not sure I'll be able to work on this in the near future (a month) as I
>> have to switch to the urgent security issue.
>> But it is still can be possible to do in a background with a low 
>> priority.
>>
>> Other action items for this integration:
>>    1. Test that the HS DTrace are not broken on Solaris
>>    2. Find or setup a Linux machine with the systemTap
>>    3. Check that the fix is built Ok
>>    4. Test that the fix works on Linux
>>        Need a unit test for the fix (Mark, do you have any?)
>>
>> Thanks,
>> Serguei
>>
>>
>> On 5/23/12 9:27 AM, Keith McGuigan wrote:
>>>
>>> Hi Mark -
>>>
>>> I'd prefer that it's done the "right" way (based on *my* definition of
>>> "right", of course :) ), but I won't put up a fuss if whomever
>>> shepherds this through agrees with you and wants to keep it in it's
>>> current form.  I expect that will be Serguei, or someone else from the
>>> serviceability team?  (Serguei?)
>>>
>>> -- 
>>> - Keith
>>>
>>> On 5/23/2012 12:19 PM, Mark Wielaard wrote:
>>>> On Wed, 2012-05-23 at 10:23 -0400, Keith McGuigan wrote:
>>>>> On 5/23/2012 9:56 AM, Mark Wielaard wrote:
>>>>>>>     It's (of course) just a style thing, but traditionally in 
>>>>>>> hotspot
>>>>>>> we've wanted the os or arch specific code in os or arch specific
>>>>>>> directories, instead of littering the code with #ifdefs.  I know
>>>>>>> the OSX
>>>>>>> stuff started violated this some, but I hope we're going to
>>>>>>> resolve that
>>>>>>> rather than continue down that path.
>>>>>>
>>>>>> I wouldn't have mind if the OSX stuff was split out this way, but
>>>>>> now I
>>>>>> don't have any example to follow here. What/How do you suggest it is
>>>>>> done?
>>>>>
>>>>> My first thought would be to create a dtrace_solaris.hpp file in
>>>>> src/os/solaris/vm and a dtrace_linux.hpp file in src/os/linux/vm (and
>>>>> maybe the same for bsd and windows?) for any OS-specific stuff, like
>>>>> the
>>>>> macro definitions and maybe even that solaris workaround.  Leave
>>>>> anything common in the original file (like the default empty
>>>>> definitions
>>>>> and such).  Then you include the specific-file based on the target 
>>>>> OS,
>>>>> similar to what's done here:
>>>>> http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/file/03d61caacd1e/src/share/vm/runtime/interfaceSupport.hpp 
>>>>>
>>>>>
>>>>>
>>>>> Yeah... it's a bit of a pain, and maybe in this case it's a little
>>>>> overkill.  But it's definitely nice to have a location for the
>>>>> OS-specific stuff and it keeps the code rather neat.
>>>>
>>>> So, do you require this change or not? I don't particularly like that
>>>> style (you loose the overview/a central place where all the magic 
>>>> macros
>>>> are defined), but I can certainly do it if you require it now. But 
>>>> then
>>>> I want to do it as additional patches on top of the current patches to
>>>> make sure they are separate. Since I will then have to touch code 
>>>> for at
>>>> least two architectures to which I don't have access (I also don't 
>>>> have
>>>> access to windows nor bsd, but I hope I don't have to touch those 
>>>> too).
>>>> So I want such changes to be testable separately by the architecture
>>>> maintainers.
>>>>
>>>> Thanks,
>>>>
>>>> Mark
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-dev/attachments/20120523/d48540d8/attachment.html