--------------------- PatchSet 1381 Date: 2001/01/25 12:10:45 Author: adri Branch: sfs Tag: (none) Log: Tidy up the request list code. * turn request_queue into a dlink_list * remove sfs_requestor_list * add a dlink_node to sfs_requestor * Kill the list walks that are being done when we insert the request into the request_queue. This makes the code a lot simpler and should run a bit faster when under load. It also means I can add a "done list" for completed requests. Members: src/fs/sfs/sfs_defines.h:1.1.2.2->1.1.2.3 src/fs/sfs/sfs_interface.c:1.1.2.3->1.1.2.4 src/fs/sfs/sfs_lib.h:1.1.2.2->1.1.2.3 src/fs/sfs/sfs_llo.c:1.1.2.1->1.1.2.2 src/fs/sfs/sfs_util.c:1.1.2.3->1.1.2.4 Index: squid/src/fs/sfs/sfs_defines.h =================================================================== RCS file: /cvsroot/squid-sf//squid/src/fs/sfs/Attic/sfs_defines.h,v retrieving revision 1.1.2.2 retrieving revision 1.1.2.3 diff -u -r1.1.2.2 -r1.1.2.3 --- squid/src/fs/sfs/sfs_defines.h 24 Jan 2001 16:06:38 -0000 1.1.2.2 +++ squid/src/fs/sfs/sfs_defines.h 25 Jan 2001 12:10:45 -0000 1.1.2.3 @@ -1,4 +1,4 @@ -/* $Id: sfs_defines.h,v 1.1.2.2 2001/01/24 16:06:38 adri Exp $ */ +/* $Id: sfs_defines.h,v 1.1.2.3 2001/01/25 12:10:45 adri Exp $ */ #ifndef SFS_DEFINES_H #define SFS_DEFINES_H @@ -72,6 +72,7 @@ }; typedef struct sfs_requestor { + dlink_node node; /* List position */ enum sfs_request_type request_type; enum sfs_request_state request_state; pthread_cond_t done_signal; @@ -86,12 +87,6 @@ int ret; } sfs_requestor; -typedef struct sfs_requestor_list { - struct sfs_requestor_list *prev; - struct sfs_requestor_list *next; - struct sfs_requestor *req; -} sfs_requestor_list; - /* This corresponds to the structure as it's stored on disk. */ typedef struct sfs_inode_t { sfsblock_t len; @@ -179,7 +174,7 @@ sfs_blockbuf_t *tail[2]; /* list of blocks */ int accepting_requests; int pending_requests; - sfs_requestor_list *request_queue; + dlink_list request_queue; pthread_mutex_t req_lock; pthread_cond_t req_signal; pthread_mutex_t req_signal_lock; Index: squid/src/fs/sfs/sfs_interface.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/fs/sfs/Attic/sfs_interface.c,v retrieving revision 1.1.2.3 retrieving revision 1.1.2.4 diff -u -r1.1.2.3 -r1.1.2.4 --- squid/src/fs/sfs/sfs_interface.c 24 Jan 2001 16:40:01 -0000 1.1.2.3 +++ squid/src/fs/sfs/sfs_interface.c 25 Jan 2001 12:10:45 -0000 1.1.2.4 @@ -314,7 +314,8 @@ CBIT_SET(_sfs_mounted[i].mhb, j); } _sfs_mounted[i].dirty = NULL; - _sfs_mounted[i].request_queue = NULL; + _sfs_mounted[i].request_queue.head = NULL; + _sfs_mounted[i].request_queue.tail = NULL; _sfs_mounted[i].pending_requests = 0; pthread_mutex_init(&(_sfs_mounted[i].req_lock), NULL); pthread_mutex_init(&(_sfs_mounted[i].req_signal_lock), NULL); Index: squid/src/fs/sfs/sfs_lib.h =================================================================== RCS file: /cvsroot/squid-sf//squid/src/fs/sfs/Attic/sfs_lib.h,v retrieving revision 1.1.2.2 retrieving revision 1.1.2.3 diff -u -r1.1.2.2 -r1.1.2.3 --- squid/src/fs/sfs/sfs_lib.h 24 Jan 2001 15:34:16 -0000 1.1.2.2 +++ squid/src/fs/sfs/sfs_lib.h 25 Jan 2001 12:10:45 -0000 1.1.2.3 @@ -21,7 +21,7 @@ /* Internal functions */ /* sfs_util.c */ extern void _sfs_waitfor_request(sfs_requestor *req); -extern int _sfs_remove_request(sfs_requestor_list **lreq, sfs_requestor *req); +extern int _sfs_remove_request(dlink_list *list, sfs_requestor *req); extern void _sfs_done_request(sfs_requestor *req, int retval); extern sfs_requestor * _sfs_create_requestor(int sfsid, enum sfs_request_type reqtype); Index: squid/src/fs/sfs/sfs_llo.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/fs/sfs/Attic/sfs_llo.c,v retrieving revision 1.1.2.1 retrieving revision 1.1.2.2 diff -u -r1.1.2.1 -r1.1.2.2 --- squid/src/fs/sfs/sfs_llo.c 24 Jan 2001 14:11:54 -0000 1.1.2.1 +++ squid/src/fs/sfs/sfs_llo.c 25 Jan 2001 12:10:45 -0000 1.1.2.2 @@ -47,7 +47,8 @@ { sigset_t new; int i; - sfs_requestor_list *temp; + sfs_requestor *req; + dlink_node *tnode; /* Make sure to ignore signals which may possibly get sent to the parent */ /* squid thread. Causes havoc with mutex's and condition waits otherwise */ @@ -77,49 +78,51 @@ printf("DEBUG: Going into wait...\n"); pthread_cond_wait(&(mount_point->req_signal), &(mount_point->req_signal_lock)); printf("DEBUG: Coming out of wait...\n"); - temp = mount_point->request_queue; + tnode = mount_point->request_queue.head; + /* Should I lock the request queue while I cycle through these? Probably wise, but slow... */ - while ((mount_point->pending_requests > 0) && (temp)) { + while ((mount_point->pending_requests > 0) && (tnode != NULL)) { printf("DEBUG: %d pending requests\n",mount_point->pending_requests); - if (temp->req->request_state == _SFS_PENDING) { + req = tnode->data; + if (req->request_state == _SFS_PENDING) { /* If we're not accepting requests, return fail for each request. */ /* Note, we can't just lock the request queue, as things are still being removed from it by other threads. */ if (!(mount_point->accepting_requests)) { - _sfs_done_request(temp->req,-1); + _sfs_done_request(req,-1); } else { /* This portion sets the state, and works out exactly what to do - open, read, write, close, sync, unlink. */ - temp->req->request_state = _SFS_IN_PROGRESS; + req->request_state = _SFS_IN_PROGRESS; mount_point->pending_requests--; - switch (temp->req->request_type) { + switch (req->request_type) { case _SFS_OP_UNLINK: - sfs_do_unlink(temp->req); + sfs_do_unlink(req); break; case _SFS_OP_UMOUNT: - sfs_do_umount(temp->req); + sfs_do_umount(req); break; case _SFS_OP_READ: - sfs_do_read(temp->req); + sfs_do_read(req); break; case _SFS_OP_WRITE: - sfs_do_write(temp->req); + sfs_do_write(req); break; case _SFS_OP_OPEN_READ: case _SFS_OP_OPEN_WRITE: - sfs_do_open(temp->req); + sfs_do_open(req); break; case _SFS_OP_CLOSE: - sfs_do_close(temp->req); + sfs_do_close(req); break; default: - _sfs_done_request(temp->req,-1); + _sfs_done_request(req,-1); } } } - if (temp) - temp = temp->next; + if (tnode->next) + tnode = tnode->next; } i = (i + 1) % 10; if (i == 0) { @@ -268,7 +271,7 @@ { sfs_mountfs_t *mnt; sfs_blockbuf_t *block_ptr; - sfs_requestor_list *lreq; + dlink_node *lnode; sfs_openfile_t *openfd; int i; @@ -302,20 +305,21 @@ while (mnt->clean) { mnt->clean = sfs_splay_delete(req->sfsid, mnt->clean); } - if (mnt->request_queue) { + if (mnt->request_queue.head != NULL) { /* Make doubly sure we've cleared any pending requests - shouldn't need to, but we _are_ umounting... This is actually bodgy code, if it ever does anything, then something's gone wrong. Probably shouldn't do this, but it saves deadlock, and we'd rather see corruption than hang indefinately.*/ - lreq = mnt->request_queue; - while (lreq) { - if (lreq->req->request_state == _SFS_PENDING) { - _sfs_done_request(lreq->req,-1); + lnode = mnt->request_queue.head; + while (lnode) { + req = lnode->data; + if (req->request_state == _SFS_PENDING) { + _sfs_done_request(req,-1); } - lreq = lreq->next; + lnode = lnode->next; } i = 0; - while ((mnt->request_queue->next) && (i < 5)) { + while ((mnt->request_queue.head->next) && (i < 5)) { /* Waiting for the request queue to be empty of all bar the umount request */ i++; sleep(1); Index: squid/src/fs/sfs/sfs_util.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/fs/sfs/Attic/sfs_util.c,v retrieving revision 1.1.2.3 retrieving revision 1.1.2.4 diff -u -r1.1.2.3 -r1.1.2.4 --- squid/src/fs/sfs/sfs_util.c 24 Jan 2001 15:34:16 -0000 1.1.2.3 +++ squid/src/fs/sfs/sfs_util.c 25 Jan 2001 12:10:45 -0000 1.1.2.4 @@ -22,40 +22,21 @@ } int -_sfs_remove_request(sfs_requestor_list **lreq,sfs_requestor *req) +_sfs_remove_request(dlink_list *list, sfs_requestor *req) /* This doesn't free the buffer - not sure whether we have any need to keep the buffer anywhere or not, but the option is there... */ { - sfs_requestor_list *ptr, *x; - + /* + * We used to make sure that the request was actually *ON* the + * given list. This may be a good thing, but I've ripped it out + * for the time being. + */ printf("DEBUG: Removing %d request from queue\n",req->request_type); - ptr = *lreq; - while((ptr->next != NULL) && (ptr->req != req)) - ptr = ptr->next; - if (ptr->req == req) { - pthread_mutex_lock(&(_sfs_mounted[req->sfsid].req_lock)); - if (ptr->prev) - ptr->prev->next = ptr->next; - if (ptr->next) - ptr->next->prev = ptr->prev; -/* Could probably do this with a while over *lreq, actually */ - if (*lreq == ptr) { - x = ptr; - while (x->prev) - x = x->prev; - if (x == ptr) - *lreq = x->next; - else - *lreq = x; - } - pthread_mutex_unlock(&(_sfs_mounted[req->sfsid].req_lock)); - - xfree(req); - xfree(ptr); - return (0); - } else { - return -1; - } + pthread_mutex_lock(&(_sfs_mounted[req->sfsid].req_lock)); + dlinkDelete(&req->node, list); + pthread_mutex_unlock(&(_sfs_mounted[req->sfsid].req_lock)); + xfree(req); + return (0); } void @@ -97,27 +78,13 @@ int _sfs_submit_request(sfs_requestor *req) { - sfs_requestor_list *lreq; - sfs_requestor_list *ptr; - - if ((lreq =(sfs_requestor_list *) xcalloc(1, sizeof(sfs_requestor_list))) == NULL) - return -1; - - lreq->req = req; - lreq->next = NULL; printf("DEBUG: Locking req_lock\n"); pthread_mutex_lock(&(_sfs_mounted[req->sfsid].req_lock)); printf("DEBUG: Locked req_lock\n"); - ptr = _sfs_mounted[req->sfsid].request_queue; - if (ptr) { - while (ptr->next) - ptr = ptr->next; - ptr->next = lreq; - lreq->prev = ptr; - } else { - _sfs_mounted[req->sfsid].request_queue = lreq; - lreq->prev = NULL; - } + + /* add the request to the end of the list rather than the start */ + dlinkAddTail(req, &req->node, &_sfs_mounted[req->sfsid].request_queue); + _sfs_mounted[req->sfsid].pending_requests++; printf("DEBUG: Added %d request\n",req->request_type); pthread_mutex_unlock(&(_sfs_mounted[req->sfsid].req_lock));