Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8e8fe529 authored by Yiwei Zhang's avatar Yiwei Zhang
Browse files

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
parent c3930937

services/surfaceflinger/BufferStateLayer.cpp

+1 −2
Original line number Diff line number Diff line
@@ -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);

services/surfaceflinger/Layer.cpp

+2 −3
Original line number Diff line number Diff line
@@ -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);

services/surfaceflinger/TimeStats/TimeStats.cpp

+4 −11
Original line number Diff line number Diff line
@@ -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) {

services/surfaceflinger/TimeStats/TimeStats.h

+2 −2
Original line number Diff line number Diff line
@@ -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);