From: hno <> Date: Sun, 10 Nov 2002 09:29:58 +0000 (+0000) Subject: date: 2002/11/09 10:42:01; author: hno; state: Exp; lines: +46 -20 X-Git-Tag: SQUID_3_0_PRE1~541 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=211f1d0bb2a4fef2a4a9c76361f7ec662effdf85;p=thirdparty%2Fsquid.git date: 2002/11/09 10:42:01; author: hno; state: Exp; lines: +46 -20 Bugzilla #451: Occasional corruption of objects This patch fixes a cbdata boundary violation in the aufs storeio implementation wich caused data corruption on aborted cache hits. --- diff --git a/src/fs/aufs/aiops.cc b/src/fs/aufs/aiops.cc index d47c14ce15..7929d6803b 100644 --- a/src/fs/aufs/aiops.cc +++ b/src/fs/aufs/aiops.cc @@ -1,5 +1,5 @@ /* - * $Id: aiops.cc,v 1.17 2002/11/09 09:35:26 hno Exp $ + * $Id: aiops.cc,v 1.18 2002/11/10 02:29:58 hno Exp $ * * DEBUG: section 43 AIOPS * AUTHOR: Stewart Forster @@ -172,7 +172,7 @@ squidaio_get_pool(int size) return p; } -static void * +void * squidaio_xmalloc(int size) { void *p; @@ -198,7 +198,7 @@ squidaio_xstrdup(const char *str) return p; } -static void +void squidaio_xfree(void *p, int size) { MemPool *pool; @@ -503,9 +503,6 @@ squidaio_cleanup_request(squidaio_request_t * requestp) squidaio_xstrfree(requestp->path); break; case _AIO_OP_READ: - if (!cancelled && requestp->ret > 0) - xmemcpy(requestp->bufferp, requestp->tmpbufp, requestp->ret); - squidaio_xfree(requestp->tmpbufp, requestp->buflen); break; case _AIO_OP_WRITE: squidaio_xfree(requestp->tmpbufp, requestp->buflen); @@ -578,7 +575,6 @@ squidaio_read(int fd, char *bufp, int bufs, off_t offset, int whence, squidaio_r requestp = (squidaio_request_t *)memPoolAlloc(squidaio_request_pool); requestp->fd = fd; requestp->bufferp = bufp; - requestp->tmpbufp = (char *) squidaio_xmalloc(bufs); requestp->buflen = bufs; requestp->offset = offset; requestp->whence = whence; @@ -596,7 +592,7 @@ static void squidaio_do_read(squidaio_request_t * requestp) { lseek(requestp->fd, requestp->offset, requestp->whence); - requestp->ret = read(requestp->fd, requestp->tmpbufp, requestp->buflen); + requestp->ret = read(requestp->fd, requestp->bufferp, requestp->buflen); requestp->err = errno; } diff --git a/src/fs/aufs/async_io.cc b/src/fs/aufs/async_io.cc index 144db3591f..8704362e2f 100644 --- a/src/fs/aufs/async_io.cc +++ b/src/fs/aufs/async_io.cc @@ -1,6 +1,6 @@ /* - * $Id: async_io.cc,v 1.16 2002/10/13 20:35:24 robertc Exp $ + * $Id: async_io.cc,v 1.17 2002/11/10 02:29:58 hno Exp $ * * DEBUG: section 32 Asynchronous Disk I/O * AUTHOR: Pete Bentley @@ -54,26 +54,27 @@ typedef struct squidaio_ctrl_t { AIOCB *done_handler; void *done_handler_data; squidaio_result_t result; + int len; char *bufp; FREE *free_func; dlink_node node; } squidaio_ctrl_t; static struct { - int open_start; - int open_finish; - int close_start; - int close_finish; - int cancel; - int write_start; - int write_finish; - int read_start; - int read_finish; - int stat_start; - int stat_finish; - int unlink_start; - int unlink_finish; - int check_callback; + int open_start; + int open_finish; + int close_start; + int close_finish; + int cancel; + int write_start; + int write_finish; + int read_start; + int read_finish; + int stat_start; + int stat_finish; + int unlink_start; + int unlink_finish; + int check_callback; } squidaio_counts; typedef struct squidaio_unlinkq_t { @@ -153,29 +154,35 @@ aioClose(int fd) void aioCancel(int fd) { - squidaio_ctrl_t *curr; + squidaio_ctrl_t *ctrlp; dlink_node *m, *next; assert(initialised); squidaio_counts.cancel++; for (m = used_list.head; m; m = next) { next = m->next; - curr = (squidaio_ctrl_t *)m->data; - if (curr->fd != fd) + ctrlp = (squidaio_ctrl_t *)m->data; + if (ctrlp->fd != fd) continue; - squidaio_cancel(&curr->result); + squidaio_cancel(&ctrlp->result); - if (curr->done_handler) { - AIOCB *callback = curr->done_handler; + if (ctrlp->done_handler) { + AIOCB *callback = ctrlp->done_handler; void *cbdata; - curr->done_handler = NULL; + ctrlp->done_handler = NULL; debug(32, 2) ("this be aioCancel\n"); - if (cbdataReferenceValidDone(curr->done_handler_data, &cbdata)) - callback(fd, cbdata, -2, -2); + if (cbdataReferenceValidDone(ctrlp->done_handler_data, &cbdata)) + callback(fd, cbdata, NULL, -2, -2); + /* free data if requested to aioWrite() */ + if (ctrlp->free_func) + ctrlp->free_func(ctrlp->bufp); + /* free temporary read buffer */ + if (ctrlp->operation == _AIO_READ) + squidaio_xfree(ctrlp->bufp, ctrlp->len); } dlinkDelete(m, &used_list); - memPoolFree(squidaio_ctrl_pool, curr); + memPoolFree(squidaio_ctrl_pool, ctrlp); } } @@ -208,7 +215,7 @@ aioWrite(int fd, int offset, char *bufp, int len, AIOCB * callback, void *callba void -aioRead(int fd, int offset, char *bufp, int len, AIOCB * callback, void *callback_data) +aioRead(int fd, int offset, int len, AIOCB * callback, void *callback_data) { squidaio_ctrl_t *ctrlp; int seekmode; @@ -220,6 +227,8 @@ aioRead(int fd, int offset, char *bufp, int len, AIOCB * callback, void *callbac ctrlp->done_handler = callback; ctrlp->done_handler_data = cbdataReference(callback_data); ctrlp->operation = _AIO_READ; + ctrlp->len = len; + ctrlp->bufp = squidaio_xmalloc(len); if (offset >= 0) seekmode = SEEK_SET; else { @@ -227,7 +236,7 @@ aioRead(int fd, int offset, char *bufp, int len, AIOCB * callback, void *callbac offset = 0; } ctrlp->result.data = ctrlp; - squidaio_read(fd, bufp, len, offset, seekmode, &ctrlp->result); + squidaio_read(fd, ctrlp->bufp, len, offset, seekmode, &ctrlp->result); dlinkAdd(ctrlp, &ctrlp->node, &used_list); return; } /* aioRead */ @@ -296,29 +305,29 @@ aioCheckCallbacks(SwapDir * SD) if ((resultp = squidaio_poll_done()) == NULL) break; ctrlp = (squidaio_ctrl_t *) resultp->data; - switch (resultp->result_type){ - case _AIO_OP_NONE: - case _AIO_OP_TRUNCATE: - case _AIO_OP_OPENDIR: - break; - case _AIO_OP_OPEN: - ++squidaio_counts.open_finish; - break; - case _AIO_OP_READ: - ++squidaio_counts.read_finish; - break; - case _AIO_OP_WRITE: - ++squidaio_counts.write_finish; - break; - case _AIO_OP_CLOSE: - ++squidaio_counts.close_finish; - break; - case _AIO_OP_UNLINK: - ++squidaio_counts.unlink_finish; - break; - case _AIO_OP_STAT: - ++squidaio_counts.stat_finish; - break; + switch (resultp->result_type) { + case _AIO_OP_NONE: + case _AIO_OP_TRUNCATE: + case _AIO_OP_OPENDIR: + break; + case _AIO_OP_OPEN: + ++squidaio_counts.open_finish; + break; + case _AIO_OP_READ: + ++squidaio_counts.read_finish; + break; + case _AIO_OP_WRITE: + ++squidaio_counts.write_finish; + break; + case _AIO_OP_CLOSE: + ++squidaio_counts.close_finish; + break; + case _AIO_OP_UNLINK: + ++squidaio_counts.unlink_finish; + break; + case _AIO_OP_STAT: + ++squidaio_counts.stat_finish; + break; } if (ctrlp == NULL) continue; /* XXX Should not happen */ @@ -329,13 +338,23 @@ aioCheckCallbacks(SwapDir * SD) ctrlp->done_handler = NULL; if (cbdataReferenceValidDone(ctrlp->done_handler_data, &cbdata)) { retval = 1; /* Return that we've actually done some work */ - callback(ctrlp->fd, cbdata, + callback(ctrlp->fd, cbdata, ctrlp->bufp, ctrlp->result.aio_return, ctrlp->result.aio_errno); + } else { + if (ctrlp->operation == _AIO_OPEN) { + /* The open operation was aborted.. */ + int fd = ctrlp->result.aio_return; + if (fd >= 0) + aioClose(fd); + } } } /* free data if requested to aioWrite() */ if (ctrlp->free_func) ctrlp->free_func(ctrlp->bufp); + /* free temporary read buffer */ + if (ctrlp->operation == _AIO_READ) + squidaio_xfree(ctrlp->bufp, ctrlp->len); if (ctrlp->operation == _AIO_CLOSE) aioFDWasClosed(ctrlp->fd); memPoolFree(squidaio_ctrl_pool, ctrlp); @@ -348,13 +367,13 @@ aioStats(StoreEntry * sentry) { storeAppendPrintf(sentry, "ASYNC IO Counters:\n"); storeAppendPrintf(sentry, "Operation\t# Requests\tNumber serviced\n"); - storeAppendPrintf(sentry, "open\t%d\t%d\n", squidaio_counts.open_start,squidaio_counts.open_finish); - storeAppendPrintf(sentry, "close\t%d\t%d\n", squidaio_counts.close_start,squidaio_counts.close_finish); + storeAppendPrintf(sentry, "open\t%d\t%d\n", squidaio_counts.open_start, squidaio_counts.open_finish); + storeAppendPrintf(sentry, "close\t%d\t%d\n", squidaio_counts.close_start, squidaio_counts.close_finish); storeAppendPrintf(sentry, "cancel\t%d\t-\n", squidaio_counts.cancel); - storeAppendPrintf(sentry, "write\t%d\t%d\n", squidaio_counts.write_start,squidaio_counts.write_finish); - storeAppendPrintf(sentry, "read\t%d\t%d\n", squidaio_counts.read_start,squidaio_counts.read_finish); - storeAppendPrintf(sentry, "stat\t%d\t%d\n", squidaio_counts.stat_start,squidaio_counts.stat_finish); - storeAppendPrintf(sentry, "unlink\t%d\t%d\n", squidaio_counts.unlink_start,squidaio_counts.unlink_finish); + storeAppendPrintf(sentry, "write\t%d\t%d\n", squidaio_counts.write_start, squidaio_counts.write_finish); + storeAppendPrintf(sentry, "read\t%d\t%d\n", squidaio_counts.read_start, squidaio_counts.read_finish); + storeAppendPrintf(sentry, "stat\t%d\t%d\n", squidaio_counts.stat_start, squidaio_counts.stat_finish); + storeAppendPrintf(sentry, "unlink\t%d\t%d\n", squidaio_counts.unlink_start, squidaio_counts.unlink_finish); storeAppendPrintf(sentry, "check_callback\t%d\t-\n", squidaio_counts.check_callback); storeAppendPrintf(sentry, "queue\t%d\t-\n", squidaio_get_queue_len()); } diff --git a/src/fs/aufs/store_asyncufs.h b/src/fs/aufs/store_asyncufs.h index 664240a10b..f8c6134922 100644 --- a/src/fs/aufs/store_asyncufs.h +++ b/src/fs/aufs/store_asyncufs.h @@ -48,7 +48,7 @@ struct _squidaio_result_t { typedef struct _squidaio_result_t squidaio_result_t; -typedef void AIOCB(int fd, void *, int aio_return, int aio_errno); +typedef void AIOCB(int fd, void *cbdata, const char *buf, int aio_return, int aio_errno); int squidaio_cancel(squidaio_result_t *); int squidaio_open(const char *, int, mode_t, squidaio_result_t *); @@ -63,6 +63,8 @@ squidaio_result_t *squidaio_poll_done(void); int squidaio_operations_pending(void); int squidaio_sync(void); int squidaio_get_queue_len(void); +void *squidaio_xmalloc(int size); +void squidaio_xfree(void *p, int size); void aioInit(void); void aioDone(void); @@ -70,7 +72,7 @@ void aioCancel(int); void aioOpen(const char *, int, mode_t, AIOCB *, void *); void aioClose(int); void aioWrite(int, int offset, char *, int size, AIOCB *, void *, FREE *); -void aioRead(int, int offset, char *, int size, AIOCB *, void *); +void aioRead(int, int offset, int size, AIOCB *, void *); void aioStat(char *, struct stat *, AIOCB *, void *); void aioUnlink(const char *, AIOCB *, void *); void aioTruncate(const char *, off_t length, AIOCB *, void *); @@ -89,7 +91,7 @@ struct _squidaiostate_t { unsigned int read_kicking:1; unsigned int inreaddone:1; } flags; - const char *read_buf; + char *read_buf; link_list *pending_writes; link_list *pending_reads; }; diff --git a/src/fs/aufs/store_dir_aufs.cc b/src/fs/aufs/store_dir_aufs.cc index 0b88093831..e0b89e6c9d 100644 --- a/src/fs/aufs/store_dir_aufs.cc +++ b/src/fs/aufs/store_dir_aufs.cc @@ -1,6 +1,6 @@ /* - * $Id: store_dir_aufs.cc,v 1.51 2002/10/13 20:35:24 robertc Exp $ + * $Id: store_dir_aufs.cc,v 1.52 2002/11/10 02:29:58 hno Exp $ * * DEBUG: section 47 Store Directory Routines * AUTHOR: Duane Wessels @@ -145,7 +145,7 @@ storeAufsDirReconfigure(SwapDir * sd, int index, char *path) void storeAufsDirDump(StoreEntry * entry, SwapDir * s) { - commonUfsDirDump (entry, s); + commonUfsDirDump(entry, s); dump_cachedir_options(entry, options, s); } diff --git a/src/fs/aufs/store_io_aufs.cc b/src/fs/aufs/store_io_aufs.cc index 13190d4dc0..9177576db8 100644 --- a/src/fs/aufs/store_io_aufs.cc +++ b/src/fs/aufs/store_io_aufs.cc @@ -20,7 +20,7 @@ static DWCB storeAufsWriteDone; #endif static void storeAufsIOCallback(storeIOState * sio, int errflag); static AIOCB storeAufsOpenDone; -static int storeAufsSomethingPending(storeIOState *); +static int storeAufsNeedCompletetion(storeIOState *); static int storeAufsKickWriteQueue(storeIOState * sio); static CBDUNL storeAufsIOFreeEntry; @@ -141,7 +141,7 @@ storeAufsClose(SwapDir * SD, storeIOState * sio) squidaiostate_t *aiostate = (squidaiostate_t *) sio->fsstate; debug(79, 3) ("storeAufsClose: dirno %d, fileno %08X, FD %d\n", sio->swap_dirn, sio->swap_filen, aiostate->fd); - if (storeAufsSomethingPending(sio)) { + if (storeAufsNeedCompletetion(sio)) { aiostate->flags.close_request = 1; return; } @@ -167,7 +167,7 @@ storeAufsRead(SwapDir * SD, storeIOState * sio, char *buf, size_t size, off_t of q->size = size; q->offset = offset; q->callback = callback; - q->callback_data = callback_data; + q->callback_data = cbdataReference(callback_data); linklistPush(&(aiostate->pending_reads), q); return; } @@ -179,7 +179,7 @@ storeAufsRead(SwapDir * SD, storeIOState * sio, char *buf, size_t size, off_t of sio->offset = offset; aiostate->flags.reading = 1; #if ASYNC_READ - aioRead(aiostate->fd, offset, buf, size, storeAufsReadDone, sio); + aioRead(aiostate->fd, offset, size, storeAufsReadDone, sio); #else file_read(aiostate->fd, buf, size, offset, storeAufsReadDone, sio); #endif @@ -257,17 +257,19 @@ storeAufsKickReadQueue(storeIOState * sio) { squidaiostate_t *aiostate = (squidaiostate_t *) sio->fsstate; struct _queued_read *q = (struct _queued_read *)linklistShift(&(aiostate->pending_reads)); + void *cbdata; if (NULL == q) return 0; debug(79, 3) ("storeAufsKickReadQueue: reading queued request of %ld bytes\n", (long int) q->size); - storeAufsRead(INDEXSD(sio->swap_dirn), sio, q->buf, q->size, q->offset, q->callback, q->callback_data); + if (cbdataReferenceValidDone(q->callback_data, &cbdata)) + storeAufsRead(INDEXSD(sio->swap_dirn), sio, q->buf, q->size, q->offset, q->callback, cbdata); memPoolFree(aufs_qread_pool, q); return 1; } static void -storeAufsOpenDone(int unused, void *my_data, int fd, int errflag) +storeAufsOpenDone(int unused, void *my_data, const char *unused2, int fd, int errflag) { storeIOState *sio = (storeIOState *)my_data; squidaiostate_t *aiostate = (squidaiostate_t *) sio->fsstate; @@ -288,7 +290,7 @@ storeAufsOpenDone(int unused, void *my_data, int fd, int errflag) if (FILE_MODE(sio->mode) == O_WRONLY) { if (storeAufsKickWriteQueue(sio)) return; - } else if (FILE_MODE(sio->mode) == O_RDONLY) { + } else if ((FILE_MODE(sio->mode) == O_RDONLY) && !aiostate->flags.close_request) { if (storeAufsKickReadQueue(sio)) return; } @@ -299,7 +301,7 @@ storeAufsOpenDone(int unused, void *my_data, int fd, int errflag) #if ASYNC_READ static void -storeAufsReadDone(int fd, void *my_data, int len, int errflag) +storeAufsReadDone(int fd, void *my_data, const char *buf, int len, int errflag) #else static void storeAufsReadDone(int fd, const char *buf, int len, int errflag, void *my_data) @@ -310,6 +312,7 @@ storeAufsReadDone(int fd, const char *buf, int len, int errflag, void *my_data) STRCB *callback = sio->read.callback; void *cbdata; ssize_t rlen; + int inreaddone = aiostate->flags.inreaddone; /* Protect from callback loops */ debug(79, 3) ("storeAufsReadDone: dirno %d, fileno %08X, FD %d, len %d\n", sio->swap_dirn, sio->swap_filen, fd, len); aiostate->flags.inreaddone = 1; @@ -334,10 +337,15 @@ storeAufsReadDone(int fd, const char *buf, int len, int errflag, void *my_data) #endif assert(callback); sio->read.callback = NULL; - if (cbdataReferenceValidDone(sio->read.callback_data, &cbdata)) + if (!aiostate->flags.close_request && cbdataReferenceValidDone(sio->read.callback_data, &cbdata)) { +#if ASYNC_READ + if (rlen > 0) + memcpy(aiostate->read_buf, buf, rlen); +#endif callback(cbdata, aiostate->read_buf, rlen); + } aiostate->flags.inreaddone = 0; - if (aiostate->flags.close_request) + if (aiostate->flags.close_request && !inreaddone) storeAufsIOCallback(sio, errflag); } @@ -396,38 +404,39 @@ storeAufsIOCallback(storeIOState * sio, int errflag) squidaiostate_t *aiostate = (squidaiostate_t *) sio->fsstate; int fd = aiostate->fd; debug(79, 3) ("storeAufsIOCallback: errflag=%d\n", errflag); - debug(79, 3) ("%s:%d\n", __FILE__, __LINE__); + debug(79, 9) ("%s:%d\n", __FILE__, __LINE__); if (callback) { void *cbdata; sio->callback = NULL; if (cbdataReferenceValidDone(sio->callback_data, &cbdata)) callback(cbdata, errflag, sio); } - debug(79, 3) ("%s:%d\n", __FILE__, __LINE__); + debug(79, 9) ("%s:%d\n", __FILE__, __LINE__); aiostate->fd = -1; cbdataFree(sio); if (fd < 0) return; - debug(79, 3) ("%s:%d\n", __FILE__, __LINE__); + debug(79, 9) ("%s:%d\n", __FILE__, __LINE__); aioClose(fd); fd_close(fd); store_open_disk_fd--; - debug(79, 3) ("%s:%d\n", __FILE__, __LINE__); + debug(79, 9) ("%s:%d\n", __FILE__, __LINE__); } static int -storeAufsSomethingPending(storeIOState * sio) +storeAufsNeedCompletetion(storeIOState * sio) { squidaiostate_t *aiostate = (squidaiostate_t *) sio->fsstate; - if (aiostate->flags.reading) - return 1; + if (aiostate->flags.writing) return 1; - if (aiostate->flags.opening) + if (aiostate->flags.opening && FILE_MODE(sio->mode) == O_WRONLY) return 1; if (aiostate->flags.inreaddone) return 1; + + /* Note: Pending read operations are silently cancelled on close */ return 0; } @@ -438,7 +447,24 @@ storeAufsSomethingPending(storeIOState * sio) * to bother with that. */ static void -storeAufsIOFreeEntry(void *sio) +storeAufsIOFreeEntry(void *siop) { - memPoolFree(squidaio_state_pool, ((storeIOState *) sio)->fsstate); + storeIOState *sio = (storeIOState *) siop; + squidaiostate_t *aiostate = (squidaiostate_t *) sio->fsstate; + struct _queued_write *qw; + struct _queued_read *qr; + while ((qw = linklistShift(&aiostate->pending_writes))) { + if (qw->free_func) + qw->free_func(qw->buf); + memPoolFree(aufs_qwrite_pool, qw); + } + while ((qr = linklistShift(&aiostate->pending_reads))) { + cbdataReferenceDone(qr->callback_data); + memPoolFree(aufs_qread_pool, qr); + } + if (sio->read.callback_data) + cbdataReferenceDone(sio->read.callback_data); + if (sio->callback_data) + cbdataReferenceDone(sio->callback_data); + memPoolFree(squidaio_state_pool, aiostate); }