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

Commit 7bb484d4 authored by Akilesh Kailash's avatar Akilesh Kailash
Browse files

snapuserd: Lock the buffer during snapshot-merge 



Bug: 377819507
Test: Incremental OTA on Pixel
Change-Id: I08fa7129282cc005a565987856166088c092f40a
Signed-off-by: default avatarAkilesh Kailash <akailash@google.com>
parent b6d84eba

fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp

+16 −0
Original line number Diff line number Diff line
@@ -233,6 +233,11 @@ bool MergeWorker::MergeOrderedOpsAsync() { 
            return false;

        }



        std::optional<std::lock_guard<std::mutex>> buffer_lock;

        // Acquire the buffer lock at this point so that RA thread

        // doesn't step into this buffer. See b/377819507

        buffer_lock.emplace(snapuserd_->GetBufferLock());



        snapuserd_->SetMergeInProgress(ra_block_index_);



        loff_t offset = 0;
@@ -383,6 +388,9 @@ bool MergeWorker::MergeOrderedOpsAsync() { 
        // Mark the block as merge complete

        snapuserd_->SetMergeCompleted(ra_block_index_);



        // Release the buffer lock

        buffer_lock.reset();



        // Notify RA thread that the merge thread is ready to merge the next

        // window

        snapuserd_->NotifyRAForMergeReady();
@@ -415,6 +423,11 @@ bool MergeWorker::MergeOrderedOps() { 
            return false;

        }



        std::optional<std::lock_guard<std::mutex>> buffer_lock;

        // Acquire the buffer lock at this point so that RA thread

        // doesn't step into this buffer. See b/377819507

        buffer_lock.emplace(snapuserd_->GetBufferLock());



        snapuserd_->SetMergeInProgress(ra_block_index_);



        loff_t offset = 0;
@@ -468,6 +481,9 @@ bool MergeWorker::MergeOrderedOps() { 
        // Mark the block as merge complete

        snapuserd_->SetMergeCompleted(ra_block_index_);



        // Release the buffer lock

        buffer_lock.reset();



        // Notify RA thread that the merge thread is ready to merge the next

        // window

        snapuserd_->NotifyRAForMergeReady();

fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.h

+4 −0
Original line number Diff line number Diff line
@@ -186,6 +186,7 @@ class SnapshotHandler : public std::enable_shared_from_this<SnapshotHandler> { 


    bool IsIouringSupported();

    bool CheckPartitionVerification();

    std::mutex& GetBufferLock() { return buffer_lock_; }



  private:

    bool ReadMetadata();
@@ -216,6 +217,9 @@ class SnapshotHandler : public std::enable_shared_from_this<SnapshotHandler> { 
    std::mutex lock_;

    std::condition_variable cv;



    // Lock the buffer used for snapshot-merge

    std::mutex buffer_lock_;



    void* mapped_addr_;

    size_t total_mapped_addr_length_;

fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp

+23 −16
Original line number Diff line number Diff line
@@ -706,6 +706,12 @@ bool ReadAhead::ReadAheadIOStart() { 
        return false;

    }



    // Acquire buffer lock before doing memcpy to the scratch buffer. Although,

    // by now snapshot-merge thread shouldn't be working on this scratch space

    // but we take additional measure to ensure that the buffer is not being

    // used by the merge thread at this point. see b/377819507

    {

        std::lock_guard<std::mutex> buffer_lock(snapuserd_->GetBufferLock());

        // Copy the data to scratch space

        memcpy(metadata_buffer_, ra_temp_meta_buffer_.get(), snapuserd_->GetBufferMetadataSize());

        memcpy(read_ahead_buffer_, ra_temp_buffer_.get(), total_blocks_merged_ * BLOCK_SZ);
@@ -724,6 +730,7 @@ bool ReadAhead::ReadAheadIOStart() { 


        total_ra_blocks_completed_ += total_blocks_merged_;

        snapuserd_->SetMergedBlockCountForNextCommit(total_blocks_merged_);

    }



    // Flush the scratch data - Technically, we should flush only for overlapping

    // blocks; However, since this region is mmap'ed, the dirty pages can still