From 4588dbeecd23c17d1cb7f7fa60afd56702acd455 Mon Sep 17 00:00:00 2001 From: Tomas Bzatek Date: Wed, 5 Jan 2022 20:17:55 +0100 Subject: [PATCH 1/3] udisksata: Move the low-level PM state call --- doc/udisks2-sections.txt.daemon.sections.in | 2 + src/udisksata.c | 62 +++++++++++++++++++++ src/udisksata.h | 13 +++++ 3 files changed, 77 insertions(+) diff --git a/doc/udisks2-sections.txt.daemon.sections.in b/doc/udisks2-sections.txt.daemon.sections.in index 7803ff2b91..4e53992781 100644 --- a/doc/udisks2-sections.txt.daemon.sections.in +++ b/doc/udisks2-sections.txt.daemon.sections.in @@ -269,6 +269,8 @@ UDisksAtaCommandProtocol UDisksAtaCommandInput UDisksAtaCommandOutput udisks_ata_send_command_sync +udisks_ata_get_pm_state +UDISKS_ATA_PM_STATE_AWAKE
diff --git a/src/udisksata.c b/src/udisksata.c index 9491af5e16..e6da8c3588 100644 --- a/src/udisksata.c +++ b/src/udisksata.c @@ -308,3 +308,65 @@ udisks_ata_send_command_sync (gint fd, out: return ret; } + +/** + * udisks_ata_get_pm_state: + * @device: ATA drive block device path. + * @error: Return location for error. + * @pm_state: Return location for the current power state value. + * + * Get the current power mode state. + * + * The format of @pm_state is the result obtained from sending the + * ATA command `CHECK POWER MODE` to the drive. + * + * Known values include: + * - `0x00`: Device is in PM2: Standby state. + * - `0x40`: Device is in the PM0: Active state, the NV Cache power mode is enabled, and the spindle is spun down or spinning down. + * - `0x41`: Device is in the PM0: Active state, the NV Cache power mode is enabled, and the spindle is spun up or spinning up. + * - `0x80`: Device is in PM1: Idle state. + * - `0xff`: Device is in the PM0: Active state or PM1: Idle State. + * + * Typically user interfaces will report "Drive is spun down" if @pm_state is + * 0x00 and "Drive is spun up" otherwise. + * + * Returns: %TRUE if the operation succeeded, %FALSE if @error is set. + */ +gboolean +udisks_ata_get_pm_state (const gchar *device, GError **error, guchar *count) +{ + int fd; + gboolean rc = FALSE; + /* ATA8: 7.8 CHECK POWER MODE - E5h, Non-Data */ + UDisksAtaCommandInput input = {.command = 0xe5}; + UDisksAtaCommandOutput output = {0}; + + g_warn_if_fail (device != NULL); + + fd = open (device, O_RDONLY|O_NONBLOCK); + if (fd == -1) + { + g_set_error (error, UDISKS_ERROR, UDISKS_ERROR_FAILED, + "Error opening device file %s while getting PM state: %m", + device); + goto out; + } + + if (!udisks_ata_send_command_sync (fd, + -1, + UDISKS_ATA_COMMAND_PROTOCOL_NONE, + &input, + &output, + error)) + { + g_prefix_error (error, "Error sending ATA command CHECK POWER MODE: "); + goto out; + } + /* count field is used for the state, see ATA8: table 102 */ + *count = output.count; + rc = TRUE; + out: + if (fd != -1) + close (fd); + return rc; +} diff --git a/src/udisksata.h b/src/udisksata.h index 1d4918f176..d652f3ab8a 100644 --- a/src/udisksata.h +++ b/src/udisksata.h @@ -73,6 +73,16 @@ struct _UDisksAtaCommandOutput guchar *buffer; }; +/** + * UDISKS_ATA_PM_STATE_AWAKE: + * @pm_state: The power state value. + * + * Decodes the power state value as returned by #udisks_ata_get_pm_state. + * + * Returns: %TRUE when the drive is awake, %FALSE when sleeping. +*/ +#define UDISKS_ATA_PM_STATE_AWAKE(pm_state) (pm_state >= 0x41) + gboolean udisks_ata_send_command_sync (gint fd, gint timeout_msec, UDisksAtaCommandProtocol protocol, @@ -80,6 +90,9 @@ gboolean udisks_ata_send_command_sync (gint fd, UDisksAtaCommandOutput *output, GError **error); +gboolean udisks_ata_get_pm_state (const gchar *device, + GError **error, + guchar *count); G_END_DECLS From bb5104b6f612056b15710429ff20229a1d6a2e10 Mon Sep 17 00:00:00 2001 From: Tomas Bzatek Date: Wed, 5 Jan 2022 20:19:10 +0100 Subject: [PATCH 2/3] udiskslinuxdriveata: Move the low-level PM state call away --- doc/udisks2-sections.txt.daemon.sections.in | 1 - src/udiskslinuxdriveata.c | 41 +-------------------- src/udiskslinuxdriveata.h | 10 ----- 3 files changed, 2 insertions(+), 50 deletions(-) diff --git a/doc/udisks2-sections.txt.daemon.sections.in b/doc/udisks2-sections.txt.daemon.sections.in index 4e53992781..d738f14598 100644 --- a/doc/udisks2-sections.txt.daemon.sections.in +++ b/doc/udisks2-sections.txt.daemon.sections.in @@ -161,7 +161,6 @@ udisks_linux_drive_ata_smart_selftest_sync udisks_linux_drive_ata_apply_configuration udisks_linux_drive_ata_secure_erase_sync udisks_linux_drive_ata_get_pm_state -UDISKS_LINUX_DRIVE_ATA_IS_AWAKE UDISKS_LINUX_DRIVE_ATA UDISKS_IS_LINUX_DRIVE_ATA diff --git a/src/udiskslinuxdriveata.c b/src/udiskslinuxdriveata.c index 2f5f40bf11..ebb58a5673 100644 --- a/src/udiskslinuxdriveata.c +++ b/src/udiskslinuxdriveata.c @@ -437,43 +437,6 @@ selftest_status_to_string (SkSmartSelfTestExecutionStatus status) return ret; } -static gboolean -get_pm_state (UDisksLinuxDevice *device, GError **error, guchar *count) -{ - int fd; - gboolean rc = FALSE; - /* ATA8: 7.8 CHECK POWER MODE - E5h, Non-Data */ - UDisksAtaCommandInput input = {.command = 0xe5}; - UDisksAtaCommandOutput output = {0}; - - fd = open (g_udev_device_get_device_file (device->udev_device), O_RDONLY|O_NONBLOCK); - if (fd == -1) - { - g_set_error (error, UDISKS_ERROR, UDISKS_ERROR_FAILED, - "Error opening device file %s while getting PM state: %m", - g_udev_device_get_device_file (device->udev_device)); - goto out; - } - - if (!udisks_ata_send_command_sync (fd, - -1, - UDISKS_ATA_COMMAND_PROTOCOL_NONE, - &input, - &output, - error)) - { - g_prefix_error (error, "Error sending ATA command CHECK POWER MODE: "); - goto out; - } - /* count field is used for the state, see ATA8: table 102 */ - *count = output.count; - rc = TRUE; - out: - if (fd != -1) - close (fd); - return rc; -} - static gboolean update_io_stats (UDisksLinuxDriveAta *drive, UDisksLinuxDevice *device) { const gchar *drivepath = g_udev_device_get_sysfs_path (device->udev_device); @@ -603,7 +566,7 @@ udisks_linux_drive_ata_refresh_smart_sync (UDisksLinuxDriveAta *drive, gboolean noio = FALSE; if (drive->standby_enabled) noio = update_io_stats (drive, device); - if (!get_pm_state (device, error, &count)) + if (!udisks_ata_get_pm_state (g_udev_device_get_device_file (device->udev_device), error, &count)) goto out; awake = count == 0xFF || count == 0x80; /* don't wake up disk unless specically asked to */ @@ -1322,7 +1285,7 @@ udisks_linux_drive_ata_get_pm_state (UDisksLinuxDriveAta *drive, goto out; } - ret = get_pm_state (device, error, pm_state); + ret = udisks_ata_get_pm_state (g_udev_device_get_device_file (device->udev_device), error, pm_state); out: g_clear_object (&device); diff --git a/src/udiskslinuxdriveata.h b/src/udiskslinuxdriveata.h index 51c551b2d4..4dc05d9172 100644 --- a/src/udiskslinuxdriveata.h +++ b/src/udiskslinuxdriveata.h @@ -29,16 +29,6 @@ G_BEGIN_DECLS #define UDISKS_LINUX_DRIVE_ATA(o) (G_TYPE_CHECK_INSTANCE_CAST ((o), UDISKS_TYPE_LINUX_DRIVE_ATA, UDisksLinuxDriveAta)) #define UDISKS_IS_LINUX_DRIVE_ATA(o) (G_TYPE_CHECK_INSTANCE_TYPE ((o), UDISKS_TYPE_LINUX_DRIVE_ATA)) -/** - * UDISKS_LINUX_DRIVE_ATA_IS_AWAKE: - * @pm_state: The power state value. - * - * Decodes the power state value as returned by #udisks_linux_drive_ata_get_pm_state. - * - * Returns: %TRUE when the drive is awake, %FALSE when sleeping. -*/ -#define UDISKS_LINUX_DRIVE_ATA_IS_AWAKE(pm_state) (pm_state >= 0x41) - GType udisks_linux_drive_ata_get_type (void) G_GNUC_CONST; UDisksDriveAta *udisks_linux_drive_ata_new (void); gboolean udisks_linux_drive_ata_update (UDisksLinuxDriveAta *drive, From 9a2a96b46803b1d76d105f3bed994188b8205133 Mon Sep 17 00:00:00 2001 From: Tomas Bzatek Date: Sun, 2 Jan 2022 23:45:12 +0100 Subject: [PATCH 3/3] udiskslinuxfilesystem: Make the 'size' property retrieval on-demand Filesystem size value retrieval is very expensive as it typically calls filesystem tools that read superblock -> doing some I/O. Other filesystem properties are typically retrieved from existing stateful sources, either udev or sysfs. This change overrides the gdbus-codegen-generated GDBusInterfaceSkeleton property retrieval and adds a custom hook that retrieves the filesystem size value when actually requested. One limitation of such approach is that the hook is called with the GDBusObjectManager lock held and thus it needs to be as minimal as possible and avoiding access to any GDBusObject. --- src/udiskslinuxfilesystem.c | 129 +++++++++++++++++++++++++++--------- 1 file changed, 97 insertions(+), 32 deletions(-) diff --git a/src/udiskslinuxfilesystem.c b/src/udiskslinuxfilesystem.c index a8390a044b..413a5a377d 100644 --- a/src/udiskslinuxfilesystem.c +++ b/src/udiskslinuxfilesystem.c @@ -56,6 +56,7 @@ #include "udiskssimplejob.h" #include "udiskslinuxdriveata.h" #include "udiskslinuxmountoptions.h" +#include "udisksata.h" /** * SECTION:udiskslinuxfilesystem @@ -78,6 +79,10 @@ struct _UDisksLinuxFilesystem { UDisksFilesystemSkeleton parent_instance; GMutex lock; + guint64 cached_fs_size; + gchar *cached_device_file; + gchar *cached_fs_type; + gboolean cached_drive_is_ata; }; struct _UDisksLinuxFilesystemClass @@ -85,7 +90,14 @@ struct _UDisksLinuxFilesystemClass UDisksFilesystemSkeletonClass parent_class; }; +enum +{ + PROP_0, + PROP_SIZE, +}; + static void filesystem_iface_init (UDisksFilesystemIface *iface); +static guint64 get_filesystem_size (UDisksLinuxFilesystem *filesystem); G_DEFINE_TYPE_WITH_CODE (UDisksLinuxFilesystem, udisks_linux_filesystem, UDISKS_TYPE_FILESYSTEM_SKELETON, G_IMPLEMENT_INTERFACE (UDISKS_TYPE_FILESYSTEM, filesystem_iface_init)); @@ -106,6 +118,8 @@ udisks_linux_filesystem_finalize (GObject *object) UDisksLinuxFilesystem *filesystem = UDISKS_LINUX_FILESYSTEM (object); g_mutex_clear (&(filesystem->lock)); + g_free (filesystem->cached_device_file); + g_free (filesystem->cached_fs_type); if (G_OBJECT_CLASS (udisks_linux_filesystem_parent_class)->finalize != NULL) G_OBJECT_CLASS (udisks_linux_filesystem_parent_class)->finalize (object); @@ -119,6 +133,44 @@ udisks_linux_filesystem_init (UDisksLinuxFilesystem *filesystem) G_DBUS_INTERFACE_SKELETON_FLAGS_HANDLE_METHOD_INVOCATIONS_IN_THREAD); } +static void +udisks_linux_filesystem_get_property (GObject *object, + guint prop_id, + GValue *value, + GParamSpec *pspec) +{ + UDisksLinuxFilesystem *filesystem = UDISKS_LINUX_FILESYSTEM (object); + + switch (prop_id) + { + case PROP_SIZE: + g_value_set_uint64 (value, get_filesystem_size (filesystem)); + break; + + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); + break; + } +} + +static void +udisks_linux_filesystem_set_property (GObject *object, + guint prop_id, + const GValue *value, + GParamSpec *pspec) +{ + switch (prop_id) + { + case PROP_SIZE: + g_warning ("udisks_linux_filesystem_set_property() should never be called, value = %lu", g_value_get_uint64 (value)); + break; + + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); + break; + } +} + static void udisks_linux_filesystem_class_init (UDisksLinuxFilesystemClass *klass) { @@ -126,6 +178,10 @@ udisks_linux_filesystem_class_init (UDisksLinuxFilesystemClass *klass) gobject_class = G_OBJECT_CLASS (klass); gobject_class->finalize = udisks_linux_filesystem_finalize; + gobject_class->get_property = udisks_linux_filesystem_get_property; + gobject_class->set_property = udisks_linux_filesystem_set_property; + + g_object_class_override_property (gobject_class, PROP_SIZE, "size"); } /** @@ -144,49 +200,58 @@ udisks_linux_filesystem_new (void) /* ---------------------------------------------------------------------------------------------------- */ +/* WARNING: called with GDBusObjectManager lock held, avoid any object lookup */ static guint64 -get_filesystem_size (UDisksLinuxBlockObject *object) +get_filesystem_size (UDisksLinuxFilesystem *filesystem) { guint64 size = 0; - UDisksLinuxDevice *device; - gchar *dev; - const gchar *type; GError *error = NULL; - device = udisks_linux_block_object_get_device (object); - dev = udisks_linux_block_object_get_device_file (object); - type = g_udev_device_get_property (device->udev_device, "ID_FS_TYPE"); + if (!filesystem->cached_device_file || !filesystem->cached_fs_type) + return 0; + + /* if the drive is ATA and is sleeping, skip filesystem size check to prevent + * drive waking up - nothing has changed anyway since it's been sleeping... + */ + if (filesystem->cached_drive_is_ata) + { + guchar pm_state = 0; + + if (udisks_ata_get_pm_state (filesystem->cached_device_file, NULL, &pm_state)) + if (!UDISKS_ATA_PM_STATE_AWAKE (pm_state) && filesystem->cached_fs_size > 0) + return filesystem->cached_fs_size; + } - if (g_strcmp0 (type, "ext2") == 0) + if (g_strcmp0 (filesystem->cached_fs_type, "ext2") == 0) { - BDFSExt2Info *info = bd_fs_ext2_get_info (dev, &error); + BDFSExt2Info *info = bd_fs_ext2_get_info (filesystem->cached_device_file, &error); if (info) { size = info->block_size * info->block_count; bd_fs_ext2_info_free (info); } } - else if (g_strcmp0 (type, "ext3") == 0) + else if (g_strcmp0 (filesystem->cached_fs_type, "ext3") == 0) { - BDFSExt3Info *info = bd_fs_ext3_get_info (dev, &error); + BDFSExt3Info *info = bd_fs_ext3_get_info (filesystem->cached_device_file, &error); if (info) { size = info->block_size * info->block_count; bd_fs_ext3_info_free (info); } } - else if (g_strcmp0 (type, "ext4") == 0) + else if (g_strcmp0 (filesystem->cached_fs_type, "ext4") == 0) { - BDFSExt4Info *info = bd_fs_ext4_get_info (dev, &error); + BDFSExt4Info *info = bd_fs_ext4_get_info (filesystem->cached_device_file, &error); if (info) { size = info->block_size * info->block_count; bd_fs_ext4_info_free (info); } } - else if (g_strcmp0 (type, "xfs") == 0) + else if (g_strcmp0 (filesystem->cached_fs_type, "xfs") == 0) { - BDFSXfsInfo *info = bd_fs_xfs_get_info (dev, &error); + BDFSXfsInfo *info = bd_fs_xfs_get_info (filesystem->cached_device_file, &error); if (info) { size = info->block_size * info->block_count; @@ -194,10 +259,9 @@ get_filesystem_size (UDisksLinuxBlockObject *object) } } - g_free (dev); - g_object_unref (device); g_clear_error (&error); + filesystem->cached_fs_size = size; return size; } @@ -234,14 +298,12 @@ void udisks_linux_filesystem_update (UDisksLinuxFilesystem *filesystem, UDisksLinuxBlockObject *object) { + UDisksDriveAta *ata = NULL; UDisksMountMonitor *mount_monitor; UDisksLinuxDevice *device; - UDisksDriveAta *ata = NULL; GPtrArray *p; GList *mounts; GList *l; - gboolean skip_fs_size = FALSE; - guchar pm_state; mount_monitor = udisks_daemon_get_mount_monitor (udisks_linux_block_object_get_daemon (object)); device = udisks_linux_block_object_get_device (object); @@ -263,20 +325,24 @@ udisks_linux_filesystem_update (UDisksLinuxFilesystem *filesystem, g_ptr_array_free (p, TRUE); g_list_free_full (mounts, g_object_unref); - /* if the drive is ATA and is sleeping, skip filesystem size check to prevent - * drive waking up - nothing has changed anyway since it's been sleeping... + /* cached device properties for on-demand filesystem size retrieval */ + g_free (filesystem->cached_device_file); + g_free (filesystem->cached_fs_type); + filesystem->cached_fs_type = g_strdup (g_udev_device_get_property (device->udev_device, "ID_FS_TYPE")); + if (g_strcmp0 (filesystem->cached_fs_type, "ext2") == 0 || + g_strcmp0 (filesystem->cached_fs_type, "ext3") == 0 || + g_strcmp0 (filesystem->cached_fs_type, "ext4") == 0 || + g_strcmp0 (filesystem->cached_fs_type, "xfs") == 0) + filesystem->cached_device_file = udisks_linux_block_object_get_device_file (object); + + /* TODO: this only looks for a drive object associated with the current + * block object. In case of a complex layered structure this needs to walk + * the tree and return a list of physical drives to check the powermanagement on. */ ata = get_drive_ata (object); - if (ata != NULL) - { - if (udisks_linux_drive_ata_get_pm_state (UDISKS_LINUX_DRIVE_ATA (ata), NULL, &pm_state)) - skip_fs_size = ! UDISKS_LINUX_DRIVE_ATA_IS_AWAKE (pm_state); - } + filesystem->cached_drive_is_ata = ata != NULL && udisks_drive_ata_get_pm_supported (ata); g_clear_object (&ata); - if (! skip_fs_size) - udisks_filesystem_set_size (UDISKS_FILESYSTEM (filesystem), get_filesystem_size (object)); - g_dbus_interface_skeleton_flush (G_DBUS_INTERFACE_SKELETON (filesystem)); g_object_unref (device); @@ -1872,10 +1938,9 @@ handle_resize (UDisksFilesystem *filesystem, /* At least resize2fs might need another uevent after it is done. */ + UDISKS_LINUX_FILESYSTEM (filesystem)->cached_fs_size = 0; udisks_linux_block_object_trigger_uevent_sync (UDISKS_LINUX_BLOCK_OBJECT (object), UDISKS_DEFAULT_WAIT_TIMEOUT); - - udisks_filesystem_set_size (filesystem, get_filesystem_size (UDISKS_LINUX_BLOCK_OBJECT (object))); g_dbus_interface_skeleton_flush (G_DBUS_INTERFACE_SKELETON (filesystem)); udisks_filesystem_complete_resize (filesystem, invocation); udisks_simple_job_complete (UDISKS_SIMPLE_JOB (job), TRUE, NULL);