--------------------- PatchSet 10031 Date: 2007/09/25 00:29:27 Author: adri Branch: store_copy Tag: (none) Log: * Add in checks for new_callback where callback is checked; its used in a variety of places in the store client codebase as a check for a pending store copy. * Bring in using storeClientRef() (which hm, I think I might rename to something more clear in a moment) into the client-side code, but only in the clientSendMoreData() path. * A slight modification to the storeClientRef() code to find the first page to copy from - it was finding the page -before- and then bombing out with 0 bytes to copy. Hm! * Change the storeClientRef() logic a little for determining offsets and data lengths. Ugly, but it seems to work. * Back out setting the node_ref stuff to null in storeClientRef() and storeClientCopy() just in case it was overwriting a pending invocation and messing with reference counting. I will fix this later when i re-introduce some better mem_node_ref functions to handle referencing "right". * Small modifications to clientBodyWriteComplete() and clientHttpRequest to persist the mem_node_ref if required when writing the client body without ranges. This also maintains my older "don't copy" work there. * Its possible stmemUnref() is called on an unref'ed mem_node_ref thats been NULLed; handle this gracefully. This code has only had some very cursory testing, so be warned! Members: src/client_side.c:1.202->1.202.2.1 src/protos.h:1.146.2.1->1.146.2.2 src/stmem.c:1.10.2.2->1.10.2.3 src/store_client.c:1.25.10.2->1.25.10.3 src/structs.h:1.158.2.1->1.158.2.2 src/typedefs.h:1.43.2.1->1.43.2.2 Index: squid/src/client_side.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/client_side.c,v retrieving revision 1.202 retrieving revision 1.202.2.1 diff -u -r1.202 -r1.202.2.1 --- squid/src/client_side.c 24 Sep 2007 13:52:06 -0000 1.202 +++ squid/src/client_side.c 25 Sep 2007 00:29:27 -0000 1.202.2.1 @@ -1,6 +1,6 @@ /* - * $Id: client_side.c,v 1.202 2007/09/24 13:52:06 squidadm Exp $ + * $Id: client_side.c,v 1.202.2.1 2007/09/25 00:29:27 adri Exp $ * * DEBUG: section 33 Client-side Routines * AUTHOR: Duane Wessels @@ -134,7 +134,7 @@ static void clientFollowXForwardedForDone(int answer, void *data); #endif /* FOLLOW_X_FORWARDED_FOR */ static int clientOnlyIfCached(clientHttpRequest * http); -static STCB clientSendMoreData; +static STNCB clientSendMoreData; static STCB clientSendMoreHeaderData; static STCB clientCacheHit; static void clientSetKeepaliveFlag(clientHttpRequest *); @@ -2886,10 +2886,10 @@ if (mb.size > 0) { comm_write_mbuf(http->conn->fd, mb, clientWriteComplete, http); } else { - storeClientCopy(http->sc, http->entry, + storeClientRef(http->sc, http->entry, http->out.offset, http->out.offset, - STORE_CLIENT_BUF_SZ, memAllocate(MEM_STORE_CLIENT_BUF), + STORE_CLIENT_BUF_SZ, clientSendMoreData, http); } @@ -2901,14 +2901,16 @@ * such, writes processed message to the client's socket */ static void -clientSendMoreData(void *data, char *buf, ssize_t size) +clientSendMoreData(void *data, mem_node_ref ref, ssize_t size) { + const char *buf = ref.node->data + ref.offset; clientHttpRequest *http = data; StoreEntry *entry = http->entry; ConnStateData *conn = http->conn; int fd = conn->fd; MemBuf mb; debug(33, 5) ("clientSendMoreData: %s, %d bytes\n", http->uri, (int) size); + assert(size + ref.offset <= STORE_CLIENT_BUF_SZ); assert(size <= STORE_CLIENT_BUF_SZ); assert(http->request != NULL); dlinkDelete(&http->active, &ClientActiveRequests); @@ -2919,27 +2921,30 @@ if (DLINK_HEAD(conn->reqs) != http) { /* there is another object in progress, defer this one */ debug(33, 1) ("clientSendMoreData: Deferring %s\n", storeUrl(entry)); - memFree(buf, MEM_STORE_CLIENT_BUF); + storeClientDeref(&ref); return; } else if (entry && EBIT_TEST(entry->flags, ENTRY_ABORTED)) { /* call clientWriteComplete so the client socket gets closed */ clientWriteComplete(fd, NULL, 0, COMM_OK, http); - memFree(buf, MEM_STORE_CLIENT_BUF); + storeClientDeref(&ref); return; } else if (size < 0) { /* call clientWriteComplete so the client socket gets closed */ clientWriteComplete(fd, NULL, 0, COMM_OK, http); - memFree(buf, MEM_STORE_CLIENT_BUF); + storeClientDeref(&ref); return; } else if (size == 0) { /* call clientWriteComplete so the client socket gets closed */ clientWriteComplete(fd, NULL, 0, COMM_OK, http); - memFree(buf, MEM_STORE_CLIENT_BUF); + storeClientDeref(&ref); return; } + /* XXX restore this optimisation later on! */ if (!http->request->range) { /* Avoid copying to MemBuf for non-range requests */ http->out.offset += size; + /* XXX eww - these refcounting semantics should be better adrian! fix it! */ + http->nr = ref; comm_write(fd, buf, size, clientWriteBodyComplete, http, NULL); /* NULL because clientWriteBodyComplete frees it */ return; @@ -2975,7 +2980,7 @@ } /* write body */ comm_write_mbuf(fd, mb, clientWriteComplete, http); - memFree(buf, MEM_STORE_CLIENT_BUF); + storeClientDeref(&ref); } /* @@ -2987,11 +2992,12 @@ static void clientWriteBodyComplete(int fd, char *buf, size_t size, int errflag, void *data) { + clientHttpRequest *http = data; /* * NOTE: clientWriteComplete doesn't currently use its "buf" * (second) argument, so we pass in NULL. */ - memFree(buf, MEM_STORE_CLIENT_BUF); + storeClientDeref(&http->nr); clientWriteComplete(fd, NULL, size, errflag, data); } @@ -3124,10 +3130,11 @@ * storage manager. */ if (EBIT_TEST(entry->flags, ENTRY_ABORTED)) debug(33, 0) ("clientWriteComplete 2: ENTRY_ABORTED\n"); - storeClientCopy(http->sc, entry, + debug(33, 3) ("clientWriteComplete: copying from offset %d\n", (int) http->out.offset); + storeClientRef(http->sc, entry, http->out.offset, http->out.offset, - STORE_CLIENT_BUF_SZ, memAllocate(MEM_STORE_CLIENT_BUF), + STORE_CLIENT_BUF_SZ, clientSendMoreData, http); } Index: squid/src/protos.h =================================================================== RCS file: /cvsroot/squid-sf//squid/src/protos.h,v retrieving revision 1.146.2.1 retrieving revision 1.146.2.2 diff -u -r1.146.2.1 -r1.146.2.2 --- squid/src/protos.h 24 Sep 2007 14:29:37 -0000 1.146.2.1 +++ squid/src/protos.h 25 Sep 2007 00:29:27 -0000 1.146.2.2 @@ -1,6 +1,6 @@ /* - * $Id: protos.h,v 1.146.2.1 2007/09/24 14:29:37 adri Exp $ + * $Id: protos.h,v 1.146.2.2 2007/09/25 00:29:27 adri Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -1087,7 +1087,7 @@ extern store_client *storeClientRegister(StoreEntry * e, void *data); extern void storeClientCopy(store_client *, StoreEntry *, squid_off_t, squid_off_t, size_t, char *, STCB *, void *); extern void storeClientRef(store_client *, StoreEntry *, squid_off_t, squid_off_t, size_t, STNCB *, void *); -extern void storeClientDeref(store_client *, mem_node_ref *r); +extern void storeClientDeref(mem_node_ref *r); extern int storeClientCopyPending(store_client *, StoreEntry * e, void *data); extern int storeClientUnregister(store_client * sc, StoreEntry * e, void *data); extern squid_off_t storeLowestMemReaderOffset(const StoreEntry * entry); Index: squid/src/stmem.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/stmem.c,v retrieving revision 1.10.2.2 retrieving revision 1.10.2.3 diff -u -r1.10.2.2 -r1.10.2.3 --- squid/src/stmem.c 24 Sep 2007 14:37:14 -0000 1.10.2.2 +++ squid/src/stmem.c 25 Sep 2007 00:29:27 -0000 1.10.2.3 @@ -1,6 +1,6 @@ /* - * $Id: stmem.c,v 1.10.2.2 2007/09/24 14:37:14 adri Exp $ + * $Id: stmem.c,v 1.10.2.3 2007/09/25 00:29:27 adri Exp $ * * DEBUG: section 19 Store Memory Primitives * AUTHOR: Harvest Derived @@ -155,14 +155,13 @@ stmemRef(const mem_hdr *mem, squid_off_t offset, mem_node_ref *r) { mem_node *p = mem->head; - squid_off_t t_off = mem->origin_offset; - int bytes_into_this_packet = 0; + volatile squid_off_t t_off = mem->origin_offset; - debug(19, 6) ("memRef: offset %" PRINTF_OFF_T "\n", offset); + debug(19, 3) ("memRef: offset %" PRINTF_OFF_T "; initial offset in memory %d\n", offset, (int) mem->origin_offset); if (p == NULL) return 0; /* Seek our way into store */ - while ((t_off + p->len) < offset) { + while ((t_off + p->len) <= offset) { t_off += p->len; if (!p->next) { debug(19, 1) ("memRef: p->next == NULL\n"); @@ -171,15 +170,21 @@ assert(p->next); p = p->next; } - bytes_into_this_packet = offset - t_off; r->node = (mem_node *) stmemNodeGet(p); - r->offset = bytes_into_this_packet; - return p->len - bytes_into_this_packet; + + r->offset = offset - t_off; + assert(r->offset >= 0); + assert(r->offset >= 0); + assert(p->len + t_off - offset > 0); + debug(19, 3) ("memRef: returning node %p, offset %d, %d bytes\n", p, (int) r->offset, (int) p->len + t_off - offset); + return p->len + t_off - offset; } void stmemUnref(mem_node_ref *r) { + if (! r->node) + return; stmemNodeFree((void *) r->node); r->node = NULL; r->offset = -1; Index: squid/src/store_client.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/store_client.c,v retrieving revision 1.25.10.2 retrieving revision 1.25.10.3 diff -u -r1.25.10.2 -r1.25.10.3 --- squid/src/store_client.c 24 Sep 2007 14:37:14 -0000 1.25.10.2 +++ squid/src/store_client.c 25 Sep 2007 00:29:27 -0000 1.25.10.3 @@ -1,6 +1,6 @@ /* - * $Id: store_client.c,v 1.25.10.2 2007/09/24 14:37:14 adri Exp $ + * $Id: store_client.c,v 1.25.10.3 2007/09/25 00:29:27 adri Exp $ * * DEBUG: section 20 Storage Manager Client-Side Interface * AUTHOR: Duane Wessels @@ -153,7 +153,7 @@ if (callback) callback(cbdata, buf, sz); else if (new_callback) - new_callback(cbdata, nr); + new_callback(cbdata, nr, sz); else fatal("storeClientCallback: neither callback handler set!\n"); } @@ -166,7 +166,7 @@ store_client *sc = data; debug(20, 3) ("storeClientCopyEvent: Running\n"); sc->flags.copy_event_pending = 0; - if (!sc->callback) + if ((!sc->callback) && (!sc->new_callback)) return; storeClientCopy2(sc->entry, sc); } @@ -205,8 +205,6 @@ sc->copy_buf = NULL; sc->copy_size = size; sc->copy_offset = copy_offset; - sc->node_ref.node = NULL; - sc->node_ref.offset = -1; /* If the read is being deferred, run swapout in case this client has the * lowest seen_offset. storeSwapOut() frees the memory and clears the * ENTRY_DEFER_READ bit if necessary */ @@ -249,8 +247,6 @@ sc->copy_buf = buf; sc->copy_size = size; sc->copy_offset = copy_offset; - sc->node_ref.node = NULL; - sc->node_ref.offset = -1; /* If the read is being deferred, run swapout in case this client has the * lowest seen_offset. storeSwapOut() frees the memory and clears the * ENTRY_DEFER_READ bit if necessary */ @@ -300,7 +296,9 @@ cbdataLock(sc); /* ick, prevent sc from getting freed */ sc->flags.store_copying = 1; debug(20, 3) ("storeClientCopy2: %s\n", storeKeyText(e->hash.key)); - assert(sc->callback != NULL); + if ( (!sc->callback) && (!sc->new_callback)) { + fatal("storeClientCopy2: missing a callback!\n"); + } /* * We used to check for ENTRY_ABORTED here. But there were some * problems. For example, we might have a slow client (or two) and @@ -562,7 +560,7 @@ assert(sc->entry == e); if (sc == NULL) return 0; - if (sc->callback == NULL) + if (sc->callback == NULL && sc->new_callback == NULL) return 0; return 1; } @@ -594,7 +592,7 @@ sc->swapin_sio = NULL; statCounter.swap.ins++; } - if (NULL != sc->callback) { + if (NULL != sc->callback || NULL != sc->new_callback) { /* callback with ssize = -1 to indicate unexpected termination */ debug(20, 3) ("storeClientUnregister: store_client for %s has a callback\n", mem->url); @@ -653,7 +651,7 @@ sc = node->data; nx = node->next; debug(20, 3) ("InvokeHandlers: checking client #%d\n", i++); - if (sc->callback == NULL) + if (sc->callback == NULL && sc->new_callback == NULL) continue; if (sc->flags.disk_io_pending) continue; @@ -737,7 +735,7 @@ } void -storeClientDeref(store_client *sc, mem_node_ref *r) +storeClientDeref(mem_node_ref *r) { stmemUnref(r); } Index: squid/src/structs.h =================================================================== RCS file: /cvsroot/squid-sf//squid/src/structs.h,v retrieving revision 1.158.2.1 retrieving revision 1.158.2.2 diff -u -r1.158.2.1 -r1.158.2.2 --- squid/src/structs.h 24 Sep 2007 14:29:38 -0000 1.158.2.1 +++ squid/src/structs.h 25 Sep 2007 00:29:27 -0000 1.158.2.2 @@ -1,6 +1,6 @@ /* - * $Id: structs.h,v 1.158.2.1 2007/09/24 14:29:38 adri Exp $ + * $Id: structs.h,v 1.158.2.2 2007/09/25 00:29:27 adri Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -1237,6 +1237,7 @@ } redirect; dlink_node active; squid_off_t maxBodySize; + mem_node_ref nr; }; struct _ConnStateData { Index: squid/src/typedefs.h =================================================================== RCS file: /cvsroot/squid-sf//squid/src/typedefs.h,v retrieving revision 1.43.2.1 retrieving revision 1.43.2.2 diff -u -r1.43.2.1 -r1.43.2.2 --- squid/src/typedefs.h 24 Sep 2007 14:29:38 -0000 1.43.2.1 +++ squid/src/typedefs.h 25 Sep 2007 00:29:28 -0000 1.43.2.2 @@ -1,6 +1,6 @@ /* - * $Id: typedefs.h,v 1.43.2.1 2007/09/24 14:29:38 adri Exp $ + * $Id: typedefs.h,v 1.43.2.2 2007/09/25 00:29:28 adri Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -288,7 +288,7 @@ typedef void SIH(storeIOState *, void *); /* swap in */ typedef int QS(const void *, const void *); /* qsort */ typedef void STCB(void *, char *, ssize_t); /* store callback */ -typedef void STNCB(void *, struct _mem_node_ref r); /* new store callback */ +typedef void STNCB(void *, struct _mem_node_ref r, ssize_t); /* new store callback */ typedef void STABH(void *); typedef void ERCB(int fd, void *, size_t); typedef void OBJH(StoreEntry *);