RFR: 8210832: Remove sneaky locking in class Monitor
Patricio Chilano
patricio.chilano.mateo at oracle.com
Tue Feb 5 15:00:02 UTC 2019
On 2/5/19 2:58 AM, Robbin Ehn wrote:
> On 2019-02-05 01:06, David Holmes wrote:
>> v6 incremental looks good to me.
>
> +1
Thanks Robbin!
Patricio
> /Robbin
>
>>
>> Thanks,
>> David
>> -----
>>
>> On 5/02/2019 6:57 am, Patricio Chilano wrote:
>>> Hi Dan,
>>>
>>> Here is v06. It also contains David's suggested changes for
>>> assert_owner(), and based on your correction about declaring
>>> pointers I fixed one in ThreadBlockInVMWithDeadlockCheck
>>> declaration. Files mutex.cpp and mutex.hpp have different styles of
>>> "pointer to" declarations so except the one you mentioned I didn't
>>> try to fix them all since I don't know if changing all of them to "
>>> * " is the right approach.
>>>
>>> Inc: http://cr.openjdk.java.net/~pchilanomate/8210832/v06/inc/webrev
>>> Full: http://cr.openjdk.java.net/~pchilanomate/8210832/v06/webrev/
>>>
>>> Thanks,
>>> Patricio
>>>
>>> On 2/4/19 2:47 PM, Daniel D. Daugherty wrote:
>>>> On 2/4/19 2:40 PM, Patricio Chilano wrote:
>>>>> Hi Dan,
>>>>>
>>>>> On 2/4/19 2:05 PM, Daniel D. Daugherty wrote:
>>>>>> On 2/3/19 8:46 PM, Patricio Chilano wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Here is v05:
>>>>>>>
>>>>>>> Full: http://cr.openjdk.java.net/~pchilanomate/8210832/v05/webrev/
>>>>>>> Inc: http://cr.openjdk.java.net/~pchilanomate/8210832/v05/inc/
>>>>>>
>>>>>> src/hotspot/share/runtime/interfaceSupport.inline.hpp
>>>>>> L295: // - When transitioning in (constructor), it checks for
>>>>>> safepoints without blocking, i.e calls
>>>>>> nit - s/i.e /i.e., /
>>>>>> (You missed the change from my v04 review)
>>>>> Done! I did made the change s/having to block/blocking from your
>>>>> review but didn't see the extra comma you added after i.e.
>>>>
>>>> Extra period and extra comma... I'm a font/typography nut... :-)
>>>>
>>>>
>>>>>
>>>>>> L299: class ThreadBlockinVMWithDeadlockCheck : public
>>>>>> ThreadStateTransition {
>>>>>> L312: ThreadBlockinVMWithDeadlockCheck(JavaThread *thread,
>>>>>> Monitor** in_flight_monitor_adr)
>>>>>> L326: ~ThreadBlockinVMWithDeadlockCheck() {
>>>>>> s/ThreadBlockinVMWithDeadlockCheck/ThreadBlockInVMWithDeadlockCheck/
>>>>>> Lower case 'i' -> 'I' to match the
>>>>>> CamelCastStyleInOtherHelpers.
>>>>> Done.
>>>>>
>>>>>> src/hotspot/share/runtime/mutex.cpp
>>>>>> L36: static inline void assert_owner(Thread *owner, Thread
>>>>>> *thread) {
>>>>>> nit - the predominant style in this file is ' * ' (space
>>>>>> on each side of '*')
>>>>> Ok, is that the style that we are supposed to follow in general ?
>>>>> Otherwise I can change all the ones that do not match the
>>>>> preferred style.
>>>>
>>>> That's a good question. The general rule is to follow the
>>>> predominant style in the file. However, I'm not sure what
>>>> the HotSpot recommended style is (I used to know this...).
>>>>
>>>>
>>>>>
>>>>>> L38: "invalid owner: owner=" INTPTR_FORMAT ", should
>>>>>> be=" INTPTR_FORMAT,
>>>>>> Another possible style of mesg:
>>>>>> "invalid owner: actual=" INTPTR_FORMAT ",
>>>>>> expect=" INTPTR_FORMAT,
>>>>> I like more the first style, but if you really prefer the second
>>>>> one I can change it.
>>>>
>>>> I'm okay with what you have.
>>>>
>>>>
>>>>>
>>>>>> ThreadBlockinVMWithDeadlockCheck renames to
>>>>>> ThreadBlockInVMWithDeadlockCheck in
>>>>>> this file also.
>>>>> Done.
>>>>>
>>>>>> src/hotspot/share/runtime/mutex.hpp
>>>>>> No comments.
>>>>>>
>>>>>> src/hotspot/share/runtime/safepointMechanism.inline.hpp
>>>>>> L83: if (!uses_thread_local_poll() ||
>>>>>> local_poll_armed(thread)) {
>>>>>> Did you decide this line does not need a comment at all?
>>>>> How about adding the following comment based on your previous email:
>>>>>
>>>>> // If using thread local polls, we should not check the
>>>>> // global_poll() and callback via block() if the VMThread
>>>>> // has not yet armed the local poll. Otherwise, when used in
>>>>> // combination with should_block(), the latter could miss
>>>>> // detecting the same safepoint that this method would detect
>>>>> // if only checking global polls.
>>>>
>>>> I'm good with that comment.
>>>>
>>>> Dan
>>>>
>>>>
>>>>>
>>>>>
>>>>> Patricio
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Tested tiers 1-6.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Patricio
>>>>>>>
>>>>>>> On 2/1/19 7:58 PM, Daniel D. Daugherty wrote:
>>>>>>>> On 2/1/19 7:49 PM, David Holmes wrote:
>>>>>>>>> <trimming>
>>>>>>>>>
>>>>>>>>> On 2/02/2019 7:48 am, Patricio Chilano wrote:
>>>>>>>>>> On 2/1/19 4:28 PM, coleen.phillimore at oracle.com wrote:
>>>>>>>>>>>> In regards to the name ThreadLockBlockInVM ... I don't have
>>>>>>>>>>>> a good suggestion.
>>>>>>>>>>>> ThreadBlockInVMWithSafepointCheckingButOnlyBlockOnTheWayOut
>>>>>>>>>>>> is rather unwieldy. ;-) But the "Lock" part really doesn't
>>>>>>>>>>>> mean anything. So your suggestion of
>>>>>>>>>>>> ThreadBlockinVMWithDeadlockPrevention seems a big
>>>>>>>>>>>> improvement to me. :)
>>>>>>>>>>>
>>>>>>>>>>> How about ThreadBlockInVMForLock ? This answers the
>>>>>>>>>>> question "why" this class, vs. "what" this class does. Since
>>>>>>>>>>> the latter can change.
>>>>>>>>>> I like that name. Is that name okay with you David?
>>>>>>>>>
>>>>>>>>> Sorry no. "Lock" still adds zero information about when/why
>>>>>>>>> you would use this. It's used within Monitor::lock and
>>>>>>>>> Monitor::wait so is not specific to "locking" even in that
>>>>>>>>> class. This is about blocking in the VM and we need to know
>>>>>>>>> what this version does differently to the existing TBIVM.
>>>>>>>>> There are lots of different types of "locking" in the VM and
>>>>>>>>> we don't use this with them.
>>>>>>>>>
>>>>>>>>> Dan can be a tie-breaker on this bikeshed. Grab your brush
>>>>>>>>> Dan! ;-)
>>>>>>>>
>>>>>>>> ThreadBlockInVMBecauseSneakyLockingIsEvil?
>>>>>>>>
>>>>>>>> ThreadBlockinVMWithDeadlockPrevention is okay.
>>>>>>>>
>>>>>>>> ThreadBlockinVMWithDeadlockCheck
>>>>>>>> ThreadBlockinVMWithDeadlockChk
>>>>>>>> ThreadBlockinVMWithDeadlockCk (David won't like this one :-) )
>>>>>>>>
>>>>>>>> Just pick a name and run with it... :-)
>>>>>>>>
>>>>>>>> Dan
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> David
>>>>>>>>> -----
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Patricio
>>>>>>>>>>> thanks,
>>>>>>>>>>> Coleen
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> David
>>>>>>>>>>>> -----
>>>>>>>>>>>>
>>>>>>>>>>>> On 1/02/2019 3:05 pm, Patricio Chilano wrote:
>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 1/31/19 12:54 AM, David Holmes wrote:
>>>>>>>>>>>>>> On 31/01/2019 7:37 am, Patricio Chilano wrote:
>>>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 1/30/19 2:29 AM, David Holmes wrote:
>>>>>>>>>>>>>>>> Hi Patricio,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> <trimming>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> First, thanks for all the many weeks of work you've put
>>>>>>>>>>>>>>>> into this, pulling together a number of ideas from
>>>>>>>>>>>>>>>> different people to make it all work!
>>>>>>>>>>>>>>> Thanks! Credit to you for the PlatformMonitor
>>>>>>>>>>>>>>> implementation : )
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> :) Nothing innovative there.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I've only got a few minor comments/suggestions.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 30/01/2019 10:24 am, Patricio Chilano wrote:
>>>>>>>>>>>>>>>>> Full:
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~pchilanomate/8210832/v03/webrev/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Inc:
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~pchilanomate/8210832/v03/inc/webrev/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> src/hotspot/share/runtime/interfaceSupport.inline.hpp
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I'm very unclear how ThreadLockBlockInVM differs from
>>>>>>>>>>>>>>>> ThreadBlockInVM. You've duplicated a lot of complex
>>>>>>>>>>>>>>>> code which is masking the actual difference between the
>>>>>>>>>>>>>>>> two wrappers to me. It seems to me that an extra arg to
>>>>>>>>>>>>>>>> transition_and_fence should allow you to handle the new
>>>>>>>>>>>>>>>> behaviour without having to duplicate so much of this
>>>>>>>>>>>>>>>> code. In any case the semantics of ThreadLockBlockInVM
>>>>>>>>>>>>>>>> needs to be described.
>>>>>>>>>>>>>>> I could do it with one extra argument, but I would need
>>>>>>>>>>>>>>> to add two extra branches in transition_and_fence(), one
>>>>>>>>>>>>>>> to decide if I'm in the Monitor case to avoid calling
>>>>>>>>>>>>>>> SafepointMechanism::block_if_requested() directly and
>>>>>>>>>>>>>>> another one to actually decide if I'm transitioning in
>>>>>>>>>>>>>>> or out, since the actions to perform are different. I
>>>>>>>>>>>>>>> think it is easier to read without adding new
>>>>>>>>>>>>>>> conditionals, and also we will save those extra
>>>>>>>>>>>>>>> branches, but if you think it's better this way I can
>>>>>>>>>>>>>>> change it.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I would like something that tells me more clearly how
>>>>>>>>>>>>>> this new transition helper differs from the existing
>>>>>>>>>>>>>> TBIVM. Sharing the code between them and using different
>>>>>>>>>>>>>> args would be one way. Documenting the difference in
>>>>>>>>>>>>>> comments would be another. Your choice.
>>>>>>>>>>>>> Ok, I added a description on top of TLBIVM.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Also I'm unclear what the "Lock" in ThreadLockBlockInVM
>>>>>>>>>>>>>>>> actually refers to. I find the name quite jarring to read.
>>>>>>>>>>>>>>> What about changing it to ThreadBlockinMonitor?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> That's not quite conveying the semantics. The problem is
>>>>>>>>>>>>>> that the semantics we are changing compared to TBIVM are
>>>>>>>>>>>>>> not evident in the TBIVM name. If TBIVM was actually
>>>>>>>>>>>>>> ThreadBlockInVMWithSafepointBlocking, then this new
>>>>>>>>>>>>>> transition would obviously be
>>>>>>>>>>>>>> ThreadBlockInVMWithoutSafepointBlocking - but perhaps
>>>>>>>>>>>>>> that lengthy, but clear name would be okay anyway?
>>>>>>>>>>>>> Not convinced on that name since we are blocking at
>>>>>>>>>>>>> safepoints in the destructor. Based on the comments I
>>>>>>>>>>>>> added how about ThreadBlockinVMWithDeadlockPrevention ? or
>>>>>>>>>>>>> ThreadBlockinVMWithPreemption? (as in eliminate one of the
>>>>>>>>>>>>> conditions for deadlocks).
>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On the subject of naming, do_preempt and
>>>>>>>>>>>>>>>> preempt_by_safepoint don't really convey to me what
>>>>>>>>>>>>>>>> happens - what is being "preempted" here? I would
>>>>>>>>>>>>>>>> suggest a more direct Monitor::release_for_safepoint
>>>>>>>>>>>>>>> Changed.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Logging: why "nativemonitor"? The logging in mutex.cpp
>>>>>>>>>>>>>>>> doesn't relate to a "native" monitor?? Actually I'm not
>>>>>>>>>>>>>>>> even sure if we need bother at all with the one logging
>>>>>>>>>>>>>>>> statement that is present.
>>>>>>>>>>>>>>> I added it to eventually track unbounded try locks. Not
>>>>>>>>>>>>>>> sure I follow you with the name, isn't that how we name
>>>>>>>>>>>>>>> this monitors? I tried to differentiate them from Java
>>>>>>>>>>>>>>> monitors. What about just "monitor"?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> How about vmmonitor ?
>>>>>>>>>>>>> Ok, changed.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> src/hotspot/share/runtime/mutex.cpp
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> void Monitor::lock_without_safepoint_check(Thread *
>>>>>>>>>>>>>>>> self) {
>>>>>>>>>>>>>>>> // Ensure that the Monitor does not require or allow
>>>>>>>>>>>>>>>> safepoint checks.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> The comment there should only say "not require".
>>>>>>>>>>>>>>> Done.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> void Monitor::preempt_by_safepoint() {
>>>>>>>>>>>>>>>> _lock.unlock();
>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Apart from renaming this as suggested above, aren't
>>>>>>>>>>>>>>>> there any suitable assertions we should have here?
>>>>>>>>>>>>>>>> safepoint-in-progress or handshake-in-progress? _owner
>>>>>>>>>>>>>>>> == Thread::current?
>>>>>>>>>>>>>>> Ok, I added an assertion that owner should be NULL.
>>>>>>>>>>>>>>> Asserting safepoint-in-progress does not really work
>>>>>>>>>>>>>>> because _state could change to _not_synchronized right
>>>>>>>>>>>>>>> after you checked for it in TLBIVM.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Okay.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Nit:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> assert(_owner == Thread::current(), "should be equal:
>>>>>>>>>>>>>>>> owner=" INTPTR_FORMAT
>>>>>>>>>>>>>>>> ", self=" INTPTR_FORMAT,
>>>>>>>>>>>>>>>> p2i(_owner), p2i(Thread::current()));
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> with Dan's enhanced assertions there's an indentation
>>>>>>>>>>>>>>>> issue. The second line should indent to the first
>>>>>>>>>>>>>>>> comma, but that will make the second line extend way
>>>>>>>>>>>>>>>> past 80 columns.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Also you could factor that assertion for
>>>>>>>>>>>>>>>> _owner==Thread::current() into its own function or
>>>>>>>>>>>>>>>> macro to avoid the repetition.
>>>>>>>>>>>>>>> Corrected indentation based on Dan's reply to align with
>>>>>>>>>>>>>>> _owner.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I though it should indent to the comma because it is a
>>>>>>>>>>>>>> continuation of the same argument being passed to the
>>>>>>>>>>>>>> assert "function". But I'm okay with Dan's suggestion.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Factoring it into its own little function or macro would
>>>>>>>>>>>>>> still be good to avoid the repetition.
>>>>>>>>>>>>> Ok, added new function assert_owned_by_self(). I could
>>>>>>>>>>>>> change it to assert_owner(Thread*) and use it for the NULL
>>>>>>>>>>>>> case too unifying the printed messages to something like
>>>>>>>>>>>>> "invalid owner: owner=" INTPTR_FORMAT ", should be:"
>>>>>>>>>>>>> INTPTR_FORMAT. What do you think?
>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> OSThreadWaitState osts(self->osthread(), false /* not
>>>>>>>>>>>>>>>> Object.wait() */);
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> This needs to be returned to its original place as per
>>>>>>>>>>>>>>>> Dan's comments.
>>>>>>>>>>>>>>> Done.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> } else {
>>>>>>>>>>>>>>>> Monitor::lock(self);
>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> You don't need Monitor:: here
>>>>>>>>>>>>>>> Removed.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> // Temporary JVM_RawMonitor* support. A raw monitor can
>>>>>>>>>>>>>>>> just be a PlatformMonitor now.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> This needs to be resolved before committing. Some of
>>>>>>>>>>>>>>>> the existing commentary on what raw monitors are needs
>>>>>>>>>>>>>>>> to be retained. Not clear if we need to set the _owner
>>>>>>>>>>>>>>>> field or can just skip it.
>>>>>>>>>>>>>>> Is it okay if I keep the following comments?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> // Yet another degenerate version of Monitor::lock() or
>>>>>>>>>>>>>>> lock_without_safepoint_check()
>>>>>>>>>>>>>>> // jvm_raw_lock() and _unlock() can be called by
>>>>>>>>>>>>>>> non-Java threads via JVM_RawMonitorEnter.
>>>>>>>>>>>>>>> //
>>>>>>>>>>>>>>> // There's no expectation that JVM_RawMonitors will
>>>>>>>>>>>>>>> interoperate properly with the native
>>>>>>>>>>>>>>> // Mutex-Monitor constructs. We happen to implement
>>>>>>>>>>>>>>> JVM_RawMonitors in terms of
>>>>>>>>>>>>>>> // native Mutex-Monitors simply as a matter of convenience.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yep that's perfect. And as a future RFE we can replace
>>>>>>>>>>>>>> them with direct use of PlatformMonitor (or PlatformMutex).
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I could keep setting the owner as _owner =
>>>>>>>>>>>>>>> Thread::current_or_null() in jvm_raw_lock(), at least it
>>>>>>>>>>>>>>> wouldn't hurt.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> It's useful for checking usage errors, but we won't have
>>>>>>>>>>>>>> that if we replace with PlatformMonitor, so may as well
>>>>>>>>>>>>>> drop it now IMO.
>>>>>>>>>>>>> Ok, I added asserts that _owner should be NULL after
>>>>>>>>>>>>> acquiring it and before releasing it though.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Monitor::~Monitor() {
>>>>>>>>>>>>>>>> assert(_owner == NULL, "should be NULL: owner="
>>>>>>>>>>>>>>>> INTPTR_FORMAT, p2i(_owner));
>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Will this automatically result in the PlatformMonitor
>>>>>>>>>>>>>>>> destructor being called?
>>>>>>>>>>>>>>> Yes, should I add a comment to make it clear that
>>>>>>>>>>>>>>> ~PlatformMonitor() will be executed?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> No need - assume other people have a better understanding
>>>>>>>>>>>>>> of C++ than I do :)
>>>>>>>>>>>>>
>>>>>>>>>>>>> Below is version v04, which also contains a correction
>>>>>>>>>>>>> pointed out off-list by Robbin to do a local poll first in
>>>>>>>>>>>>> SafepointMechanism::callback_if_safepoint() if we are
>>>>>>>>>>>>> using local polls. Since the thread local poll is armed
>>>>>>>>>>>>> after changing _state to _synchronizing, if we only do a
>>>>>>>>>>>>> global poll in the TLBIVM constructor we could detect
>>>>>>>>>>>>> there is a safepoint in progress and callback but when
>>>>>>>>>>>>> coming back in the destructor
>>>>>>>>>>>>> SafepointMechanism::should_block() could miss detecting
>>>>>>>>>>>>> the safepoint in progress since that method checks first
>>>>>>>>>>>>> for local polls.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Full: http://cr.openjdk.java.net/~pchilanomate/8210832/v04/
>>>>>>>>>>>>> Inc:
>>>>>>>>>>>>> http://cr.openjdk.java.net/~pchilanomate/8210832/v04/inc/
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Patricio
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks for looking into this! Waiting on your comments
>>>>>>>>>>>>>>> to send v04.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>> Patricio
>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>> -----
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
More information about the hotspot-dev
mailing list