From: Sage Weil Date: Thu, 10 Dec 2015 21:34:27 +0000 (-0500) Subject: os/bluestore/BlueFS: ref count BlueFS::File * X-Git-Tag: v10.0.3~154^2~165 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=3649a80a89f2db51daf6579d6eab1a2ccb1fa0c8;p=ceph.git os/bluestore/BlueFS: ref count BlueFS::File * There are FileWriters that exist when the file is deleted. Signed-off-by: Sage Weil --- diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index a57b07303f7b..78dbb7a6e3fc 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -111,7 +111,7 @@ int BlueFS::mkfs(uint64_t super_offset_a, uint64_t super_offset_b) dout(1) << __func__ << " uuid " << super.uuid << dendl; // init log - File *log_file = new File; + FileRef log_file = new File; log_file->fnode.ino = 1; _allocate(0, g_conf->bluefs_max_log_runway, &log_file->fnode.extents); log_writer = new FileWriter(log_file); @@ -143,7 +143,6 @@ int BlueFS::mkfs(uint64_t super_offset_a, uint64_t super_offset_b) super = bluefs_super_t(); delete log_writer; log_writer = NULL; - delete log_file; block_all.clear(); alloc.clear(); @@ -218,9 +217,7 @@ void BlueFS::umount() } alloc.clear(); block_all.clear(); - for (auto p : file_map) { - delete p.second; - } + file_map.clear(); for (auto p : dir_map) { delete p.second; } @@ -319,7 +316,7 @@ int BlueFS::_replay() ino_last = 1; // by the log log_seq = 0; - File *log_file = _get_file(1); + FileRef log_file = _get_file(1); log_file->fnode = super.log_fnode; FileReader *log_reader = new FileReader( @@ -435,11 +432,11 @@ int BlueFS::_replay() dout(20) << __func__ << " " << pos << ": op_dir_link " << " " << dirname << "/" << filename << " to " << ino << dendl; - File *file = _get_file(ino); + FileRef file = _get_file(ino); assert(file->fnode.ino); map::iterator q = dir_map.find(dirname); assert(q != dir_map.end()); - map::iterator r = q->second->file_map.find(filename); + map::iterator r = q->second->file_map.find(filename); assert(r == q->second->file_map.end()); q->second->file_map[filename] = file; ++file->refs; @@ -455,7 +452,7 @@ int BlueFS::_replay() << " " << dirname << "/" << filename << dendl; map::iterator q = dir_map.find(dirname); assert(q != dir_map.end()); - map::iterator r = q->second->file_map.find(filename); + map::iterator r = q->second->file_map.find(filename); assert(r != q->second->file_map.end()); q->second->file_map.erase(r); } @@ -493,7 +490,7 @@ int BlueFS::_replay() ::decode(fnode, p); dout(20) << __func__ << " " << pos << ": op_file_update " << " " << fnode << dendl; - File *f = _get_file(fnode.ino); + FileRef f = _get_file(fnode.ino); f->fnode = fnode; if (fnode.ino > ino_last) { ino_last = fnode.ino; @@ -509,7 +506,6 @@ int BlueFS::_replay() << dendl; auto p = file_map.find(ino); assert(p != file_map.end()); - delete p->second; file_map.erase(p); } break; @@ -534,12 +530,11 @@ int BlueFS::_replay() return 0; } -BlueFS::File *BlueFS::_get_file(uint64_t ino) +BlueFS::FileRef BlueFS::_get_file(uint64_t ino) { - File *f; auto p = file_map.find(ino); if (p == file_map.end()) { - f = new File; + FileRef f = new File; file_map[ino] = f; dout(30) << __func__ << " ino " << ino << " = " << f << " (new)" << dendl; @@ -550,7 +545,7 @@ BlueFS::File *BlueFS::_get_file(uint64_t ino) } } -void BlueFS::_drop_link(File *file) +void BlueFS::_drop_link(FileRef file) { dout(20) << __func__ << " had refs " << file->refs << " on " << file->fnode << dendl; @@ -562,7 +557,7 @@ void BlueFS::_drop_link(File *file) alloc[r.bdev]->release(r.offset, r.length); } file_map.erase(file->fnode.ino); - delete file; + file->deleted = true; } } @@ -625,7 +620,7 @@ int BlueFS::_read( return r; } -void BlueFS::_invalidate_cache(File *f, uint64_t offset, uint64_t length) +void BlueFS::_invalidate_cache(FileRef f, uint64_t offset, uint64_t length) { dout(10) << __func__ << " file " << f->fnode << " " << offset << "~" << length << dendl; @@ -691,6 +686,7 @@ int BlueFS::_flush_range(FileWriter *h, uint64_t offset, uint64_t length) dout(10) << __func__ << " " << h << " pos " << h->pos << " " << offset << "~" << length << " to " << h->file->fnode << dendl; + assert(!h->file->deleted); if (offset + length <= h->pos) return 0; @@ -885,7 +881,7 @@ int BlueFS::_allocate(unsigned id, uint64_t len, vector *ev) return 0; } -int BlueFS::_preallocate(File *f, uint64_t off, uint64_t len) +int BlueFS::_preallocate(FileRef f, uint64_t off, uint64_t len) { dout(10) << __func__ << " file " << f->fnode << " " << off << "~" << len << dendl; @@ -940,8 +936,8 @@ int BlueFS::open_for_write( dir = p->second; } - File *file; - map::iterator q = dir->file_map.find(filename); + FileRef file; + map::iterator q = dir->file_map.find(filename); if (q == dir->file_map.end()) { if (overwrite) { dout(20) << __func__ << " dir " << dirname << " (" << dir @@ -990,14 +986,14 @@ int BlueFS::open_for_read( } Dir *dir = p->second; - map::iterator q = dir->file_map.find(filename); + map::iterator q = dir->file_map.find(filename); if (q == dir->file_map.end()) { dout(20) << __func__ << " dir " << dirname << " (" << dir << ") file " << filename << " not found" << dendl; return -ENOENT; } - File *file = q->second; + File *file = q->second.get(); *h = new FileReader(file, random ? 4096 : g_conf->bluefs_max_prefetch); dout(10) << __func__ << " h " << *h << " on " << file->fnode << dendl; @@ -1017,14 +1013,14 @@ int BlueFS::rename( return -ENOENT; } Dir *old_dir = p->second; - map::iterator q = old_dir->file_map.find(old_filename); + map::iterator q = old_dir->file_map.find(old_filename); if (q == old_dir->file_map.end()) { dout(20) << __func__ << " dir " << old_dirname << " (" << old_dir << ") file " << old_filename << " not found" << dendl; return -ENOENT; } - File *file = q->second; + FileRef file = q->second; p = dir_map.find(new_dirname); if (p == dir_map.end()) { @@ -1106,14 +1102,14 @@ int BlueFS::stat(const string& dirname, const string& filename, return -ENOENT; } Dir *dir = p->second; - map::iterator q = dir->file_map.find(filename); + map::iterator q = dir->file_map.find(filename); if (q == dir->file_map.end()) { dout(20) << __func__ << " dir " << dirname << " (" << dir << ") file " << filename << " not found" << dendl; return -ENOENT; } - File *file = q->second; + File *file = q->second.get(); if (size) *size = file->fnode.size; if (mtime) @@ -1132,7 +1128,7 @@ int BlueFS::lock_file(const string& dirname, const string& filename, return -ENOENT; } Dir *dir = p->second; - map::iterator q = dir->file_map.find(filename); + map::iterator q = dir->file_map.find(filename); File *file; if (q == dir->file_map.end()) { dout(20) << __func__ << " dir " << dirname << " (" << dir @@ -1147,7 +1143,7 @@ int BlueFS::lock_file(const string& dirname, const string& filename, log_t.op_file_update(file->fnode); log_t.op_dir_link(dirname, filename, file->fnode.ino); } else { - file = q->second; + file = q->second.get(); } if (file->locked) { dout(10) << __func__ << " already locked" << dendl; @@ -1208,13 +1204,13 @@ int BlueFS::unlink(const string& dirname, const string& filename) return -ENOENT; } Dir *dir = p->second; - map::iterator q = dir->file_map.find(filename); + map::iterator q = dir->file_map.find(filename); if (q == dir->file_map.end()) { dout(20) << __func__ << " file " << dirname << "/" << filename << " not found" << dendl; return -ENOENT; } - File *file = q->second; + FileRef file = q->second; dir->file_map.erase(filename); log_t.op_dir_unlink(dirname, filename); _drop_link(file); diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h index 853c784dc41d..633dd2c9c24e 100644 --- a/src/os/bluestore/BlueFS.h +++ b/src/os/bluestore/BlueFS.h @@ -6,27 +6,47 @@ #include "bluefs_types.h" #include "common/Mutex.h" #include "common/Cond.h" +#include "common/RefCountedObj.h" #include "BlockDevice.h" #include "boost/intrusive/list.hpp" +#include class Allocator; class BlueFS { public: - struct File { + struct File : public RefCountedObject { bluefs_fnode_t fnode; int refs; bool dirty; bool locked; + bool deleted; boost::intrusive::list_member_hook<> dirty_item; + atomic_t num_readers, num_writers; + File() - : refs(0), + : RefCountedObject(0), + refs(0), dirty(false), - locked(false) + locked(false), + deleted(false) {} + ~File() { + assert(num_readers.read() == 0); + assert(num_writers.read() == 0); + } + + friend void intrusive_ptr_add_ref(File *f) { + f->get(); + } + friend void intrusive_ptr_release(File *f) { + f->put(); + } }; + typedef boost::intrusive_ptr FileRef; + typedef boost::intrusive::list< File, boost::intrusive::member_hook< @@ -35,16 +55,21 @@ public: &File::dirty_item> > dirty_file_list_t; struct Dir { - map file_map; + map file_map; }; struct FileWriter { - File *file; + FileRef file; uint64_t pos; ///< start offset for buffer bufferlist buffer; ///< new data to write (at end of file) bufferlist tail_block; ///< existing partial block at end of file, if any - FileWriter(File *f) : file(f), pos(0) {} + FileWriter(FileRef f) : file(f), pos(0) { + file->num_writers.inc(); + } + ~FileWriter() { + file->num_writers.dec(); + } void append(const char *buf, size_t len) { buffer.append(buf, len); @@ -58,7 +83,7 @@ public: }; struct FileReader { - File *file; + FileRef file; uint64_t pos; ///< current logical offset uint64_t max_prefetch; ///< max allowed prefetch bool ignore_eof; ///< used when reading our log file @@ -67,14 +92,18 @@ public: uint64_t bl_off; ///< prefetch buffer logical offset bufferlist bl; ///< prefetch buffer - FileReader(File *f, uint64_t mpf, bool ie = false) + FileReader(FileRef f, uint64_t mpf, bool ie = false) : file(f), pos(0), max_prefetch(mpf), ignore_eof(ie), lock("BlueFS::FileReader::lock"), - bl_off(0) - { } + bl_off(0) { + file->num_readers.inc(); + } + ~FileReader() { + file->num_readers.dec(); + } uint64_t get_buf_end() { return bl_off + bl.length(); @@ -94,8 +123,8 @@ public: }; struct FileLock { - File *file; - FileLock(File *f) : file(f) {} + FileRef file; + FileLock(FileRef f) : file(f) {} }; private: @@ -103,9 +132,9 @@ private: Cond cond; // cache - map dir_map; ///< dirname -> Dir - ceph::unordered_map file_map; ///< ino -> File - dirty_file_list_t dirty_files; ///< list of dirty files + map dir_map; ///< dirname -> Dir + ceph::unordered_map file_map; ///< ino -> File + dirty_file_list_t dirty_files; ///< list of dirty files bluefs_super_t super; ///< latest superblock (as last written) uint64_t ino_last; ///< last assigned ino (this one is in use) @@ -120,8 +149,8 @@ private: void _init_alloc(); - File *_get_file(uint64_t ino); - void _drop_link(File *f); + FileRef _get_file(uint64_t ino); + void _drop_link(FileRef f); int _allocate(unsigned bdev, uint64_t len, vector *ev); int _flush_range(FileWriter *h, uint64_t offset, uint64_t length); @@ -132,7 +161,7 @@ private: void _submit_bdev(); void _flush_bdev(); - int _preallocate(File *f, uint64_t off, uint64_t len); + int _preallocate(FileRef f, uint64_t off, uint64_t len); int _truncate(FileWriter *h, uint64_t off); int _read( @@ -142,7 +171,7 @@ private: bufferptr *bp, ///< [out] optional: reference the result here char *out); ///< [out] optional: or copy it here - void _invalidate_cache(File *f, uint64_t offset, uint64_t length); + void _invalidate_cache(FileRef f, uint64_t offset, uint64_t length); int _open_super(uint64_t super_offset_a, uint64_t super_offset_b); int _write_super(); @@ -221,11 +250,11 @@ public: Mutex::Locker l(lock); return _read(h, offset, len, bp, out); } - void invalidate_cache(File *f, uint64_t offset, uint64_t len) { + void invalidate_cache(FileRef f, uint64_t offset, uint64_t len) { Mutex::Locker l(lock); _invalidate_cache(f, offset, len); } - int preallocate(File *f, uint64_t offset, uint64_t len) { + int preallocate(FileRef f, uint64_t offset, uint64_t len) { Mutex::Locker l(lock); return _preallocate(f, offset, len); }