]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
client: carefully evaluate whether out size is valid during read
authorSage Weil <sage@newdream.net>
Fri, 11 Apr 2008 16:50:54 +0000 (09:50 -0700)
committerSage Weil <sage@newdream.net>
Fri, 11 Apr 2008 16:52:49 +0000 (09:52 -0700)
src/client/Client.cc
src/client/Client.h
src/client/SyntheticClient.cc

index e93b4d825d7182164dbd629317524a67409bf263..07cab2c654949e8e70f427dfa2469bac2abf6ed4 100644 (file)
@@ -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;
index 967e8a4c34ec5035c8bce17ebece4c6dfad6d620..960d4ce5983a6faa06a3a5c2f1cab7ba48db9c59 100644 (file)
@@ -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++;
index 10e1e5120c34e5a61cf74e692617dabfdd18b1cc..b48c96e067ef40ff9f73da3f9fd8f456a9908111 100644 (file)
@@ -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);