From: Amos Jeffries Date: Fri, 15 Jul 2016 13:09:18 +0000 (+1200) Subject: Cleanup: use std::queue for UFS file I/O queues X-Git-Tag: SQUID_4_0_13~31 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2f4c26aafada08974ba1cb8aad3633cdd7b5cf65;p=thirdparty%2Fsquid.git Cleanup: use std::queue for UFS file I/O queues --- diff --git a/src/fs/ufs/UFSStoreState.cc b/src/fs/ufs/UFSStoreState.cc index 799af4e6a4..22ebd4f3da 100644 --- a/src/fs/ufs/UFSStoreState.cc +++ b/src/fs/ufs/UFSStoreState.cc @@ -132,8 +132,9 @@ Fs::Ufs::UFSStoreState::read_(char *buf, size_t size, off_t aOffset, STRCB * aCa assert (aCallback); if (!theFile->canRead()) { - debugs(79, 3, "UFSStoreState::read_: queueing read because theFile can't read"); - queueRead (buf, size, aOffset, aCallback, aCallbackData); + debugs(79, 3, "queueing read because theFile can't read"); + assert(opening); + pending_reads.emplace(buf, size, aOffset, aCallback, aCallbackData); return; } @@ -177,7 +178,8 @@ Fs::Ufs::UFSStoreState::write(char const *buf, size_t size, off_t aOffset, FREE return false; } - queueWrite(buf, size, aOffset, free_func); + debugs(79, 3, (void*)this << " queueing write of size " << size); + pending_writes.emplace(buf, size, aOffset, free_func); drainWriteQueue(); return true; } @@ -191,28 +193,20 @@ Fs::Ufs::UFSStoreState::write(char const *buf, size_t size, off_t aOffset, FREE void Fs::Ufs::UFSStoreState::doWrite() { - debugs(79, 3, HERE << this << " UFSStoreState::doWrite"); + debugs(79, 3, (void*)this); assert(theFile->canWrite()); - _queued_write *q = (_queued_write *)linklistShift(&pending_writes); - - if (q == NULL) { - debugs(79, 3, HERE << this << " UFSStoreState::doWrite queue is empty"); + if (pending_writes.empty()) { + debugs(79, 3, (void*)this << " write queue is empty"); return; } + auto &q = pending_writes.front(); + if (theFile->error()) { - debugs(79, DBG_IMPORTANT,HERE << "avoid write on theFile with error"); - debugs(79,3,HERE << "calling free_func for " << (void*) q->buf); - /* - * DPW 2006-05-24 - * Note "free_func" is memNodeWriteComplete(), which doesn't - * really free the memory. Instead it clears the node's - * write_pending flag. - */ - q->free_func((void*)q->buf); - delete q; + debugs(79, DBG_IMPORTANT, MYNAME << " avoid write on theFile with error"); + pending_writes.pop(); return; } @@ -226,10 +220,11 @@ Fs::Ufs::UFSStoreState::doWrite() * coming in. For now let's just not use the writing flag at * all. */ - debugs(79, 3, HERE << this << " calling theFile->write(" << q->size << ")"); + debugs(79, 3, (void*)this << " calling theFile->write(" << q.size << ")"); - theFile->write(new WriteRequest(q->buf, q->offset, q->size, q->free_func)); - delete q; + theFile->write(new WriteRequest(q.buf, q.offset, q.size, q.free_func)); + q.buf = nullptr; // prevent buf deletion on pop, its used by the above object + pending_writes.pop(); } void @@ -338,8 +333,6 @@ Fs::Ufs::UFSStoreState::UFSStoreState(SwapDir * SD, StoreEntry * anEntry, STIOCB closing(false), reading(false), writing(false), - pending_reads(NULL), - pending_writes(NULL), read_buf(NULL) { // StoreIOState inherited members @@ -354,71 +347,44 @@ Fs::Ufs::UFSStoreState::UFSStoreState(SwapDir * SD, StoreEntry * anEntry, STIOCB Fs::Ufs::UFSStoreState::~UFSStoreState() { - assert(pending_reads == NULL); - assert(pending_writes == NULL); + assert(pending_reads.empty()); + assert(pending_writes.empty()); } void Fs::Ufs::UFSStoreState::freePending() { - _queued_read *qr; - - while ((qr = (_queued_read *)linklistShift(&pending_reads))) { - cbdataReferenceDone(qr->callback_data); - delete qr; - } - - debugs(79,3,HERE << "UFSStoreState::freePending: freed pending reads"); + while (!pending_reads.empty()) + pending_reads.pop(); + debugs(79, 3, "freed pending reads"); - _queued_write *qw; - - while ((qw = (_queued_write *)linklistShift(&pending_writes))) { - if (qw->free_func) - qw->free_func(const_cast(qw->buf)); - delete qw; - } - - debugs(79,3,HERE << "UFSStoreState::freePending: freed pending writes"); + while (!pending_writes.empty()) + pending_writes.pop(); + debugs(79, 3, "freed pending writes"); } bool Fs::Ufs::UFSStoreState::kickReadQueue() { - _queued_read *q = (_queued_read *)linklistShift(&pending_reads); - - if (NULL == q) + if (pending_reads.empty()) return false; - debugs(79, 3, "UFSStoreState::kickReadQueue: reading queued request of " << q->size << " bytes"); + auto &q = pending_reads.front(); - void *cbdata; + debugs(79, 3, "reading queued request of " << q.size << " bytes"); - if (cbdataReferenceValidDone(q->callback_data, &cbdata)) { - read_(q->buf, q->size, q->offset, q->callback, cbdata); + bool result = true; + void *cbdata; + if (cbdataReferenceValidDone(q.callback_data, &cbdata)) { + read_(q.buf, q.size, q.offset, q.callback, cbdata); } else { - debugs(79, 2, "UFSStoreState::kickReadQueue: this: " << this << " cbdataReferenceValidDone returned false." << " closing: " << closing << " flags.try_closing: " << flags.try_closing); - delete q; - return false; + debugs(79, 2, "this=" << (void*)this << " cbdataReferenceValidDone returned false." << + " closing: " << closing << " flags.try_closing: " << flags.try_closing); + result = false; } - delete q; - - return true; -} - -void -Fs::Ufs::UFSStoreState::queueRead(char *buf, size_t size, off_t aOffset, STRCB *callback_, void *callback_data_) -{ - debugs(79, 3, "UFSStoreState::queueRead: queueing read"); - assert(opening); - assert (pending_reads == NULL); - _queued_read *q = new _queued_read; - q->buf = buf; - q->size = size; - q->offset = aOffset; - q->callback = callback_; - q->callback_data = cbdataReference(callback_data_); - linklistPush(&pending_reads, q); + pending_reads.pop(); // erase the front object + return result; } /* @@ -445,9 +411,8 @@ Fs::Ufs::UFSStoreState::drainWriteQueue() flags.write_draining = true; - while (pending_writes != NULL) { + while (!pending_writes.empty()) doWrite(); - } flags.write_draining = false; @@ -482,17 +447,3 @@ Fs::Ufs::UFSStoreState::tryClosing() theFile->close(); } -void -Fs::Ufs::UFSStoreState::queueWrite(char const *buf, size_t size, off_t aOffset, FREE * free_func) -{ - debugs(79, 3, HERE << this << " UFSStoreState::queueWrite: queueing write of size " << size); - - _queued_write *q; - q = new _queued_write; - q->buf = buf; - q->size = size; - q->offset = aOffset; - q->free_func = free_func; - linklistPush(&pending_writes, q); -} - diff --git a/src/fs/ufs/UFSStoreState.h b/src/fs/ufs/UFSStoreState.h index c722559335..55ab7181da 100644 --- a/src/fs/ufs/UFSStoreState.h +++ b/src/fs/ufs/UFSStoreState.h @@ -13,6 +13,8 @@ #include "SquidList.h" #include "StoreIOState.h" +#include + namespace Fs { namespace Ufs @@ -48,13 +50,16 @@ protected: { MEMPROXY_CLASS(UFSStoreState::_queued_read); public: - _queued_read() : - buf(nullptr), - size(0), - offset(0), - callback(nullptr), - callback_data(nullptr) + _queued_read(char *b, size_t s, off_t o, STRCB *cb, void *data) : + buf(b), + size(s), + offset(o), + callback(cb), + callback_data(cbdataReference(data)) {} + ~_queued_read() { + cbdataReferenceDone(callback_data); + } char *buf; size_t size; @@ -62,23 +67,35 @@ protected: STRCB *callback; void *callback_data; }; + std::queue pending_reads; class _queued_write { MEMPROXY_CLASS(UFSStoreState::_queued_write); public: - _queued_write() : - buf(nullptr), - size(0), - offset(0), - free_func(nullptr) + _queued_write(const char *b, size_t s, off_t o, FREE *f) : + buf(b), + size(s), + offset(o), + free_func(f) {} + ~_queued_write() { + /* + * DPW 2006-05-24 + * Note "free_func" is memNodeWriteComplete(), which doesn't + * really free the memory. Instead it clears the node's + * write_pending flag. + */ + if (free_func && buf) + free_func(const_cast(buf)); + } char const *buf; size_t size; off_t offset; FREE *free_func; }; + std::queue pending_writes; /** \todo These should be in the IO strategy */ @@ -98,10 +115,7 @@ protected: */ bool try_closing; } flags; - link_list *pending_reads; - link_list *pending_writes; - void queueRead(char *, size_t, off_t, STRCB *, void *); - void queueWrite(char const *, size_t, off_t, FREE *); + bool kickReadQueue(); void drainWriteQueue(); void tryClosing();