Skip to content

Commit

Permalink
client: do more check for the client_lock
Browse files Browse the repository at this point in the history 
Avoid to use the client_lock.unlock/.lock directly, and add more
check for the client_lock.

Fixes: https://tracker.ceph.com/issues/47039
Signed-off-by: Xiubo Li <xiubli@redhat.com>
  • Loading branch information
@lxbsz
lxbsz committed Aug 20, 2020
1 parent 97c6bcb commit 0a5dfd4
Showing 1 changed file with 50 additions and 29 deletions.
79 changes: 50 additions & 29 deletions src/client/Client.cc
View file
Expand Up @@ -5785,6 +5785,8 @@ int Client::authenticate()

int Client::fetch_fsmap(bool user)
{
ceph_assert(ceph_mutex_is_locked_by_me(client_lock));

// Retrieve FSMap to enable looking up daemon addresses. We need FSMap
// rather than MDSMap because no one MDSMap contains all the daemons, and
// a `tell` can address any daemon.
Expand Down Expand Up @@ -8362,7 +8364,7 @@ int Client::readdir_r_cb(dir_result_t *d, add_dirent_cb_t cb, void *p,
if (!mref_reader.is_state_satisfied())
return -ENOTCONN;

std::scoped_lock lock(client_lock);
std::unique_lock cl(client_lock);

dir_result_t *dirp = static_cast<dir_result_t*>(d);

Expand Down Expand Up @@ -8399,9 +8401,9 @@ int Client::readdir_r_cb(dir_result_t *d, add_dirent_cb_t cb, void *p,
_ll_get(inode);
}

client_lock.unlock();
cl.unlock();
r = cb(p, &de, &stx, next_off, inode);
client_lock.lock();
cl.lock();
if (r < 0)
return r;

Expand Down Expand Up @@ -8432,9 +8434,9 @@ int Client::readdir_r_cb(dir_result_t *d, add_dirent_cb_t cb, void *p,
_ll_get(inode);
}

client_lock.unlock();
cl.unlock();
r = cb(p, &de, &stx, next_off, inode);
client_lock.lock();
cl.lock();
if (r < 0)
return r;

Expand Down Expand Up @@ -8499,9 +8501,9 @@ int Client::readdir_r_cb(dir_result_t *d, add_dirent_cb_t cb, void *p,
_ll_get(inode);
}

client_lock.unlock();
cl.unlock();
r = cb(p, &de, &stx, next_off, inode); // _next_ offset
client_lock.lock();
cl.lock();

ldout(cct, 15) << " de " << de.d_name << " off " << hex << next_off - 1 << dec
<< " = " << r << dendl;
Expand Down Expand Up @@ -9551,16 +9553,18 @@ int Client::_read_async(Fh *f, uint64_t off, uint64_t len, bufferlist *bl)
<< " max_bytes=" << f->readahead.get_max_readahead_size()
<< " max_periods=" << conf->client_readahead_max_periods << dendl;

std::unique_lock cl{client_lock, std::adopt_lock};

// read (and possibly block)
int r = 0;
C_SaferCond onfinish("Client::_read_async flock");
r = objectcacher->file_read(&in->oset, &in->layout, in->snapid,
off, len, bl, 0, &onfinish);
if (r == 0) {
get_cap_ref(in, CEPH_CAP_FILE_CACHE);
client_lock.unlock();
cl.unlock();
r = onfinish.wait();
client_lock.lock();
cl.lock();
put_cap_ref(in, CEPH_CAP_FILE_CACHE);
}

Expand All @@ -9583,12 +9587,15 @@ int Client::_read_async(Fh *f, uint64_t off, uint64_t len, bufferlist *bl)
}
}

cl.release();
return r;
}

int Client::_read_sync(Fh *f, uint64_t off, uint64_t len, bufferlist *bl,
bool *checkeof)
{
std::unique_lock cl{client_lock, std::adopt_lock};

Inode *in = f->inode.get();
uint64_t pos = off;
int left = len;
Expand All @@ -9605,14 +9612,16 @@ int Client::_read_sync(Fh *f, uint64_t off, uint64_t len, bufferlist *bl,
pos, left, &tbl, 0,
in->truncate_size, in->truncate_seq,
&onfinish);
client_lock.unlock();
cl.unlock();
int r = onfinish.wait();

// if we get ENOENT from OSD, assume 0 bytes returned
if (r == -ENOENT)
r = 0;
if (r < 0)
if (r < 0) {
cl.release();
return r;
}
if (tbl.length()) {
r = tbl.length();

Expand All @@ -9635,14 +9644,17 @@ int Client::_read_sync(Fh *f, uint64_t off, uint64_t len, bufferlist *bl,
pos += some;
left -= some;
if (left == 0)
return read;
goto out;
}

*checkeof = true;
return read;
goto out;
}
client_lock.lock();
cl.lock();
}

out:
cl.release();
return read;
}

