Skip to content

Commit

Permalink
Merge pull request #2968 from cgwalters/drop-global-syncfs-by-default
Browse files Browse the repository at this point in the history
deploy: Remove global `sync` by default
  • Loading branch information
cgwalters committed Aug 30, 2023
2 parents d976ec5 + fa69eaa commit c0014e0
Showing 1 changed file with 1 addition and 84 deletions.
85 changes: 1 addition & 84 deletions src/libostree/ostree-sysroot-deploy.c
Original file line number Diff line number Diff line change
Expand Up @@ -1634,44 +1634,8 @@ typedef struct
{
guint64 root_syncfs_msec;
guint64 boot_syncfs_msec;
guint64 extra_syncfs_msec;
} SyncStats;

typedef struct
{
int ref_count; /* atomic */
bool success;
GMutex mutex;
GCond cond;
} SyncData;

static void
sync_data_unref (SyncData *data)
{
if (!g_atomic_int_dec_and_test (&data->ref_count))
return;
g_mutex_clear (&data->mutex);
g_cond_clear (&data->cond);
g_free (data);
}

// An asynchronous sync() call. See below.
static void *
sync_in_thread (void *ptr)
{
SyncData *syncdata = ptr;
ot_journal_print (LOG_INFO, "Starting global sync()");
sync ();
ot_journal_print (LOG_INFO, "Completed global sync()");
g_mutex_lock (&syncdata->mutex);
// Signal success
syncdata->success = true;
g_cond_broadcast (&syncdata->cond);
g_mutex_unlock (&syncdata->mutex);
sync_data_unref (syncdata);
return NULL;
}

/* First, sync the root directory as well as /var and /boot which may
* be separate mount points. Then *in addition*, do a global
* `sync()`.
Expand Down Expand Up @@ -1702,52 +1666,6 @@ full_system_sync (OstreeSysroot *self, SyncStats *out_stats, GCancellable *cance
end_msec = g_get_monotonic_time () / 1000;
out_stats->boot_syncfs_msec = (end_msec - start_msec);

/* And now out of an excess of conservativism, we still invoke
* sync(). The advantage of still using `syncfs()` above is that we
* actually get error codes out of that API, and we more clearly
* delineate what we actually want to sync in the future when this
* global sync call is removed.
*
* Due to https://bugzilla.redhat.com/show_bug.cgi?id=2003532 which is
* a bug in the interaction between Ceph and systemd, we have a capped
* timeout of 5s on the sync here. In general, we don't actually want
* to "own" flushing data on *all* mounted filesystems as part of
* our process. Again, we're only keeping the `sync` here out of
* conservatisim. We could likely just remove it and e.g. systemd
* would take care of sync as part of unmounting individual filesystems.
*/
start_msec = g_get_monotonic_time () / 1000;
if (!(self->opt_flags & OSTREE_SYSROOT_GLOBAL_OPT_SKIP_SYNC))
{
SyncData *syncdata = g_new (SyncData, 1);
syncdata->ref_count = 2; // One for us, one for thread
syncdata->success = FALSE;
g_mutex_init (&syncdata->mutex);
g_cond_init (&syncdata->cond);
g_mutex_lock (&syncdata->mutex);
GThread *worker = g_thread_new ("ostree sync", sync_in_thread, syncdata);
// Wait up to 5 seconds for sync() to finish
gint64 end_time = g_get_monotonic_time () + (5 * G_TIME_SPAN_SECOND);
while (!syncdata->success)
{
if (!g_cond_wait_until (&syncdata->cond, &syncdata->mutex, end_time))
{
ot_journal_print (LOG_INFO, "Timed out waiting for global sync()");
break;
}
}
g_mutex_unlock (&syncdata->mutex);
sync_data_unref (syncdata);
// We can't join the thread here, for two reasons. First, the whole
// point here is to avoid blocking on `sync`. The other reason is
// today there is no return value on success from `sync`, so there's
// no point to joining the thread even if we had a nonblocking mechanism
// to do so.
g_thread_unref (worker);
}
end_msec = g_get_monotonic_time () / 1000;
out_stats->extra_syncfs_msec = (end_msec - start_msec);

return TRUE;
}

Expand Down Expand Up @@ -3025,8 +2943,7 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, GPtrArray *n
bootloader_is_atomic ? "yes" : "no", "OSTREE_DID_BOOTSWAP=%s",
requires_new_bootversion ? "yes" : "no", "OSTREE_N_DEPLOYMENTS=%u", new_deployments->len,
"OSTREE_SYNCFS_ROOT_MSEC=%" G_GUINT64_FORMAT, syncstats.root_syncfs_msec,
"OSTREE_SYNCFS_BOOT_MSEC=%" G_GUINT64_FORMAT, syncstats.boot_syncfs_msec,
"OSTREE_SYNCFS_EXTRA_MSEC=%" G_GUINT64_FORMAT, syncstats.extra_syncfs_msec, NULL);
"OSTREE_SYNCFS_BOOT_MSEC=%" G_GUINT64_FORMAT, syncstats.boot_syncfs_msec, NULL);
_ostree_sysroot_emit_journal_msg (self, msg);
}

Expand Down

0 comments on commit c0014e0

Please sign in to comment.