facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-07 15:27:13 +03:00
Code Issues Projects Releases Wiki Activity

Remove FileInode::parentInode_

Browse Source 
Summary:
This member variable was not updated properly when files were renamed.
InodeBase now tracks our parent properly, so we don't need our own copy.

This does still call getParentBuggy() (which does not perform proper locking)
in a couple places for performing overlay operations.  We'll need to fix this
later when addressing other overlay concurrency handling issues.

Reviewed By: bolinfest

Differential Revision: D4348481

fbshipit-source-id: 19c1ffced6f63e1ff041d0bab2363fecdb93d5a3
This commit is contained in:
Adam Simpkins 2016-12-22 15:35:01 -08:00 committed by Facebook Github Bot
parent 9ba08b9d1e
commit 3e41ecaa26
3 changed files with 23 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
eden/fs/inodes/FileHandle.cpp
View File

@ -57,7 +57,7 @@ folly::Future<fusell::BufVec> FileHandle::read(size_t size, off_t off) {
folly::Future<size_t> FileHandle::write(fusell::BufVec&& buf, off_t off) {
SCOPE_SUCCESS {
auto myname = inode_->getPathBuggy();
inode_->parentInode_->getMount()->getJournal().wlock()->addDelta(
inode_->getMount()->getJournal().wlock()->addDelta(
std::make_unique<JournalDelta>(JournalDelta{myname}));
};
return data_->write(std::move(buf), off);
@ -66,7 +66,7 @@ folly::Future<size_t> FileHandle::write(fusell::BufVec&& buf, off_t off) {
folly::Future<size_t> FileHandle::write(folly::StringPiece str, off_t off) {
SCOPE_SUCCESS {
auto myname = inode_->getPathBuggy();
inode_->parentInode_->getMount()->getJournal().wlock()->addDelta(
inode_->getMount()->getJournal().wlock()->addDelta(
std::make_unique<JournalDelta>(JournalDelta{myname}));
};
return data_->write(str, off);

30
eden/fs/inodes/FileInode.cpp
View File

@ -33,8 +33,7 @@ FileInode::FileInode(
TreeInodePtr parentInode,
PathComponentPiece name,
TreeInode::Entry* entry)
: InodeBase(ino, parentInode, name),
parentInode_(parentInode),
: InodeBase(ino, std::move(parentInode), name),
entry_(entry),
data_(std::make_shared<FileData>(this, mutex_, entry)) {}
@ -44,8 +43,7 @@ FileInode::FileInode(
PathComponentPiece name,
TreeInode::Entry* entry,
folly::File&& file)
: InodeBase(ino, parentInode, name),
parentInode_(parentInode),
: InodeBase(ino, std::move(parentInode), name),
entry_(entry),
data_(std::make_shared<FileData>(this, mutex_, entry_, std::move(file))) {
}
@ -58,10 +56,10 @@ folly::Future<fusell::Dispatcher::Attr> FileInode::getattr() {
// materialized the data from the entry_, we have to materialize it
// from the store. If we augmented our metadata we could avoid this,
// and this would speed up operations like `ls`.
auto overlay = parentInode_->getOverlay();
auto overlay = getMount()->getOverlay();
data->materializeForRead(O_RDONLY, path, overlay);
fusell::Dispatcher::Attr attr(parentInode_->getMount()->getMountPoint());
fusell::Dispatcher::Attr attr(getMount()->getMountPoint());
attr.st = data->stat();
attr.st.st_ino = getNodeId();
return attr;
@ -80,17 +78,17 @@ folly::Future<fusell::Dispatcher::Attr> FileInode::setattr(
open_flags |= O_TRUNC;
}
parentInode_->materializeDirAndParents();
getParentBuggy()->materializeDirAndParents();
auto path = getPathBuggy();
auto overlay = parentInode_->getOverlay();
auto overlay = getMount()->getOverlay();
data->materializeForWrite(open_flags, path, overlay);
fusell::Dispatcher::Attr result(parentInode_->getMount()->getMountPoint());
fusell::Dispatcher::Attr result(getMount()->getMountPoint());
result.st = data->setAttr(attr, to_set);
result.st.st_ino = getNodeId();
parentInode_->getMount()->getJournal().wlock()->addDelta(
getMount()->getJournal().wlock()->addDelta(
std::make_unique<JournalDelta>(JournalDelta{path}));
return result;
@ -126,7 +124,7 @@ folly::Future<std::string> FileInode::readlink() {
}
// Load the symlink contents from the store
auto blob = parentInode_->getStore()->getBlob(entry_->hash.value());
auto blob = getMount()->getObjectStore()->getBlob(entry_->hash.value());
auto buf = blob->getContents();
return buf.moveToFbString().toStdString();
}
@ -149,7 +147,7 @@ void FileInode::fileHandleDidClose() {
}
AbsolutePath FileInode::getLocalPath() const {
return parentInode_->getOverlay()->getContentDir() + getPathBuggy();
return getMount()->getOverlay()->getContentDir() + getPathBuggy();
}
folly::Future<std::shared_ptr<fusell::FileHandle>> FileInode::open(
@ -159,9 +157,9 @@ folly::Future<std::shared_ptr<fusell::FileHandle>> FileInode::open(
data.reset();
fileHandleDidClose();
};
auto overlay = parentInode_->getOverlay();
auto overlay = getMount()->getOverlay();
if (fi.flags & (O_RDWR | O_WRONLY | O_CREAT | O_TRUNC)) {
parentInode_->materializeDirAndParents();
getParentBuggy()->materializeDirAndParents();
data->materializeForWrite(fi.flags, getPathBuggy(), overlay);
} else {
data->materializeForRead(fi.flags, getPathBuggy(), overlay);
@ -177,7 +175,7 @@ std::shared_ptr<FileHandle> FileInode::finishCreate() {
data.reset();
fileHandleDidClose();
};
data->materializeForWrite(0, getPathBuggy(), parentInode_->getOverlay());
data->materializeForWrite(0, getPathBuggy(), getMount()->getOverlay());
return std::make_shared<FileHandle>(
std::static_pointer_cast<FileInode>(shared_from_this()), data, 0);
@ -233,7 +231,7 @@ Future<Hash> FileInode::getSHA1() {
// TODO(mbolin): Make this more fault-tolerant. Currently, there is no logic
// to account for the case where we don't have the SHA-1 for the blob, the
// hash doesn't correspond to a blob, etc.
return parentInode_->getStore()->getSha1ForBlob(entry_->hash.value());
return getMount()->getObjectStore()->getSha1ForBlob(entry_->hash.value());
}
const TreeInode::Entry* FileInode::getEntry() const {

15
eden/fs/inodes/FileInode.h
View File

@ -74,14 +74,13 @@ class FileInode : public InodeBase {
/// Compute the path to the overlay file for this item.
AbsolutePath getLocalPath() const;
// We hold the ref on the parentInode so that entry_ remains
// valid while we're both alive
//
// TODO: parentInode_ is accessed without locking.
// It also does not appear to be updated on rename.
// We should update uses of parentInode_ with InodeBase::location_ instead,
// and then delete parentInode_.
TreeInodePtr parentInode_;
/**
* Our Entry in our parent TreeInode's contents_
*
* TODO: We need to replace this with our own copy. As-is we should never
* access this without holding our parent's contents_ lock, which we aren't
* doing correctly.
*/
TreeInode::Entry* entry_;
std::shared_ptr<FileData> data_;