Expand Down Expand Up @@ -9874,7 +9886,8 @@ int64_t Client::_write(Fh *f, int64_t offset, uint64_t size, const char *buf,
ldout(cct, 10) << " snaprealm " << *in->snaprealm << dendl;

std::unique_ptr<C_SaferCond> onuninline = nullptr;

std::unique_lock cl{client_lock, std::adopt_lock};

if (in->inline_version < CEPH_INLINE_NONE) {
if (endoff > cct->_conf->client_max_inline_size ||
endoff > CEPH_INLINE_MAX_SIZE ||
Expand Down Expand Up @@ -9939,9 +9952,9 @@ int64_t Client::_write(Fh *f, int64_t offset, uint64_t size, const char *buf,
offset, size, bl, ceph::real_clock::now(), 0,
in->truncate_size, in->truncate_seq,
&onfinish);
client_lock.unlock();
cl.unlock();
r = onfinish.wait();
client_lock.lock();
cl.lock();
_sync_write_commit(in);
if (r < 0)
goto done;
Expand Down Expand Up @@ -9986,9 +9999,9 @@ int64_t Client::_write(Fh *f, int64_t offset, uint64_t size, const char *buf,
done:

if (nullptr != onuninline) {
client_lock.unlock();
cl.unlock();
int uninline_ret = onuninline->wait();
client_lock.lock();
cl.lock();

if (uninline_ret >= 0 || uninline_ret == -ECANCELED) {
in->inline_data.clear();
Expand All @@ -10000,6 +10013,7 @@ int64_t Client::_write(Fh *f, int64_t offset, uint64_t size, const char *buf,
}

put_cap_ref(in, CEPH_CAP_FILE_WR);
cl.release();
return r;
}

Expand Down Expand Up @@ -10086,6 +10100,8 @@ int Client::fsync(int fd, bool syncdataonly)

int Client::_fsync(Inode *in, bool syncdataonly)
{
std::unique_lock cl{client_lock, std::adopt_lock};

int r = 0;
std::unique_ptr<C_SaferCond> object_cacher_completion = nullptr;
ceph_tid_t flush_tid = 0;
Expand Down Expand Up @@ -10120,10 +10136,10 @@ int Client::_fsync(Inode *in, bool syncdataonly)
}

if (nullptr != object_cacher_completion) { // wait on a real reply instead of guessing
client_lock.unlock();
cl.unlock();
ldout(cct, 15) << "waiting on data to flush" << dendl;
r = object_cacher_completion->wait();
client_lock.lock();
cl.lock();
ldout(cct, 15) << "got " << r << " from flush writeback" << dendl;
} else {
// FIXME: this can starve
Expand All @@ -10148,6 +10164,7 @@ int Client::_fsync(Inode *in, bool syncdataonly)
lat -= start;
logger->tinc(l_c_fsync, lat);

cl.release();
return r;
}

Expand Down Expand Up @@ -13650,7 +13667,7 @@ int Client::ll_read_block(Inode *in, uint64_t blockid,
C_SaferCond onfinish;
bufferlist bl;

std::scoped_lock lock(client_lock);
std::unique_lock cl(client_lock);

objecter->read(oid,
object_locator_t(layout->pool_id),
Expand All @@ -13661,7 +13678,7 @@ int Client::ll_read_block(Inode *in, uint64_t blockid,
CEPH_OSD_FLAG_READ,
&onfinish);

client_lock.unlock();
cl.unlock();
int r = onfinish.wait();

if (r >= 0) {
Expand Down Expand Up @@ -13709,7 +13726,7 @@ int Client::ll_write_block(Inode *in, uint64_t blockid,
fakesnap.seq = snapseq;

/* lock just in time */
client_lock.lock();
std::scoped_lock lock(client_lock);
objecter->write(oid,
object_locator_t(layout->pool_id),
offset,
Expand All @@ -13720,7 +13737,6 @@ int Client::ll_write_block(Inode *in, uint64_t blockid,
0,
onsafe.get());

client_lock.unlock();
if (nullptr != onsafe) {
r = onsafe->wait();
}
Expand Down Expand Up @@ -13884,6 +13900,8 @@ int Client::_fallocate(Fh *fh, int mode, int64_t offset, int64_t length)
if (r < 0)
return r;

std::unique_lock cl{client_lock, std::adopt_lock};

std::unique_ptr<C_SaferCond> onuninline = nullptr;
if (mode & FALLOC_FL_PUNCH_HOLE) {
if (in->inline_version < CEPH_INLINE_NONE &&
Expand Down Expand Up @@ -13929,9 +13947,9 @@ int Client::_fallocate(Fh *fh, int mode, int64_t offset, int64_t length)
in->change_attr++;
in->mark_caps_dirty(CEPH_CAP_FILE_WR);

client_lock.unlock();
cl.unlock();
onfinish.wait();
client_lock.lock();
cl.lock();
_sync_write_commit(in);
}
} else if (!(mode & FALLOC_FL_KEEP_SIZE)) {
Expand All @@ -13951,9 +13969,9 @@ int Client::_fallocate(Fh *fh, int mode, int64_t offset, int64_t length)
}

if (nullptr != onuninline) {
client_lock.unlock();
cl.unlock();
int ret = onuninline->wait();
client_lock.lock();
cl.lock();

if (ret >= 0 || ret == -ECANCELED) {
in->inline_data.clear();
Expand All @@ -13965,6 +13983,7 @@ int Client::_fallocate(Fh *fh, int mode, int64_t offset, int64_t length)
}

put_cap_ref(in, CEPH_CAP_FILE_WR);
cl.release();
return r;
}
#else
Expand Down Expand Up @@ -14561,6 +14580,8 @@ enum {

int Client::check_pool_perm(Inode *in, int need)
{
ceph_assert(ceph_mutex_is_locked_by_me(client_lock));

if (!cct->_conf->client_check_pool_perm)
return 0;

Expand Down

0 comments on commit 0a5dfd4

Please sign in to comment.