[PATCH 10/22] udev: Use shared NoopDDA to utilize code cache better
Daniel Stone
daniel at fooishbar.org
Wed Dec 29 16:52:25 PST 2010
On Wed, Dec 29, 2010 at 09:19:50PM +0100, Mark Kettenis wrote:
> > - RegisterBlockAndWakeupHandlers(block_handler, wakeup_handler, NULL);
> > + RegisterBlockAndWakeupHandlers((BlockHandlerProcPtr)NoopDDA, wakeup_handler, NULL);
>
> YUCK! Obfuscating code like this is sooo wrong. I don't think
> optimization hacks like this that only really matter for hardware with
> last-century sized caches belong in the Xorg tree.
Mark,
While I'm very pleased to see your newfound enthusiasm for modernity
(cf. the pthreads discussion about supporting operating systems which
still don't have usable threads in 2011, and how the baseline should be
last-century POSIX), I don't think this is valid.
Firstly, because I don't think NoopDDA is really an ugly hack at all;
it's pretty clear that you want it to do nothing. That being said, I
think it could be cleaner still - see below. But I don't see how it's a
hack, really.
Secondly, because as nice as having physically huge systems consuming
mindblowing amounts of power is, lower-power architectures such as ARM,
Atom, et al, still have small caches by necessity. They're pretty
cutting-edge in most respects (look at newer Cortexes), but due to size
constraints, cannot fit in large amounts of cache. They also tend to
have much lower memory bandwidth, so cache misses are punitively
expensive.
Anyway, I think it could be done cleaner, with something like the
attached. Seems to run and work; comments welcome.
Cheers,
Daniel
commit 3ae8c44514b121a483e1fc3af5e31d3294460d95
Author: Daniel Stone <daniel at fooishbar.org>
Date: Thu Dec 30 00:49:52 2010 +0000
Allow NULL block/wakeup handlers
If someone wants to register only one of a block or wakeup handler,
allow them to pass NULL rather than having a stub function or using
NoopDDA. Change all existing callers doing this to pass NULL to
RegisterBlockAndWakeupHandlers and RemoveBlockAndWakeupHandlers, and
remove the stub functions.
Signed-off-by: Daniel Stone <daniel at fooishbar.org>
diff --git a/Xext/xselinux_hooks.c b/Xext/xselinux_hooks.c
index 560e1e9..74fc65a 100644
--- a/Xext/xselinux_hooks.c
+++ b/Xext/xselinux_hooks.c
@@ -804,11 +804,6 @@ SELinuxResourceState(CallbackListPtr *pcbl, pointer unused, pointer calldata)
static int netlink_fd;
static void
-SELinuxBlockHandler(void *data, struct timeval **tv, void *read_mask)
-{
-}
-
-static void
SELinuxWakeupHandler(void *data, int err, void *read_mask)
{
if (FD_ISSET(netlink_fd, (fd_set *)read_mask))
@@ -838,8 +833,7 @@ SELinuxFlaskReset(void)
/* Tear down SELinux stuff */
audit_close(audit_fd);
avc_netlink_release_fd();
- RemoveBlockAndWakeupHandlers(SELinuxBlockHandler, SELinuxWakeupHandler,
- NULL);
+ RemoveBlockAndWakeupHandlers(NULL, SELinuxWakeupHandler, NULL);
RemoveGeneralSocket(netlink_fd);
avc_destroy();
@@ -908,8 +902,7 @@ SELinuxFlaskInit(void)
netlink_fd = avc_netlink_acquire_fd();
AddGeneralSocket(netlink_fd);
- RegisterBlockAndWakeupHandlers(SELinuxBlockHandler, SELinuxWakeupHandler,
- NULL);
+ RegisterBlockAndWakeupHandlers(NULL, SELinuxWakeupHandler, NULL);
/* Register callbacks */
ret &= AddCallback(&ClientStateCallback, SELinuxClientState, NULL);
diff --git a/config/dbus-core.c b/config/dbus-core.c
index 4c5e10f..7a9b78e 100644
--- a/config/dbus-core.c
+++ b/config/dbus-core.c
@@ -61,11 +61,6 @@ wakeup_handler(pointer data, int err, pointer read_mask)
}
}
-static void
-block_handler(pointer data, struct timeval **tv, pointer read_mask)
-{
-}
-
/**
* Disconnect (if we haven't already been forcefully disconnected), clean up
* after ourselves, and call all registered disconnect hooks.
@@ -86,7 +81,7 @@ teardown(void)
if (bus_info.connection)
dbus_connection_unref(bus_info.connection);
- RemoveBlockAndWakeupHandlers(block_handler, wakeup_handler, &bus_info);
+ RemoveBlockAndWakeupHandlers(NULL, wakeup_handler, &bus_info);
if (bus_info.fd != -1)
RemoveGeneralSocket(bus_info.fd);
bus_info.fd = -1;
@@ -164,7 +159,7 @@ connect_to_bus(void)
dbus_error_free(&error);
AddGeneralSocket(bus_info.fd);
- RegisterBlockAndWakeupHandlers(block_handler, wakeup_handler, &bus_info);
+ RegisterBlockAndWakeupHandlers(NULL, wakeup_handler, &bus_info);
for (hook = bus_info.hooks; hook; hook = hook->next) {
if (hook->connect)
diff --git a/config/udev.c b/config/udev.c
index 01e8293..9f008d1 100644
--- a/config/udev.c
+++ b/config/udev.c
@@ -259,11 +259,6 @@ wakeup_handler(pointer data, int err, pointer read_mask)
}
}
-static void
-block_handler(pointer data, struct timeval **tv, pointer read_mask)
-{
-}
-
int
config_udev_init(void)
{
@@ -296,7 +291,7 @@ config_udev_init(void)
}
udev_enumerate_unref(enumerate);
- RegisterBlockAndWakeupHandlers(block_handler, wakeup_handler, NULL);
+ RegisterBlockAndWakeupHandlers(NULL, wakeup_handler, NULL);
AddGeneralSocket(udev_monitor_get_fd(udev_monitor));
return 1;
@@ -313,7 +308,7 @@ config_udev_fini(void)
udev = udev_monitor_get_udev(udev_monitor);
RemoveGeneralSocket(udev_monitor_get_fd(udev_monitor));
- RemoveBlockAndWakeupHandlers(block_handler, wakeup_handler, udev_monitor);
+ RemoveBlockAndWakeupHandlers(NULL, wakeup_handler, udev_monitor);
udev_monitor_unref(udev_monitor);
udev_monitor = NULL;
udev_unref(udev);
diff --git a/dix/dixutils.c b/dix/dixutils.c
index 104363b..33cb24b 100644
--- a/dix/dixutils.c
+++ b/dix/dixutils.c
@@ -386,8 +386,9 @@ BlockHandler(pointer pTimeout, pointer pReadmask)
screenInfo.screens[i]->blockData,
pTimeout, pReadmask);
for (i = 0; i < numHandlers; i++)
- (*handlers[i].BlockHandler) (handlers[i].blockData,
- pTimeout, pReadmask);
+ if (handlers[i].BlockHandler)
+ (*handlers[i].BlockHandler) (handlers[i].blockData,
+ pTimeout, pReadmask);
if (handlerDeleted)
{
for (i = 0; i < numHandlers;)
@@ -416,8 +417,9 @@ WakeupHandler(int result, pointer pReadmask)
++inHandler;
for (i = numHandlers - 1; i >= 0; i--)
- (*handlers[i].WakeupHandler) (handlers[i].blockData,
- result, pReadmask);
+ if (handlers[i].WakeupHandler)
+ (*handlers[i].WakeupHandler) (handlers[i].blockData,
+ result, pReadmask);
for (i = 0; i < screenInfo.numScreens; i++)
(* screenInfo.screens[i]->WakeupHandler)(i,
screenInfo.screens[i]->wakeupData,
diff --git a/hw/dmx/dmxsync.c b/hw/dmx/dmxsync.c
index 2c7ccb8..af268dc 100644
--- a/hw/dmx/dmxsync.c
+++ b/hw/dmx/dmxsync.c
@@ -99,11 +99,6 @@ static void dmxSyncBlockHandler(pointer blockData, OSTimePtr pTimeout,
TimerForce(dmxSyncTimer);
}
-static void dmxSyncWakeupHandler(pointer blockData, int result,
- pointer pReadMask)
-{
-}
-
/** Request the XSync() batching optimization with the specified \a
* interval (in mS). If the \a interval is 0, 100mS is used. If the \a
* interval is less than 0, then the XSync() batching optimization is
@@ -124,9 +119,7 @@ void dmxSyncActivate(const char *interval)
void dmxSyncInit(void)
{
if (dmxSyncInterval) {
- RegisterBlockAndWakeupHandlers(dmxSyncBlockHandler,
- dmxSyncWakeupHandler,
- NULL);
+ RegisterBlockAndWakeupHandlers(dmxSyncBlockHandler, NULL, NULL);
dmxLog(dmxInfo, "XSync batching with %d ms interval\n",
dmxSyncInterval);
} else {
diff --git a/hw/kdrive/ephyr/ephyr.c b/hw/kdrive/ephyr/ephyr.c
index 8096a24..27cc384 100644
--- a/hw/kdrive/ephyr/ephyr.c
+++ b/hw/kdrive/ephyr/ephyr.c
@@ -376,12 +376,6 @@ ephyrInternalDamageBlockHandler (pointer data,
ephyrInternalDamageRedisplay (pScreen);
}
-static void
-ephyrInternalDamageWakeupHandler (pointer data, int i, pointer LastSelectMask)
-{
- /* FIXME: Not needed ? */
-}
-
Bool
ephyrSetInternalDamage (ScreenPtr pScreen)
{
@@ -397,9 +391,8 @@ ephyrSetInternalDamage (ScreenPtr pScreen)
pScreen,
pScreen);
- if (!RegisterBlockAndWakeupHandlers (ephyrInternalDamageBlockHandler,
- ephyrInternalDamageWakeupHandler,
- (pointer) pScreen))
+ if (!RegisterBlockAndWakeupHandlers (ephyrInternalDamageBlockHandler, NULL,
+ pScreen))
return FALSE;
pPixmap = (*pScreen->GetScreenPixmap) (pScreen);
@@ -422,8 +415,8 @@ ephyrUnsetInternalDamage (ScreenPtr pScreen)
DamageDestroy (scrpriv->pDamage);
RemoveBlockAndWakeupHandlers (ephyrInternalDamageBlockHandler,
- ephyrInternalDamageWakeupHandler,
- (pointer) pScreen);
+ NULL,
+ pScreen);
}
#ifdef RANDR
diff --git a/hw/kdrive/linux/linux.c b/hw/kdrive/linux/linux.c
index 9863c14..716c751 100644
--- a/hw/kdrive/linux/linux.c
+++ b/hw/kdrive/linux/linux.c
@@ -179,11 +179,6 @@ LinuxSetSwitchMode (int mode)
}
}
-static void
-LinuxApmBlock (pointer blockData, OSTimePtr pTimeout, pointer pReadmask)
-{
-}
-
static Bool LinuxApmRunning;
static void
@@ -258,7 +253,7 @@ LinuxEnable (void)
{
LinuxApmRunning = TRUE;
fcntl (LinuxApmFd, F_SETFL, fcntl (LinuxApmFd, F_GETFL) | NOBLOCK);
- RegisterBlockAndWakeupHandlers (LinuxApmBlock, LinuxApmWakeup, 0);
+ RegisterBlockAndWakeupHandlers (NULL, LinuxApmWakeup, NULL);
AddEnabledDevice (LinuxApmFd);
}
@@ -294,7 +289,7 @@ LinuxDisable (void)
enabled = FALSE;
if (LinuxApmFd >= 0)
{
- RemoveBlockAndWakeupHandlers (LinuxApmBlock, LinuxApmWakeup, 0);
+ RemoveBlockAndWakeupHandlers (NULL, LinuxApmWakeup, 0);
RemoveEnabledDevice (LinuxApmFd);
close (LinuxApmFd);
LinuxApmFd = -1;
diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c
index 53f82f9..6892ba2 100644
--- a/hw/vfb/InitOutput.c
+++ b/hw/vfb/InitOutput.c
@@ -548,12 +548,6 @@ vfbBlockHandler(pointer blockData, OSTimePtr pTimeout, pointer pReadmask)
static void
-vfbWakeupHandler(pointer blockData, int result, pointer pReadmask)
-{
-}
-
-
-static void
vfbAllocateMmappedFramebuffer(vfbScreenInfoPtr pvfb)
{
#define DUMMY_BUFFER_SIZE 65536
@@ -599,8 +593,7 @@ vfbAllocateMmappedFramebuffer(vfbScreenInfoPtr pvfb)
return;
}
- if (!RegisterBlockAndWakeupHandlers(vfbBlockHandler, vfbWakeupHandler,
- NULL))
+ if (!RegisterBlockAndWakeupHandlers(vfbBlockHandler, NULL, NULL))
{
pvfb->pXWDHeader = NULL;
}
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index a1fda54..d5be3cc 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -787,8 +787,7 @@ InitOutput(ScreenInfo *pScreenInfo, int argc, char **argv)
xf86Resetting = FALSE;
xf86Initialising = FALSE;
- RegisterBlockAndWakeupHandlers((BlockHandlerProcPtr)NoopDDA, xf86Wakeup,
- NULL);
+ RegisterBlockAndWakeupHandlers(NULL, xf86Wakeup, NULL);
}
/*
diff --git a/miext/rootless/rootlessScreen.c b/miext/rootless/rootlessScreen.c
index 61d2f5d..b6886cb 100644
--- a/miext/rootless/rootlessScreen.c
+++ b/miext/rootless/rootlessScreen.c
@@ -618,14 +618,6 @@ RootlessBlockHandler(pointer pbdata, OSTimePtr pTimeout, pointer pReadmask)
}
}
-
-static void
-RootlessWakeupHandler(pointer data, int i, pointer LastSelectMask)
-{
- // nothing here
-}
-
-
static Bool
RootlessAllocatePrivates(ScreenPtr pScreen)
{
@@ -728,9 +720,7 @@ Bool RootlessInit(ScreenPtr pScreen, RootlessFrameProcsPtr procs)
RootlessWrap(pScreen);
- if (!RegisterBlockAndWakeupHandlers(RootlessBlockHandler,
- RootlessWakeupHandler,
- (pointer) pScreen))
+ if (!RegisterBlockAndWakeupHandlers(RootlessBlockHandler, NULL, pScreen))
{
return FALSE;
}
diff --git a/miext/shadow/shadow.c b/miext/shadow/shadow.c
index cb1b299..905a52d 100644
--- a/miext/shadow/shadow.c
+++ b/miext/shadow/shadow.c
@@ -72,11 +72,6 @@ shadowBlockHandler(pointer data, OSTimePtr pTimeout, pointer pRead)
}
static void
-shadowWakeupHandler(pointer data, int i, pointer LastSelectMask)
-{
-}
-
-static void
shadowGetImage(DrawablePtr pDrawable, int sx, int sy, int w, int h,
unsigned int format, unsigned long planeMask, char *pdstLine)
{
@@ -182,8 +177,7 @@ shadowAdd(ScreenPtr pScreen, PixmapPtr pPixmap, ShadowUpdateProc update,
{
shadowBuf(pScreen);
- if (!RegisterBlockAndWakeupHandlers(shadowBlockHandler, shadowWakeupHandler,
- (pointer)pScreen))
+ if (!RegisterBlockAndWakeupHandlers(shadowBlockHandler, NULL, pScreen))
return FALSE;
/*
@@ -227,8 +221,7 @@ shadowRemove(ScreenPtr pScreen, PixmapPtr pPixmap)
pBuf->pPixmap = 0;
}
- RemoveBlockAndWakeupHandlers(shadowBlockHandler, shadowWakeupHandler,
- (pointer) pScreen);
+ RemoveBlockAndWakeupHandlers(shadowBlockHandler, NULL, pScreen);
}
Bool
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20101230/c647bfe0/attachment.pgp>
More information about the xorg-devel
mailing list