diff options
| author | 2018-11-02 18:34:07 -0700 | |
|---|---|---|
| committer | 2018-11-02 18:43:01 -0700 | |
| commit | 8e8fe529a69761aad6a84fb8025cebd3673d77db (patch) | |
| tree | 13b8f06e4a81d4318abc98cfea8b90bc968400d0 | |
| parent | c39309375b269bce3a68dadd0bc1d5bc29bc57c0 (diff) | |
SF TimeStats: fix a hazard issue
The setAcquireFence and setDesiredTime could happen in between the
splited setLayerName and setPostTime, where setLayerName inserts an
entry but setPostTime actually initializes the structure and waitData
position. This change combine setLayerName and setPostTime again to
eliminate the hazard.
Test: build
Bug: b/118770127
Change-Id: Ia1961cb688bbabf23d88305750e69c9fa41da8dd
| -rw-r--r-- | services/surfaceflinger/BufferStateLayer.cpp | 3 | ||||
| -rw-r--r-- | services/surfaceflinger/Layer.cpp | 5 | ||||
| -rw-r--r-- | services/surfaceflinger/TimeStats/TimeStats.cpp | 15 | ||||
| -rw-r--r-- | services/surfaceflinger/TimeStats/TimeStats.h | 4 |
4 files changed, 9 insertions, 18 deletions
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index 5df8ade34e..73098bf2aa 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -470,8 +470,7 @@ status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nse } // TODO(marissaw): properly support mTimeStats - mTimeStats.setLayerName(layerID, getName().c_str()); - mTimeStats.setPostTime(layerID, getFrameNumber(), latchTime); + mTimeStats.setPostTime(layerID, getFrameNumber(), getName().c_str(), latchTime); mTimeStats.setAcquireFence(layerID, getFrameNumber(), getCurrentFenceTime()); mTimeStats.setLatchTime(layerID, getFrameNumber(), latchTime); diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 88c3c8a018..2e564e7d94 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -1476,9 +1476,8 @@ void Layer::onDisconnect() { void Layer::addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps, FrameEventHistoryDelta* outDelta) { if (newTimestamps) { - const int32_t layerID = getSequence(); - mTimeStats.setLayerName(layerID, getName().c_str()); - mTimeStats.setPostTime(layerID, newTimestamps->frameNumber, newTimestamps->postedTime); + mTimeStats.setPostTime(getSequence(), newTimestamps->frameNumber, getName().c_str(), + newTimestamps->postedTime); } Mutex::Autolock lock(mFrameEventHistoryMutex); diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp index c219afd85d..ace7c1b454 100644 --- a/services/surfaceflinger/TimeStats/TimeStats.cpp +++ b/services/surfaceflinger/TimeStats/TimeStats.cpp @@ -241,25 +241,18 @@ static bool layerNameIsValid(const std::string& layerName) { return std::regex_match(layerName.begin(), layerName.end(), layerNameRegex); } -void TimeStats::setLayerName(int32_t layerID, const std::string& layerName) { +void TimeStats::setPostTime(int32_t layerID, uint64_t frameNumber, const std::string& layerName, + nsecs_t postTime) { if (!mEnabled.load()) return; ATRACE_CALL(); - ALOGV("[%d]-[%s]", layerID, layerName.c_str()); + ALOGV("[%d]-[%" PRIu64 "]-[%s]-PostTime[%" PRId64 "]", layerID, frameNumber, layerName.c_str(), + postTime); std::lock_guard<std::mutex> lock(mMutex); if (!mTimeStatsTracker.count(layerID) && layerNameIsValid(layerName)) { mTimeStatsTracker[layerID].layerName = layerName; } -} - -void TimeStats::setPostTime(int32_t layerID, uint64_t frameNumber, nsecs_t postTime) { - if (!mEnabled.load()) return; - - ATRACE_CALL(); - ALOGV("[%d]-[%" PRIu64 "]-PostTime[%" PRId64 "]", layerID, frameNumber, postTime); - - std::lock_guard<std::mutex> lock(mMutex); if (!mTimeStatsTracker.count(layerID)) return; LayerRecord& layerRecord = mTimeStatsTracker[layerID]; if (layerRecord.timeRecords.size() == MAX_NUM_TIME_RECORDS) { diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h index d1e554cbd2..184bf40967 100644 --- a/services/surfaceflinger/TimeStats/TimeStats.h +++ b/services/surfaceflinger/TimeStats/TimeStats.h @@ -86,8 +86,8 @@ public: void incrementMissedFrames(); void incrementClientCompositionFrames(); - void setLayerName(int32_t layerID, const std::string& layerName); - void setPostTime(int32_t layerID, uint64_t frameNumber, nsecs_t postTime); + void setPostTime(int32_t layerID, uint64_t frameNumber, const std::string& layerName, + nsecs_t postTime); void setLatchTime(int32_t layerID, uint64_t frameNumber, nsecs_t latchTime); void setDesiredTime(int32_t layerID, uint64_t frameNumber, nsecs_t desiredTime); void setAcquireTime(int32_t layerID, uint64_t frameNumber, nsecs_t acquireTime); |