Revision: 14427
Author: adrian.chadd
Date: Sat Feb 20 05:55:18 2010
Log: First phase of the store client reworking!
This commit changes the lifecycle of the store client code somewhat.
The basic idea is to keep the store client around long enough to
satisfy any outstanding disk IO requests. This will then allow the
async io to use an intermediary copy.
This code doesn't quite yet fully work - it works in the normal case,
but aborted/failed disk IO seems to tickle the completion paths
to be called and unwound in very bizarre methods.
Known TODO items:
* storeClientUnregister() is called in places where disk IO failed
via the store IO "close" path, initiated by the store disk IO
layer (ie, "aufs") when the disk IO fails. That needs to be
fixed somehow.
http://code.google.com/p/lusca-cache/source/detail?r=14427
Modified:
/playpen/LUSCA_HEAD_zerocopy_storeread/src/protos.h
/playpen/LUSCA_HEAD_zerocopy_storeread/src/store_client.c
/playpen/LUSCA_HEAD_zerocopy_storeread/src/store_swapin.c
/playpen/LUSCA_HEAD_zerocopy_storeread/src/structs.h
=======================================
--- /playpen/LUSCA_HEAD_zerocopy_storeread/src/protos.h Sat Feb 20 04:40:02
2010
+++ /playpen/LUSCA_HEAD_zerocopy_storeread/src/protos.h Sat Feb 20 05:55:18
2010
@@ -761,7 +761,7 @@
extern squid_off_t storeLowestMemReaderOffset(const StoreEntry * entry);
extern void InvokeHandlers(StoreEntry * e);
extern int storePendingNClients(const StoreEntry * e);
-
+extern int storeClientComplete(store_client *sc);
extern const char *getMyHostname(void);
extern const char *uniqueHostname(void);
=======================================
--- /playpen/LUSCA_HEAD_zerocopy_storeread/src/store_client.c Tue Apr 21
01:37:50 2009
+++ /playpen/LUSCA_HEAD_zerocopy_storeread/src/store_client.c Sat Feb 20
05:55:18 2010
@@ -215,6 +215,7 @@
sc->seen_offset = 0;
sc->copy_offset = 0;
sc->flags.disk_io_pending = 0;
+ sc->flags.active = 1;
sc->entry = e;
storeLockObject(sc->entry);
sc->type = storeClientType(e);
@@ -254,6 +255,7 @@
mem_node_ref nr;
assert(sc->new_callback);
+ assert(sc->flags.active);
sc->new_callback = NULL;
sc->callback_data = NULL;
nr = sc->node_ref; /* XXX this should be a reference; and we should
dereference our copy! */
@@ -292,6 +294,17 @@
{
store_client *sc = data;
debug(20, 3) ("storeClientCopyEvent: Running\n");
+
+ /* Don't run the event if its meant to be freed at this point */
+ if (storeClientComplete(sc)) {
+ return;
+ }
+ /* The event wasn't freed; don't run it if its not active */
+ if (! sc->flags.active) {
+ debug(1, 1) ("storeClientCopyEvent: %p: not active, skipping\n",
sc);
+ return;
+ }
+
sc->flags.copy_event_pending = 0;
if (!sc->new_callback)
return;
@@ -345,6 +358,7 @@
#endif
assert(sc->new_callback == NULL);
assert(sc->entry == e);
+ assert(sc->flags.active);
sc->seen_offset = seen_offset;
sc->new_callback = callback;
sc->callback_data = data;
@@ -411,6 +425,7 @@
{
if (sc->flags.copy_event_pending)
return;
+ assert(sc->flags.active);
if (EBIT_TEST(e->flags, ENTRY_FWD_HDR_WAIT)) {
debug(20, 5) ("storeClientCopy2: returning because ENTRY_FWD_HDR_WAIT
set\n");
return;
@@ -456,6 +471,7 @@
MemObject *mem = e->mem_obj;
ssize_t sz = -1;
+ assert(sc->flags.active);
if (storeClientNoMoreToSend(e, sc)) {
/* There is no more to send! */
storeClientCallback(sc, 0);
@@ -535,6 +551,7 @@
MemObject *mem = sc->entry->mem_obj;
assert(sc->new_callback);
assert(!sc->flags.disk_io_pending);
+ assert(sc->flags.active);
sc->flags.disk_io_pending = 1;
assert(sc->node_ref.node == NULL); /* We should never, ever have a
node here; or we'd leak! */
stmemNodeRefCreate(&sc->node_ref); /* Creates an entry with reference
count == 1 */
@@ -580,6 +597,17 @@
sc->flags.disk_io_pending = 0;
assert(sc->new_callback);
assert(sc->node_ref.node);
+
+ /* Don't run the event if its meant to be freed at this point */
+ if (storeClientComplete(sc)) {
+ debug(1, 1) ("storeClientReadBody: %p: completed/freed,
skipping\n", sc);
+ return;
+ }
+ if (! sc->flags.active) {
+ debug(1, 1) ("storeClientReadBody: %p: not active, skipping\n",
sc);
+ return;
+ }
+
cbuf = sc->node_ref.node->data;
/* XXX update how much data in that mem page is active; argh this
should be done in a storage layer */
sc->node_ref.node->len = len;
@@ -608,6 +636,17 @@
sc->flags.disk_io_pending = 0;
assert(sc->new_callback);
assert(sc->node_ref.node);
+
+ /* Don't run the event if its meant to be freed at this point */
+ if (storeClientComplete(sc)) {
+ debug(1, 1) ("storeClientReadHeader: %p: completed/freed,
skipping\n", sc);
+ return;
+ }
+ if (! sc->flags.active) {
+ debug(1, 1) ("storeClientReadHeader: %p: not active, skipping\n",
sc);
+ return;
+ }
+
cbuf = sc->node_ref.node->data;
debug(20, 3) ("storeClientReadHeader: len %d\n", (int) len);
/* XXX update how much data in that mem page is active; argh this
should be done in a storage layer */
@@ -762,6 +801,31 @@
return 0;
return 1;
}
+
+/*
+ * Check whether the store client should be going away.
+ *
+ * This should be called by each function which is used as
+ * a callback return point.
+ *
+ * Return 1 if the store client has indeed been freed, and 0
+ * if is not yet finished doing what its supposed to be
+ * doing.
+ */
+int
+storeClientComplete(store_client *sc)
+{
+ if (sc->flags.active)
+ return 0;
+ if (sc->flags.disk_io_pending)
+ return 0;
+ if (sc->flags.copy_event_pending)
+ return 0;
+
+ stmemNodeUnref(&sc->node_ref);
+ cbdataFree(sc);
+ return 1;
+}
/*!
* @function
@@ -809,16 +873,26 @@
mem->url);
storeClientCallback(sc, -1);
}
- stmemNodeUnref(&sc->node_ref);
#if DELAY_POOLS
delayUnregisterDelayIdPtr(&sc->delay_id);
#endif
storeSwapOutMaintainMemObject(e);
+
+ /* This is in storeClientDeallocate() now */
+ /* stmemNodeUnref(&sc->node_ref); */
+
+ if (sc->flags.disk_io_pending)
+ debug(1, 1) ("storeClientUnregister: %p: unregistering client with
pending disk read!\n", sc);
+ if (sc->flags.copy_event_pending)
+ debug(1, 1) ("storeClientUnregister: %p: unregistering client with
pending copy event!\n", sc);
+
if (mem->nclients == 0)
CheckQuickAbort(e);
storeUnlockObject(sc->entry);
sc->entry = NULL;
- cbdataFree(sc);
+ sc->flags.active = 0;
+ storeClientComplete(sc);
+ /* sc may be invalid at this point */
return 1;
}
@@ -956,6 +1030,9 @@
assert(cb);
assert(cbdata);
+ /* The event should not be run with the active flag set */
+ assert(sc->flags.active);
+
/* Leave these in for now, just for debugging */
#if 0
sc->header_callback = NULL;
@@ -980,6 +1057,9 @@
sc->header_callback = callback;
sc->header_cbdata = callback_data;
+ /* The event should not be run with the active flag set */
+ assert(sc->flags.active);
+
/* This kicks off either the memory read, waiting for the data to
appear, or the disk read */
storeClientRef(sc, e, 0, 0, SM_PAGE_SIZE, storeClientCopyHeadersCB,
sc);
}
=======================================
--- /playpen/LUSCA_HEAD_zerocopy_storeread/src/store_swapin.c Sun Dec 23
03:32:11 2007
+++ /playpen/LUSCA_HEAD_zerocopy_storeread/src/store_swapin.c Sat Feb 20
05:55:18 2010
@@ -43,6 +43,7 @@
{
StoreEntry *e = sc->entry;
assert(e->mem_status == NOT_IN_MEMORY);
+ assert(sc->flags.active);
if (!EBIT_TEST(e->flags, ENTRY_VALIDATED)) {
/* We're still reloading and haven't validated this entry yet */
return;
@@ -73,6 +74,16 @@
store_client *sc = data;
debug(20, 3) ("storeSwapInFileClosed: sio=%p, errflag=%d\n",
sio, errflag);
+
+ if (storeClientComplete(sc)) {
+ return;
+ }
+ /* The event wasn't freed; don't run it if its not active */
+ if (! sc->flags.active) {
+ debug(1, 1) ("storeSwapInFileClosed: %p: not active, skipping\n",
sc);
+ return;
+ }
+
cbdataUnlock(sio);
sc->swapin_sio = NULL;
if (errflag < 0)
@@ -101,6 +112,15 @@
store_client *sc = data;
StoreEntry *e = sc->entry;
+ if (storeClientComplete(sc)) {
+ return;
+ }
+ /* The event wasn't freed; don't run it if its not active */
+ if (! sc->flags.active) {
+ debug(1, 1) ("storeSwapInFileNotify: %p: not active, skipping\n",
sc);
+ return;
+ }
+
debug(1, 3) ("storeSwapInFileNotify: changing %d/%d to %d/%d\n",
e->swap_filen, e->swap_dirn, sio->swap_filen, sio->swap_dirn);
e->swap_filen = sio->swap_filen;
=======================================
--- /playpen/LUSCA_HEAD_zerocopy_storeread/src/structs.h Sat Feb 20
04:40:02 2010
+++ /playpen/LUSCA_HEAD_zerocopy_storeread/src/structs.h Sat Feb 20
05:55:18 2010
@@ -1428,6 +1428,7 @@
unsigned int disk_io_pending:1;
unsigned int store_copying:1;
unsigned int copy_event_pending:1;
+ unsigned int active:1;
} flags;
#if DELAY_POOLS
delay_id delay_id;
--
You received this message because you are subscribed to the Google Groups
"lusca-commit" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/lusca-commit?hl=en.
