Improve transcoded FUSE cache/passthrough access
This change updates the node#transforms_complete flag on each lookup.
With the correct transforms_complete flag we can ensure the kernel
page cache has the correct bytes after the transcode file cache has
been cleared.
We achieve this by flusing the page cache on FUSE_OPEN
if transforms is not complete. This fixes the case where the following
happens:
1. Read transcoded content and set transforms_complete=true
2. Clear the transcoded cache
3. Read transcoded content again
Lastly, we also set direct_io=true if transforms_complete=false. This
will allow us handle transcode failures more gracefully by returning
the original bytes without corrupting the kernel page cache
Test: atest TranscodeTest
Bug: 181846007
Change-Id: Ibb93a79f2544688bf440180d9e1dbae61f76d287
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index 74429f9..ac9f7e5 100755
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -495,6 +495,12 @@
std::thread t([=]() { fuse_inval(fuse->se, parent_ino, child_ino, node_name, path); });
t.detach();
}
+
+ // This updated value allows us correctly decide if to keep_cache and use direct_io during
+ // FUSE_OPEN. Between the last lookup and this lookup, we might have deleted a cached
+ // transcoded file on the lower fs. A subsequent transcode at FUSE_READ should ensure we
+ // don't reuse any stale transcode page cache content.
+ node->SetTransformsComplete(transforms_complete);
}
TRACE_NODE(node, req);
@@ -1044,12 +1050,13 @@
bool redaction_needed = ri->isRedactionNeeded();
handle* handle = nullptr;
int transforms = node->GetTransforms();
+ bool transforms_complete = node->IsTransformsComplete();
if (transforms_uid > 0) {
CHECK(transforms);
}
if (fuse->passthrough) {
- *keep_cache = 1;
+ *keep_cache = transforms_complete;
// We only enabled passthrough iff these 2 conditions hold
// 1. Redaction is not needed
// 2. Node transforms are completed, e.g transcoding.
@@ -1058,10 +1065,9 @@
// arbitrary bytes the first time around. However, if we ensure that transforms are
// completed, then it's safe to use passthrough. Additionally, transcoded nodes never
// require redaction so (2) implies (1)
- handle = new struct handle(
- fd, ri, true /* cached */,
- !redaction_needed && node->IsTransformsComplete() /* passthrough */, uid,
- transforms_uid);
+ handle = new struct handle(fd, ri, true /* cached */,
+ !redaction_needed && transforms_complete /* passthrough */, uid,
+ transforms_uid);
} else {
// Without fuse->passthrough, we don't want to use the FUSE VFS cache in two cases:
// 1. When redaction is needed because app A with EXIF access might access
@@ -1087,7 +1093,7 @@
// Purges stale page cache before open
*keep_cache = 0;
} else {
- *keep_cache = 1;
+ *keep_cache = transforms_complete;
}
handle = new struct handle(fd, ri, !direct_io /* cached */, false /* passthrough */, uid,
transforms_uid);
@@ -1280,7 +1286,7 @@
fuse_reply_err(req, EFAULT);
return;
}
- node->SetTransformsComplete();
+ node->SetTransformsComplete(true);
}
fuse->fadviser.Record(h->fd, size);
diff --git a/jni/node-inl.h b/jni/node-inl.h
index 9b30e87..e531a0a 100644
--- a/jni/node-inl.h
+++ b/jni/node-inl.h
@@ -285,7 +285,9 @@
return transforms_complete_.load(std::memory_order_acquire);
}
- void SetTransformsComplete() { transforms_complete_.store(true, std::memory_order_release); }
+ void SetTransformsComplete(bool complete) {
+ transforms_complete_.store(complete, std::memory_order_release);
+ }
node* GetParent() const {
std::lock_guard<std::recursive_mutex> guard(*lock_);
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index b20e6da..035d3e7 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -1474,7 +1474,7 @@
if (transformsReason > 0) {
ioPath = mTranscodeHelper.getIoPath(path, uid);
- transformsComplete = false;
+ transformsComplete = mTranscodeHelper.isTranscodeFileCached(path, ioPath);
transforms = FLAG_TRANSFORM_TRANSCODING;
}
}