diff --git a/storaged/include/storaged_uid_monitor.h b/storaged/include/storaged_uid_monitor.h index b56e71aac..0c03402a0 100644 --- a/storaged/include/storaged_uid_monitor.h +++ b/storaged/include/storaged_uid_monitor.h @@ -79,6 +79,8 @@ struct uid_records { class uid_monitor { private: FRIEND_TEST(storaged_test, uid_monitor); + FRIEND_TEST(storaged_test, load_uid_io_proto); + // last dump from /proc/uid_io/stats, uid -> uid_info unordered_map last_uid_io_stats_; // current io usage for next report, app name -> uid_io_usage @@ -118,7 +120,7 @@ public: bool enabled() { return enabled_; }; void report(unordered_map* protos); // restores io_history from protobuf - void load_uid_io_proto(const UidIOUsage& proto); + void load_uid_io_proto(userid_t user_id, const UidIOUsage& proto); void clear_user_history(userid_t user_id); map& io_history() { return io_history_; } diff --git a/storaged/storaged.cpp b/storaged/storaged.cpp index f346c387b..77c6167de 100644 --- a/storaged/storaged.cpp +++ b/storaged/storaged.cpp @@ -194,7 +194,7 @@ void storaged_t::load_proto(userid_t user_id) { return; } - mUidm.load_uid_io_proto(proto.uid_io_usage()); + mUidm.load_uid_io_proto(user_id, proto.uid_io_usage()); if (user_id == USER_SYSTEM) { storage_info->load_perf_history_proto(proto.perf_history()); diff --git a/storaged/storaged_uid_monitor.cpp b/storaged/storaged_uid_monitor.cpp index 32b1568de..d5f2fe09f 100644 --- a/storaged/storaged_uid_monitor.cpp +++ b/storaged/storaged_uid_monitor.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -468,7 +469,7 @@ void uid_monitor::clear_user_history(userid_t user_id) } } -void uid_monitor::load_uid_io_proto(const UidIOUsage& uid_io_proto) +void uid_monitor::load_uid_io_proto(userid_t user_id, const UidIOUsage& uid_io_proto) { if (!enabled()) return; @@ -478,8 +479,23 @@ void uid_monitor::load_uid_io_proto(const UidIOUsage& uid_io_proto) const UidIORecords& records_proto = item_proto.records(); struct uid_records* recs = &io_history_[item_proto.end_ts()]; + // It's possible that the same uid_io_proto file gets loaded more than + // once, for example, if system_server crashes. In this case we avoid + // adding duplicate entries, so we build a quick way to check for + // duplicates. + std::unordered_set existing_uids; + for (const auto& rec : recs->entries) { + if (rec.ios.user_id == user_id) { + existing_uids.emplace(rec.name); + } + } + recs->start_ts = records_proto.start_ts(); for (const auto& rec_proto : records_proto.entries()) { + if (existing_uids.find(rec_proto.uid_name()) != existing_uids.end()) { + continue; + } + struct uid_record record; record.name = rec_proto.uid_name(); record.ios.user_id = rec_proto.user_id(); diff --git a/storaged/tests/storaged_test.cpp b/storaged/tests/storaged_test.cpp index e08c9ee6c..d66746dee 100644 --- a/storaged/tests/storaged_test.cpp +++ b/storaged/tests/storaged_test.cpp @@ -551,8 +551,8 @@ TEST(storaged_test, uid_monitor) { }, }; - uidm.load_uid_io_proto(protos[0].uid_io_usage()); - uidm.load_uid_io_proto(protos[1].uid_io_usage()); + uidm.load_uid_io_proto(0, protos[0].uid_io_usage()); + uidm.load_uid_io_proto(1, protos[1].uid_io_usage()); EXPECT_EQ(io_history.size(), 3UL); EXPECT_EQ(io_history.count(200), 1UL); @@ -627,3 +627,40 @@ TEST(storaged_test, uid_monitor) { EXPECT_EQ(uidm.io_history_.size(), 0UL); } + +TEST(storaged_test, load_uid_io_proto) { + uid_monitor uidm; + + uidm.io_history_[200] = { + .start_ts = 100, + .entries = { + { "app1", { + .user_id = 0, + .uid_ios.bytes[WRITE][FOREGROUND][CHARGER_ON] = 1000, + } + }, + { "app2", { + .user_id = 0, + .uid_ios.bytes[READ][FOREGROUND][CHARGER_OFF] = 2000, + } + }, + { "app3", { + .user_id = 0, + .uid_ios.bytes[READ][FOREGROUND][CHARGER_OFF] = 3000, + } + }, + }, + }; + + unordered_map protos; + uidm.update_uid_io_proto(&protos); + ASSERT_EQ(protos.size(), size_t(1)); + + // Loading the same proto many times should not add duplicate entries. + const UidIOUsage& user_0 = protos[0].uid_io_usage(); + for (size_t i = 0; i < 10000; i++) { + uidm.load_uid_io_proto(0, user_0); + } + ASSERT_EQ(uidm.io_history_.size(), size_t(1)); + ASSERT_EQ(uidm.io_history_[200].entries.size(), size_t(3)); +}