rxevent: fix mismatched #endif We should only assign to 'ev' once, rather than assigning a second time to an uninitialized value. Reported by Ben Huntsman and diagnosed by Jeffrey Altman. Change-Id: I63f50d46658e95ae570dc78dcfeffce973a9e7ed Reviewed-on: https://gerrit.openafs.org/15106 Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
rx: Make rxevent_Put NULL the event ptr being put Change rxevent_Put so that it takes a pointer to the event being put, and NULLs that pointer. This removes a lot of duplicate code in callers, as well as making it harder to reuse a discarded event. Change-Id: Ib7a51f01687e08ea3dced5932ec9ec27797a784a Reviewed-on: http://gerrit.openafs.org/8540 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Derrick Brashear <shadow@your-file-system.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
rx: Return success value when cancelling an event When cancelling an event that holds reference counts, we need to know whether the attempt to cancel the event was successful or not, otherwise references can be double put when the cancellation races with the event firing. Take the follwing Thread A Event Thread ========= ============ ... event fired ... MUTEX_ENTER(&obj->lock); if (obj->event) { rxevent_Cancel(&obj->event); obj_put(&obj->refcnt); } MUTEX_EXIT(&obj->lock) MUTEX_ENTER(&obj->lock); if (event == obj->event) rxevent_Put(&obj->event); ... MUTEX_EXIT(&obj->lock); obj_put(&obj->refcnt); Holding obj->lock doesn't control whether the event is fired or not. Only putting the reference if the event being fired matches that in obj doesn't help - it just leaks a reference in a different race. So, make rxevent_Cancel return true if it did actually cancel the event, and false if the event has already been fired. This means that Thread A can become MUTEX_ENTER(&obj->lock); if (rxevent_Cancel(&obj->event) obj_put(&obj->refcnt); MUTEX_EXIT(&obj->lock) In the example above, rxevent_Cancel would return false. Change-Id: I48e012774c97c9d9588b00687428a32795be2b37 Reviewed-on: http://gerrit.openafs.org/8539 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Derrick Brashear <shadow@your-file-system.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
rx: Don't treat calls specially in event package Many different structures can be passed to the rxevent package as data. Don't give calls special treatment by making rxevent aware of how to release their reference counts when an event is cancelled. Update all of the callers of rxevent_Cancel to use the new arguments, and where they were cancelling functions with calls as parameters add the appropriate CALL_RELE directives. In many cases, this has led to new helper functions to cancel particular call-based events. Change-Id: Ic02778e48fd950e8850b77bd3c076c235453274d Reviewed-on: http://gerrit.openafs.org/8538 Reviewed-by: Derrick Brashear <shadow@your-file-system.com> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
rx: Don't adjust non-existent events If we notice that time has gone backwards (that is, the current time is older than the time of the last event we fired), then we reschedule all pending events. On Windows, immediately after we have resumed from a suspend, this code path can be executed with an empty event tree, causing an exception: FAULTING_IP: afsrpc!adjustTimes+cf [c:\src\openafs\openafs.git\repo\src\rx\rx_event.c @ 213] 00000000`61041847 4c8b4030 mov r8,qword ptr [rax+30h] EXCEPTION_RECORD: ffffffffffffffff -- (.exr 0xffffffffffffffff) ExceptionAddress: 0000000061041847 (afsrpc!adjustTimes+0x00000000000000cf) ExceptionCode: c0000005 (Access violation) ExceptionFlags: 00000000 NumberParameters: 2 Parameter[0]: 0000000000000000 Parameter[1]: 0000000000000030 Attempt to read from address 0000000000000030 Resolve this by checking for an empty tree before we attempt to adjust event times. If the tree is empty, we just zero the last event time (so we don't keep running the adjustTimes routine), and continue as normal. Change-Id: I42a42ff1bd53a9d5c4733efc7ac5f629426b3aa1 Reviewed-on: http://gerrit.openafs.org/6425 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com> Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>
rx: Make CALL_RELE and CALL_HOLD lock refcnt mutex The reference count mutex must always be held when calling CALL_RELE or CALL_HOLD. Instead of requiring that the caller obtain, and release the mutex, do so within the HOLD and RELE macros, greatly simplifying calling code. Provide CALL_RELE_R and CALL_HOLD_R as versions of these macros which can be used by callers who already hold the reference count mutex for other purposes. Change-Id: Ie3e9df8b9d2a79476f1707bd65e588f43271c636 Reviewed-on: http://gerrit.openafs.org/6219 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Derrick Brashear <shadow@dementix.org> Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com> Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>
rx: Make the rx_call structure private Hide the rx_call structure for public view. Provide accessors for those elements which are currently accessed by applications. Note that this change as it currently stands removes the visibility of the last sent time, and sequence number information, from the VolMonitor function. Change-Id: Ib25ab5635126f893ae43acb684d92a78278d6ca6 Reviewed-on: http://gerrit.openafs.org/6181 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com> Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>
rx: Some kernels have no reschedule function If RXK_TIMEDSLEEP_ENV isn't set, then Unix kernel cache managers call rxevent_Init without a reschedule function. Check for this so we don't end up calling a NULL function in these situations. Change-Id: I5e89f5247aeffc4c27d3f81c0ccabe4979232846 Reviewed-on: http://gerrit.openafs.org/6206 Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>
rx: Use a red black tree for the event stack Instead of the current event stack, which uses a sorted linked list, use a red/black tree to maintain the timer stack. This dramatically improves event insertion times, at the expense of some additional implementation complexity. This change also adds reference counting to the rxevent structure. We've always had a race between an event being fired, and that event being simultaneously cancelled by the user thread. Reference counting avoids that race resulting in the structure appearing twice in the free list. Change-Id: Icbef6e04e01f3eef5b888bc3cb77b7a3d1be26ae Reviewed-on: http://gerrit.openafs.org/5841 Tested-by: BuildBot <buildbot@rampaginggeek.com> Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com> Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>
rx: Turn the rxevent_Cancel macro into a function Turn rxevent_Cancel into a function rather than a macro which modifies its argument as a side effect. rxevent_Cancel now checks whether the event being cancelled is already NULL, as well as NULLifying the event when it is actually cancelled. Update all of the callers to reflect this new API, and so they no longer do unecessary work. Change-Id: I75b68f1c8f1a3023edd6113600663fe2b60d6097 Reviewed-on: http://gerrit.openafs.org/5840 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Derrick Brashear <shadow@dementix.org>
rx: New signature for rx event functions For a while now, we've had both new and old-style rx event callback functions. Modify all of our event handlers, and the functions that install them, to use only new style functions, and get rid of the old-style function prototypes. Change-Id: Ic7c568df9d191edb082fb41fb7705c54ca93cf48 Reviewed-on: http://gerrit.openafs.org/5839 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Derrick Brashear <shadow@dementix.org>
rx: Tidy header includes Remove headers which are provided by libroken, and reorder header includes so that they're a bit a more legible. Change-Id: I7bb08d7ec7a75b485f7f619fd4d8179d4c7349f0 Reviewed-on: http://gerrit.openafs.org/4403 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
OpenBSD: curproc has moved in OpenBSD 4.8 With OpenBSD 4.8, curproc has moved from h/proc.h to h/systm.h. Fix rx_event.c to reflect this change. Change-Id: I38d4676d445bfafa47f2ae973d789d0b9f6a687a Reviewed-on: http://gerrit.openafs.org/3749 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Derrick Brashear <shadow@dementia.org>
libroken: Build on windows A minimal change set to get libroken to build on Windows. Sadly, libroken contains definitions for a number of platform compatibility macros which were previously scattered throughout the windows code. These scattered macros have to be removed in order to build libroken. The impact of this removal is that a very large number of files throughout the tree require the addition of "roken.h" to pick up the new compatibility code. The bulk of this change is adding these includes. In addition, some of the added includes add roken dependencies to the Unix build. So, also add libroken to the build rules in affected Unix Makefiles. Change-Id: Ifba431bd37e67b1e273fbc6f69b805a232193456 Reviewed-on: http://gerrit.openafs.org/3205 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Derrick Brashear <shadow@dementia.org>
rx: Reorganise includes RX files were including the same header set in three different places, once for user-land builds, once for kernel builds and once for ukernel. The duplication was a bit pointless, and really frustrating when adding new headers. So, reorganise the includes so that we only list headers that are used in all three builds in one location. Also take the opportunity to indent the #ifdefs so that it is clear what is going on, and to remove some more of AFS_OSF_ENV and AFS_AUX_ENV from kernel builds. Change-Id: Ic2b5d39de4dd406bbc0acaa29fc876ac882ccf10 Reviewed-on: http://gerrit.openafs.org/3160 Reviewed-by: Derrick Brashear <shadow@dementia.org> Reviewed-by: Jeffrey Altman <jaltman@openafs.org> Tested-by: Jeffrey Altman <jaltman@openafs.org>
Rx: use osi_Assert/osi_Panic instead of assert Avoid using the openafs src/util/assert.h implementation for Rx and Rx security classes. Use the built-in osi_Assert() and osi_Panic() functionality instead. This avoids all references to assert.h except for rx_pthread.c (Unix only) which requires it for the assert() references in the src/util/pthread_nosigs.h macros. Change-Id: I5fbfcd57da381e02e716e7688a58918aed05c50f Reviewed-on: http://gerrit.openafs.org/2987 Reviewed-by: Derrick Brashear <shadow@dementia.org> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@openafs.org> Tested-by: Jeffrey Altman <jaltman@openafs.org>
RX: Make rxi_Alloc return (void *) rxi_Alloc returns a pointer to an anonymous data block. Make its return value (void *) rather than (char *), so that it can be called in the same way as malloc(), and not require casting. Change-Id: Ib3094ad0273c62a4430eb56d35e3df68a550d338 Reviewed-on: http://gerrit.openafs.org/2738 Reviewed-by: Derrick Brashear <shadow@dementia.org> Tested-by: Derrick Brashear <shadow@dementia.org>
death to trailing whitespace if we're gonna clean up... Change-Id: I5ab03f29468577b62dacab41a67eadfd8c43f812 Reviewed-on: http://gerrit.openafs.org/2463 Reviewed-by: Derrick Brashear <shadow@dementia.org> Tested-by: Derrick Brashear <shadow@dementia.org>
Use AFS_PTR_FMT to format pointers Replace "0x%x" with "%"AFS_FMT_PTR for pointer printing in all locations where gcc flags a warning. This change is warnings reduction driven - there are many more occurences of this in the code which don't currently result in compiler warnings, because the va_args functions they're used in aren't defined as printflike. Reviewed-on: http://gerrit.openafs.org/75 Verified-by: Russ Allbery <rra@stanford.edu> Reviewed-by: Russ Allbery <rra@stanford.edu> Reviewed-by: Derrick Brashear <shadow@dementia.org> Verified-by: Jeffrey Altman <jaltman@openafs.org> Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Rename printf cast helpers and clean up format string warnings Some confusion had ensued about the usage of our printf cast helper functions. Rename these to attempt to allay that confusion, and restore the functions themselves to their original definitions. Essentially, afs_printable_int32_ld() and friends are helpers to go from afs specifc types to things that can be emitted by printf without causing compiler warnings. Also clean up some additional warnings from type mismatches between escapes in printf format strings and the variables being printed. Reviewed-on: http://gerrit.openafs.org/50 Verified-by: Russ Allbery <rra@stanford.edu> Reviewed-by: Russ Allbery <rra@stanford.edu>