Commit 436c119d authored by Derek Basehore's avatar Derek Basehore Committed by Commit Bot

Disable unmount-on-suspend for FUSE

This removes the code for unmounting the FUSEs (filesystem in userspace)
on suspend. Instead of unmounting to resolve the FUSE/freeze on suspend
race condition, we will be using powerd code that orders freeze using
freezer cgroups.

Crostini still has some work left to do before this can be done for it,
so unmount on suspend for its FUSE, sshfs, will be done in a later
patch.

Test: "suspend_stress_test -c 2500" on device
Bug: b/134792837
Change-Id: Icb8f9e309f6780e6f935be550c6c4e048a44b1fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2336869
Commit-Queue: Derek Basehore <dbasehore@chromium.org>
Reviewed-by: default avatarJoel Hockey <joelhockey@chromium.org>
Reviewed-by: default avatarSergei Datsenko <dats@chromium.org>
Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Reviewed-by: default avatarDavid Munro <davidmunro@google.com>
Cr-Commit-Position: refs/heads/master@{#811950}
parent fd165f29
......@@ -566,8 +566,7 @@ DriveIntegrationService::DriveIntegrationService(
drivefs_holder_(std::make_unique<DriveFsHolder>(
profile_,
this,
std::move(test_drivefs_mojo_listener_factory))),
power_manager_observer_(this) {
std::move(test_drivefs_mojo_listener_factory))) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(profile && !profile->IsOffTheRecord());
......@@ -592,10 +591,6 @@ DriveIntegrationService::DriveIntegrationService(
blocking_task_runner_.get()));
}
// PowerManagerClient is unset in unit tests.
if (chromeos::PowerManagerClient::Get()) {
power_manager_observer_.Add(chromeos::PowerManagerClient::Get());
}
SetEnabled(drive::util::IsDriveEnabledForProfile(profile));
}
......@@ -1028,21 +1023,6 @@ void DriveIntegrationService::PinFiles(
profile_->GetPrefs()->SetBoolean(prefs::kDriveFsPinnedMigrated, true);
}
void DriveIntegrationService::SuspendImminent(
power_manager::SuspendImminent::Reason reason) {
// This may a bit racy since it doesn't prevent suspend until the unmount is
// completed, instead relying on something else to defer suspending long
// enough.
RemoveDriveMountPoint();
}
void DriveIntegrationService::SuspendDone(
const base::TimeDelta& sleep_duration) {
if (is_enabled()) {
AddDriveMountPoint();
}
}
void DriveIntegrationService::GetQuickAccessItems(
int max_number,
GetQuickAccessItemsCallback callback) {
......
......@@ -18,7 +18,6 @@
#include "base/observer_list_types.h"
#include "base/scoped_observer.h"
#include "chromeos/components/drivefs/drivefs_host.h"
#include "chromeos/dbus/power/power_manager_client.h"
#include "components/drive/drive_notification_observer.h"
#include "components/drive/file_errors.h"
#include "components/drive/file_system_core_util.h"
......@@ -91,8 +90,7 @@ class DriveIntegrationServiceObserver : public base::CheckedObserver {
// that are used to integrate Drive to Chrome. The object of this class is
// created per-profile.
class DriveIntegrationService : public KeyedService,
public drivefs::DriveFsHost::MountObserver,
public chromeos::PowerManagerClient::Observer {
public drivefs::DriveFsHost::MountObserver {
public:
class PreferenceWatcher;
using DriveFsMojoListenerFactory = base::RepeatingCallback<
......@@ -271,10 +269,6 @@ class DriveIntegrationService : public KeyedService,
// Pin all the files in |files_to_pin| with DriveFS.
void PinFiles(const std::vector<base::FilePath>& files_to_pin);
// chromeos::PowerManagerClient::Observer overrides:
void SuspendImminent(power_manager::SuspendImminent::Reason reason) override;
void SuspendDone(const base::TimeDelta& sleep_duration) override;
void OnGetQuickAccessItems(
GetQuickAccessItemsCallback callback,
drive::FileError error,
......@@ -306,10 +300,6 @@ class DriveIntegrationService : public KeyedService,
base::TimeTicks mount_start_;
ScopedObserver<chromeos::PowerManagerClient,
chromeos::PowerManagerClient::Observer>
power_manager_observer_{this};
// Note: This should remain the last member so it'll be destroyed and
// invalidate its weak pointers before any other members are destroyed.
base::WeakPtrFactory<DriveIntegrationService> weak_ptr_factory_{this};
......
......@@ -182,9 +182,6 @@ SmbService::SmbService(Profile* profile,
SmbService::~SmbService() {
net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
if (chromeos::PowerManagerClient::Get()) {
chromeos::PowerManagerClient::Get()->RemoveObserver(this);
}
}
void SmbService::Shutdown() {
......@@ -817,9 +814,6 @@ void SmbService::CompleteSetup() {
base::Unretained(this))));
RestoreMounts();
net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
if (chromeos::PowerManagerClient::Get()) {
chromeos::PowerManagerClient::Get()->AddObserver(this);
}
if (setup_complete_callback_) {
std::move(setup_complete_callback_).Run();
......@@ -1023,53 +1017,5 @@ void SmbService::RecordMountCount() const {
file_systems.size() + smbfs_shares_.size());
}
void SmbService::SuspendImminent(
power_manager::SuspendImminent::Reason reason) {
for (auto it = smbfs_shares_.begin(); it != smbfs_shares_.end(); ++it) {
SmbFsShare* share = it->second.get();
// For each share, block suspend until the unmount has completed, to ensure
// that no smbfs instances are active when the system goes to sleep.
auto token = base::UnguessableToken::Create();
chromeos::PowerManagerClient::Get()->BlockSuspend(token, "SmbService");
share->Unmount(
base::BindOnce(&SmbService::OnSuspendUnmountDone, AsWeakPtr(), token));
}
}
void SmbService::OnSuspendUnmountDone(
base::UnguessableToken power_manager_suspend_token,
chromeos::MountError result) {
LOG_IF(ERROR, result != chromeos::MountError::MOUNT_ERROR_NONE)
<< "Could not unmount smbfs share during suspension: "
<< static_cast<int>(result);
// Regardless of the outcome, unblock suspension for this share.
chromeos::PowerManagerClient::Get()->UnblockSuspend(
power_manager_suspend_token);
}
void SmbService::SuspendDone(const base::TimeDelta& sleep_duration) {
for (auto it = smbfs_shares_.begin(); it != smbfs_shares_.end(); ++it) {
SmbFsShare* share = it->second.get();
const std::string mount_id = share->mount_id();
// Don't try to reconnect as we race the network stack in getting an IP
// address.
SmbFsShare::MountOptions options = share->options();
options.skip_connect = true;
// Observing power management changes from SmbService allows us to remove
// the share in OnSmbfsMountDone if remount fails.
share->Remount(
options, base::BindOnce(
&SmbService::OnSmbfsMountDone, AsWeakPtr(), mount_id,
base::BindOnce([](SmbMountResult result,
const base::FilePath& mount_path) {
LOG_IF(ERROR, result != SmbMountResult::kSuccess)
<< "Error remounting smbfs share after suspension: "
<< static_cast<int>(result);
})));
}
}
} // namespace smb_client
} // namespace chromeos
......@@ -25,7 +25,6 @@
#include "chrome/browser/chromeos/smb_client/smb_task_queue.h"
#include "chrome/browser/chromeos/smb_client/smbfs_share.h"
#include "chrome/browser/profiles/profile.h"
#include "chromeos/dbus/power/power_manager_client.h"
#include "chromeos/dbus/smb_provider_client.h"
#include "components/keyed_service/core/keyed_service.h"
#include "net/base/network_change_notifier.h"
......@@ -49,7 +48,6 @@ class SmbShareInfo;
// Creates and manages an smb file system.
class SmbService : public KeyedService,
public net::NetworkChangeNotifier::NetworkChangeObserver,
public chromeos::PowerManagerClient::Observer,
public base::SupportsWeakPtr<SmbService> {
public:
using MountResponse = base::OnceCallback<void(SmbMountResult result)>;
......@@ -126,10 +124,6 @@ class SmbService : public KeyedService,
void SetSmbFsMounterCreationCallbackForTesting(
SmbFsShare::MounterCreationCallback callback);
// chromeos::PowerManagerClient::Observer overrides
void SuspendImminent(power_manager::SuspendImminent::Reason reason) override;
void SuspendDone(const base::TimeDelta& sleep_duration) override;
private:
friend class SmbServiceTest;
......@@ -170,11 +164,6 @@ class SmbService : public KeyedService,
MountInternalCallback callback,
SmbMountResult result);
// Callback passed to SmbFsShare::Unmount() during a power management
// suspension. Ensures that suspension is blocked until the unmount completes.
void OnSuspendUnmountDone(base::UnguessableToken power_manager_suspend_token,
chromeos::MountError result);
// Retrieves the mount_id for |file_system_info|.
int32_t GetMountId(const ProvidedFileSystemInfo& info) const;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment