From 5aedd2ace88d28c9997cf6c172863aa3a91fd577 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 11 Apr 2008 09:50:54 -0700 Subject: [PATCH] client: carefully evaluate whether out size is valid during read --- src/client/Client.cc | 95 ++++++++++++++++++----------------- src/client/Client.h | 23 +++++++++ src/client/SyntheticClient.cc | 21 ++++++++ 3 files changed, 94 insertions(+), 45 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index e93b4d825d718..07cab2c654949 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -2122,16 +2122,7 @@ int Client::_do_lstat(filepath &fpath, int mask, Inode **in) int havemask = 0; if (dn) { - if (now < dn->inode->lease_ttl && - dn->inode->lease_mds >= 0) - havemask = dn->inode->lease_mask; - if (dn->inode->file_caps() & CEPH_CAP_EXCL) { - dout(10) << "_lstat has inode " << fpath << " with CAP_EXCL, yay" << dendl; - havemask |= CEPH_LOCK_ICONTENT; - } - if (havemask & CEPH_LOCK_ICONTENT) - havemask |= CEPH_LOCK_ICONTENT; // hack: if we have one, we have both, for the purposes of below - + havemask = dn->inode->get_effective_lease_mask(now); dout(10) << "_lstat has inode " << fpath << " with mask " << havemask << ", want " << mask << dendl; } else { @@ -3058,37 +3049,59 @@ int Client::_read(Fh *f, off_t offset, off_t size, bufferlist *bl) bool lazy = f->mode == FILE_MODE_LAZY; - // wait for RD cap before checking size - while (!lazy && (in->file_caps() & CEPH_CAP_RD) == 0) { - dout(7) << " don't have read cap, waiting" << dendl; - Cond cond; - in->waitfor_read.push_back(&cond); - cond.Wait(client_lock); - } + // wait for RD cap and/or a valid file size + while (1) { - // determine whether read range overlaps with file - // ...ONLY if we're doing async io - if (!lazy && (in->file_caps() & (CEPH_CAP_WRBUFFER|CEPH_CAP_RDCACHE))) { - // we're doing buffered i/o. make sure we're inside the file. - // we can trust size info bc we get accurate info when buffering/caching caps are issued. - dout(10) << "file size: " << in->inode.size << dendl; - if (offset > 0 && offset >= in->inode.size) { - if (movepos) unlock_fh_pos(f); - return 0; + if (lazy) { + // wait for lazy cap + if ((in->file_caps() & CEPH_CAP_LAZYIO) == 0) { + dout(7) << " don't have lazy cap, waiting" << dendl; + goto wait; + } + } else { + // wait for RD cap? + while ((in->file_caps() & CEPH_CAP_RD) == 0) { + dout(7) << " don't have read cap, waiting" << dendl; + goto wait; + } } - if (offset + size > (off_t)in->inode.size) - size = (off_t)in->inode.size - offset; - if (size == 0) { - dout(10) << "read is size=0, returning 0" << dendl; - if (movepos) unlock_fh_pos(f); - return 0; + // async i/o? + if ((in->file_caps() & (CEPH_CAP_WRBUFFER|CEPH_CAP_RDCACHE))) { + + // FIXME: this logic needs to move info FileCache! + + // wait for valid file size + if (!in->have_valid_size()) { + dout(7) << " don't have (rd+rdcache)|lease|excl for valid file size, waiting" << dendl; + goto wait; + } + + dout(10) << "file size: " << in->inode.size << dendl; + if (offset > 0 && offset >= in->inode.size) { + if (movepos) unlock_fh_pos(f); + return 0; + } + if (offset + size > (off_t)in->inode.size) + size = (off_t)in->inode.size - offset; + + if (size == 0) { + dout(10) << "read is size=0, returning 0" << dendl; + if (movepos) unlock_fh_pos(f); + return 0; + } + break; + } else { + // unbuffered, sync i/o. defer to osd. + break; } - } else { - // unbuffered, synchronous file i/o. - // or lazy. - // defer to OSDs for file bounds. + + wait: + Cond cond; + in->waitfor_read.push_back(&cond); + cond.Wait(client_lock); } + int r = 0; int rvalue = 0; @@ -3098,15 +3111,7 @@ int Client::_read(Fh *f, off_t offset, off_t size, bufferlist *bl) rvalue = r = in->fc.read(offset, size, *bl, client_lock); // may block. } else { // object cache OFF -- legacy inconsistent way. - - // lazy cap? - while (lazy && (in->file_caps() & CEPH_CAP_LAZYIO) == 0) { - dout(7) << " don't have lazy cap, waiting" << dendl; - Cond cond; - in->waitfor_lazy.push_back(&cond); - cond.Wait(client_lock); - } - + // do sync read Cond cond; bool done = false; diff --git a/src/client/Client.h b/src/client/Client.h index 967e8a4c34ec5..960d4ce5983a6 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -238,6 +238,29 @@ class Inode { return w; } + int get_effective_lease_mask(utime_t now) { + int havemask = 0; + if (now < lease_ttl && lease_mds >= 0) + havemask |= lease_mask; + if (file_caps() & CEPH_CAP_EXCL) + havemask |= CEPH_LOCK_ICONTENT; + if (havemask & CEPH_LOCK_ICONTENT) + havemask |= CEPH_LOCK_ICONTENT; // hack: if we have one, we have both, for the purposes of below + return havemask; + } + + bool have_valid_size() { + // RD+RDCACHE or WR+WRBUFFER => valid size + if ((file_caps() & (CEPH_CAP_RD|CEPH_CAP_RDCACHE)) == (CEPH_CAP_RD|CEPH_CAP_RDCACHE)) + return true; + if ((file_caps() & (CEPH_CAP_WR|CEPH_CAP_WRBUFFER)) == (CEPH_CAP_WR|CEPH_CAP_WRBUFFER)) + return true; + // otherwise, look for lease or EXCL... + if (get_effective_lease_mask(g_clock.now()) & CEPH_LOCK_ICONTENT) + return true; + return false; + } + void add_open(int cmode) { if (cmode & FILE_MODE_R) num_open_rd++; if (cmode & FILE_MODE_W) num_open_wr++; diff --git a/src/client/SyntheticClient.cc b/src/client/SyntheticClient.cc index 10e1e5120c34e..b48c96e067ef4 100644 --- a/src/client/SyntheticClient.cc +++ b/src/client/SyntheticClient.cc @@ -2626,6 +2626,27 @@ void SyntheticClient::make_dir_mess(const char *basedir, int n) void SyntheticClient::foo() { + if (1) { + // bug1.cpp + const char *fn = "blah"; + char buffer[8192]; + client->unlink(fn); + int handle = client->open(fn,O_CREAT|O_RDWR,S_IRWXU); + assert(handle>=0); + int r=client->write(handle,buffer,8192); + assert(r>=0); + r=client->close(handle); + assert(r>=0); + + handle = client->open(fn,O_RDWR); // open the same file, it must have some data already + assert(handle>=0); + r=client->read(handle,buffer,8192); + assert(r==8192); // THIS ASSERTION FAILS with disabled cache + r=client->close(handle); + assert(r>=0); + + return; + } if (1) { dout(0) << "first" << dendl; int fd = client->open("tester", O_WRONLY|O_CREAT); -- 2.39.5