* [PATCH] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
@ 2026-07-20 14:32 Nandakumar Raghavan
2026-08-15 5:29 ` Nandakumar Raghavan
0 siblings, 1 reply; 12+ messages in thread
From: Nandakumar Raghavan @ 2026-07-20 14:32 UTC (permalink / raw)
To: tytso; +Cc: linux-ext4, srivatsa, naraghavan
During journal replay, e2fsck writes the primary superblock back to disk
in multiple I/O operations. The payload lands before the checksum, leaving
a transient window where the on-disk superblock has a bad checksum.
If udevd processes a change uevent during this window, libblkid probes the
primary superblock, finds a checksum mismatch, and concludes the partition
has no recognisable filesystem. udev then fires a remove event, wiping all
symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
that depends on those symlinks will fail.
udevd already serialises its own partition probes against whole-disk device
access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
whole-disk device before opening the filesystem. This forces udevd to defer
all probes on that disk until e2fsck exits and the lock is released, by
which point the filesystem is fully consistent.
The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
so that the lock covers the same device node that udevd locks. If sysfs
resolution fails, a warning is emitted and the lock falls back to the
partition device itself.
Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
---
e2fsck/unix.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 105 insertions(+)
diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 335ca377..12a9f50a 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -36,6 +36,15 @@ extern int optind;
#ifdef HAVE_SYS_IOCTL_H
#include <sys/ioctl.h>
#endif
+#ifdef HAVE_SYS_FILE_H
+#include <sys/file.h>
+#endif
+#ifdef HAVE_SYS_STAT_H
+#include <sys/stat.h>
+#endif
+#ifdef HAVE_SYS_SYSMACROS_H
+#include <sys/sysmacros.h>
+#endif
#ifdef HAVE_MALLOC_H
#include <malloc.h>
#endif
@@ -1397,6 +1406,92 @@ err:
return retval;
}
+static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
+{
+ struct stat st;
+ char partition_attr[256]; /* sysfs 'partition' attribute path */
+ char parent_dev_path[256]; /* sysfs '../dev' of the parent disk */
+ char parent_devnum[32];
+ FILE *f;
+ unsigned int maj, min;
+ int parent_resolved = 0;
+ char lock_path[256];
+ const char *opened_path;
+ int fd;
+
+ if (stat(dev_name, &st) < 0)
+ return -1;
+
+ if (!S_ISBLK(st.st_mode))
+ return -1;
+
+ maj = major(st.st_rdev);
+ min = minor(st.st_rdev);
+
+ /*
+ * If dev_name is a partition (sysfs 'partition' attribute exists),
+ * resolve the parent whole-disk device so the flock covers the same
+ * device node that udevd locks before probing any partition on it.
+ */
+ snprintf(partition_attr, sizeof(partition_attr),
+ "/sys/dev/block/%u:%u/partition", maj, min);
+
+ if (access(partition_attr, F_OK) == 0) {
+ snprintf(parent_dev_path, sizeof(parent_dev_path),
+ "/sys/dev/block/%u:%u/../dev", maj, min);
+
+ f = fopen(parent_dev_path, "r");
+ if (f) {
+ if (fscanf(f, "%31s", parent_devnum) == 1) {
+ unsigned int pmaj, pmin;
+ if (sscanf(parent_devnum, "%u:%u",
+ &pmaj, &pmin) == 2) {
+ maj = pmaj;
+ min = pmin;
+ parent_resolved = 1;
+ }
+ }
+ fclose(f);
+ }
+ if (!parent_resolved)
+ log_err(ctx, _("Warning: could not resolve whole-disk "
+ "device for %s; lock may not prevent "
+ "udev races\n"), dev_name);
+ }
+
+ snprintf(lock_path, sizeof(lock_path),
+ "/dev/block/%u:%u", maj, min);
+
+ opened_path = lock_path;
+ fd = open(lock_path, O_RDONLY | O_CLOEXEC, 0);
+ if (fd < 0) {
+ fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
+ if (fd < 0)
+ return -1;
+ opened_path = dev_name;
+ if (parent_resolved)
+ log_err(ctx, _("Warning: %s not found; locking %s "
+ "instead, lock may not prevent "
+ "udev races\n"), lock_path, dev_name);
+ }
+
+ while (flock(fd, LOCK_EX) != 0) {
+ if (errno == EINTR) {
+ if (ctx->flags & E2F_FLAG_CANCEL) {
+ close(fd);
+ return -1;
+ }
+ continue;
+ }
+ com_err(ctx->program_name, errno,
+ _("while trying to lock %s"), opened_path);
+ close(fd);
+ return -1;
+ }
+
+ return fd;
+}
+
int main (int argc, char *argv[])
{
errcode_t retval = 0, retval2 = 0, orig_retval = 0;
@@ -1413,6 +1508,7 @@ int main (int argc, char *argv[])
int journal_size;
int sysval, sys_page_size = 4096;
int old_bitmaps;
+ int lock_fd = -1;
__u32 features[3];
char *cp;
enum quota_type qtype;
@@ -1488,6 +1584,12 @@ int main (int argc, char *argv[])
check_mount(ctx);
+ lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
+ if (lock_fd < 0)
+ log_err(ctx, _("Warning: could not lock %s; "
+ "proceeding without block device lock\n"),
+ ctx->filesystem_name);
+
if (!(ctx->options & E2F_OPT_PREEN) &&
!(ctx->options & E2F_OPT_NO) &&
!(ctx->options & E2F_OPT_YES)) {
@@ -2169,6 +2271,9 @@ skip_write:
ext2fs_close_free(&ctx->fs);
free(ctx->journal_name);
+ if (lock_fd >= 0)
+ close(lock_fd);
+
if (ctx->logf)
fprintf(ctx->logf, "Exit status: %d\n", exit_value);
e2fsck_free_context(ctx);
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
2026-07-20 14:32 [PATCH] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check Nandakumar Raghavan
@ 2026-08-15 5:29 ` Nandakumar Raghavan
2026-08-18 22:44 ` Andreas Dilger
0 siblings, 1 reply; 12+ messages in thread
From: Nandakumar Raghavan @ 2026-08-15 5:29 UTC (permalink / raw)
To: tytso; +Cc: linux-ext4, srivatsa
Hi,
Gentle ping on this patch.
I would appreciate any feedback when time permits.
On Mon, Jul 20, 2026 at 02:32:46PM +0000, Nandakumar Raghavan wrote:
> During journal replay, e2fsck writes the primary superblock back to disk
> in multiple I/O operations. The payload lands before the checksum, leaving
> a transient window where the on-disk superblock has a bad checksum.
>
> If udevd processes a change uevent during this window, libblkid probes the
> primary superblock, finds a checksum mismatch, and concludes the partition
> has no recognisable filesystem. udev then fires a remove event, wiping all
> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> that depends on those symlinks will fail.
>
> udevd already serialises its own partition probes against whole-disk device
> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> whole-disk device before opening the filesystem. This forces udevd to defer
> all probes on that disk until e2fsck exits and the lock is released, by
> which point the filesystem is fully consistent.
>
> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> so that the lock covers the same device node that udevd locks. If sysfs
> resolution fails, a warning is emitted and the lock falls back to the
> partition device itself.
>
> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> ---
> e2fsck/unix.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 105 insertions(+)
>
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index 335ca377..12a9f50a 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -36,6 +36,15 @@ extern int optind;
> #ifdef HAVE_SYS_IOCTL_H
> #include <sys/ioctl.h>
> #endif
> +#ifdef HAVE_SYS_FILE_H
> +#include <sys/file.h>
> +#endif
> +#ifdef HAVE_SYS_STAT_H
> +#include <sys/stat.h>
> +#endif
> +#ifdef HAVE_SYS_SYSMACROS_H
> +#include <sys/sysmacros.h>
> +#endif
> #ifdef HAVE_MALLOC_H
> #include <malloc.h>
> #endif
> @@ -1397,6 +1406,92 @@ err:
> return retval;
> }
>
> +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> +{
> + struct stat st;
> + char partition_attr[256]; /* sysfs 'partition' attribute path */
> + char parent_dev_path[256]; /* sysfs '../dev' of the parent disk */
> + char parent_devnum[32];
> + FILE *f;
> + unsigned int maj, min;
> + int parent_resolved = 0;
> + char lock_path[256];
> + const char *opened_path;
> + int fd;
> +
> + if (stat(dev_name, &st) < 0)
> + return -1;
> +
> + if (!S_ISBLK(st.st_mode))
> + return -1;
> +
> + maj = major(st.st_rdev);
> + min = minor(st.st_rdev);
> +
> + /*
> + * If dev_name is a partition (sysfs 'partition' attribute exists),
> + * resolve the parent whole-disk device so the flock covers the same
> + * device node that udevd locks before probing any partition on it.
> + */
> + snprintf(partition_attr, sizeof(partition_attr),
> + "/sys/dev/block/%u:%u/partition", maj, min);
> +
> + if (access(partition_attr, F_OK) == 0) {
> + snprintf(parent_dev_path, sizeof(parent_dev_path),
> + "/sys/dev/block/%u:%u/../dev", maj, min);
> +
> + f = fopen(parent_dev_path, "r");
> + if (f) {
> + if (fscanf(f, "%31s", parent_devnum) == 1) {
> + unsigned int pmaj, pmin;
> + if (sscanf(parent_devnum, "%u:%u",
> + &pmaj, &pmin) == 2) {
> + maj = pmaj;
> + min = pmin;
> + parent_resolved = 1;
> + }
> + }
> + fclose(f);
> + }
> + if (!parent_resolved)
> + log_err(ctx, _("Warning: could not resolve whole-disk "
> + "device for %s; lock may not prevent "
> + "udev races\n"), dev_name);
> + }
> +
> + snprintf(lock_path, sizeof(lock_path),
> + "/dev/block/%u:%u", maj, min);
> +
> + opened_path = lock_path;
> + fd = open(lock_path, O_RDONLY | O_CLOEXEC, 0);
> + if (fd < 0) {
> + fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
> + if (fd < 0)
> + return -1;
> + opened_path = dev_name;
> + if (parent_resolved)
> + log_err(ctx, _("Warning: %s not found; locking %s "
> + "instead, lock may not prevent "
> + "udev races\n"), lock_path, dev_name);
> + }
> +
> + while (flock(fd, LOCK_EX) != 0) {
> + if (errno == EINTR) {
> + if (ctx->flags & E2F_FLAG_CANCEL) {
> + close(fd);
> + return -1;
> + }
> + continue;
> + }
> + com_err(ctx->program_name, errno,
> + _("while trying to lock %s"), opened_path);
> + close(fd);
> + return -1;
> + }
> +
> + return fd;
> +}
> +
> int main (int argc, char *argv[])
> {
> errcode_t retval = 0, retval2 = 0, orig_retval = 0;
> @@ -1413,6 +1508,7 @@ int main (int argc, char *argv[])
> int journal_size;
> int sysval, sys_page_size = 4096;
> int old_bitmaps;
> + int lock_fd = -1;
> __u32 features[3];
> char *cp;
> enum quota_type qtype;
> @@ -1488,6 +1584,12 @@ int main (int argc, char *argv[])
>
> check_mount(ctx);
>
> + lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> + if (lock_fd < 0)
> + log_err(ctx, _("Warning: could not lock %s; "
> + "proceeding without block device lock\n"),
> + ctx->filesystem_name);
> +
> if (!(ctx->options & E2F_OPT_PREEN) &&
> !(ctx->options & E2F_OPT_NO) &&
> !(ctx->options & E2F_OPT_YES)) {
> @@ -2169,6 +2271,9 @@ skip_write:
> ext2fs_close_free(&ctx->fs);
> free(ctx->journal_name);
>
> + if (lock_fd >= 0)
> + close(lock_fd);
> +
> if (ctx->logf)
> fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> e2fsck_free_context(ctx);
> --
> 2.54.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
2026-08-15 5:29 ` Nandakumar Raghavan
@ 2026-08-18 22:44 ` Andreas Dilger
2026-08-24 13:24 ` Nandakumar Raghavan
0 siblings, 1 reply; 12+ messages in thread
From: Andreas Dilger @ 2026-08-18 22:44 UTC (permalink / raw)
To: Nandakumar Raghavan; +Cc: tytso, linux-ext4, srivatsa
On Aug 14, 2026, at 23:29, Nandakumar Raghavan <naraghavan@linux.microsoft.com> wrote:
>
> Hi,
>
> Gentle ping on this patch.
>
> I would appreciate any feedback when time permits.
>
> On Mon, Jul 20, 2026 at 02:32:46PM +0000, Nandakumar Raghavan wrote:
>> During journal replay, e2fsck writes the primary superblock back to disk
>> in multiple I/O operations. The payload lands before the checksum, leaving
>> a transient window where the on-disk superblock has a bad checksum.
>>
>> If udevd processes a change uevent during this window, libblkid probes the
>> primary superblock, finds a checksum mismatch, and concludes the partition
>> has no recognisable filesystem. udev then fires a remove event, wiping all
>> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
>> that depends on those symlinks will fail.
>>
>> udevd already serialises its own partition probes against whole-disk device
>> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
>> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
>> whole-disk device before opening the filesystem. This forces udevd to defer
>> all probes on that disk until e2fsck exits and the lock is released, by
>> which point the filesystem is fully consistent.
>>
>> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
>> so that the lock covers the same device node that udevd locks. If sysfs
>> resolution fails, a warning is emitted and the lock falls back to the
>> partition device itself.
>>
>> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
This looks like it would unnecessarily emit an error message if run on any
non-Linux platform (e.g. MacOS, BSD), so it needs to be conditionally built.
>> ---
>> e2fsck/unix.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 105 insertions(+)
>>
>> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
>> index 335ca377..12a9f50a 100644
>> --- a/e2fsck/unix.c
>> +++ b/e2fsck/unix.c
>> @@ -36,6 +36,15 @@ extern int optind;
Presumably this code is only useful on Linux, so the conditional headers are not needed,
only a single `#ifdef __linux__`? Not really critical either way.
>> #ifdef HAVE_SYS_IOCTL_H
>> #include <sys/ioctl.h>
>> #endif
>> +#ifdef HAVE_SYS_FILE_H
>> +#include <sys/file.h>
>> +#endif
>> +#ifdef HAVE_SYS_STAT_H
>> +#include <sys/stat.h>
>> +#endif
>> +#ifdef HAVE_SYS_SYSMACROS_H
>> +#include <sys/sysmacros.h>
>> +#endif
>> #ifdef HAVE_MALLOC_H
>> #include <malloc.h>
>> #endif
>> @@ -1397,6 +1406,92 @@ err:
>> +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
This function should similarly be #ifdef'd out if not building on Linux.
>> +{
>> + struct stat st;
>> + char partition_attr[256]; /* sysfs 'partition' attribute path */
>> + char parent_dev_path[256]; /* sysfs '../dev' of the parent disk */
>> + char parent_devnum[32];
>> + FILE *f;
>> + unsigned int maj, min;
>> + int parent_resolved = 0;
>> + char lock_path[256];
>> + const char *opened_path;
>> + int fd;
>> +
>> + if (stat(dev_name, &st) < 0)
>> + return -1;
>> +
>> + if (!S_ISBLK(st.st_mode))
>> + return -1;
>> +
>> + maj = major(st.st_rdev);
>> + min = minor(st.st_rdev);
>> +
>> + /*
>> + * If dev_name is a partition (sysfs 'partition' attribute exists),
>> + * resolve the parent whole-disk device so the flock covers the same
>> + * device node that udevd locks before probing any partition on it.
>> + */
>> + snprintf(partition_attr, sizeof(partition_attr),
>> + "/sys/dev/block/%u:%u/partition", maj, min);
>> +
>> + if (access(partition_attr, F_OK) == 0) {
>> + snprintf(parent_dev_path, sizeof(parent_dev_path),
>> + "/sys/dev/block/%u:%u/../dev", maj, min);
(minor) this could reuse `partition_attr` here?
>> + f = fopen(parent_dev_path, "r");
>> + if (f) {
>> + if (fscanf(f, "%31s", parent_devnum) == 1) {
>> + unsigned int pmaj, pmin;
>> + if (sscanf(parent_devnum, "%u:%u",
>> + &pmaj, &pmin) == 2) {
>> + maj = pmaj;
>> + min = pmin;
>> + parent_resolved = 1;
>> + }
>> + }
>> + fclose(f);
>> + }
>> + if (!parent_resolved)
>> + log_err(ctx, _("Warning: could not resolve whole-disk "
>> + "device for %s; lock may not prevent "
>> + "udev races\n"), dev_name);
>> + }
>> +
>> + snprintf(lock_path, sizeof(lock_path),
>> + "/dev/block/%u:%u", maj, min);
(style) could fit on a single line?
(style) could re-use `partition_attr` here (maybe renamed to `dev_path` or similar)
to avoid having 3 single-use pathnames on the stack.
>> +
>> + opened_path = lock_path;
>> + fd = open(lock_path, O_RDONLY | O_CLOEXEC, 0);
>> + if (fd < 0) {
>> + fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
>> + if (fd < 0)
>> + return -1;
>> + opened_path = dev_name;
>> + if (parent_resolved)
>> + log_err(ctx, _("Warning: %s not found; locking %s "
>> + "instead, lock may not prevent "
>> + "udev races\n"), lock_path, dev_name);
(style) shouldn't split error messages across lines, even if > 80 columns
>> + }
>> +
>> + while (flock(fd, LOCK_EX) != 0) {
>> + if (errno == EINTR) {
>> + if (ctx->flags & E2F_FLAG_CANCEL) {
>> + close(fd);
>> + return -1;
>> + }
>> + continue;
Does this loop/hang forever if you try to kill it with CTRL-C?
>> + }
>> + com_err(ctx->program_name, errno,
>> + _("while trying to lock %s"), opened_path);
>> + close(fd);
>> + return -1;
>> + }
>> +
>> + return fd;
>> +}
>> +
>> int main (int argc, char *argv[])
>> {
>> errcode_t retval = 0, retval2 = 0, orig_retval = 0;
>> @@ -1413,6 +1508,7 @@ int main (int argc, char *argv[])
>> int journal_size;
>> int sysval, sys_page_size = 4096;
>> int old_bitmaps;
>> + int lock_fd = -1;
>> __u32 features[3];
>> char *cp;
>> enum quota_type qtype;
>> @@ -1488,6 +1584,12 @@ int main (int argc, char *argv[])
>>
>> check_mount(ctx);
>>
>> + lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
>> + if (lock_fd < 0)
>> + log_err(ctx, _("Warning: could not lock %s; "
>> + "proceeding without block device lock\n"),
>> + ctx->filesystem_name);
>> +
>> if (!(ctx->options & E2F_OPT_PREEN) &&
>> !(ctx->options & E2F_OPT_NO) &&
>> !(ctx->options & E2F_OPT_YES)) {
>> @@ -2169,6 +2271,9 @@ skip_write:
>> ext2fs_close_free(&ctx->fs);
>> free(ctx->journal_name);
>>
>> + if (lock_fd >= 0)
>> + close(lock_fd);
>> +
>> if (ctx->logf)
>> fprintf(ctx->logf, "Exit status: %d\n", exit_value);
>> e2fsck_free_context(ctx);
>> --
>> 2.54.0
>
Cheers, Andreas
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
2026-08-18 22:44 ` Andreas Dilger
@ 2026-08-24 13:24 ` Nandakumar Raghavan
2026-08-24 16:15 ` [PATCH v2] " Nandakumar Raghavan
0 siblings, 1 reply; 12+ messages in thread
From: Nandakumar Raghavan @ 2026-08-24 13:24 UTC (permalink / raw)
To: Andreas Dilger; +Cc: tytso, linux-ext4, srivatsa
On Tue, Aug 18, 2026 at 04:44:26PM -0600, Andreas Dilger wrote:
> On Aug 14, 2026, at 23:29, Nandakumar Raghavan <naraghavan@linux.microsoft.com> wrote:
> >
> > Hi,
> >
> > Gentle ping on this patch.
> >
> > I would appreciate any feedback when time permits.
> >
> > On Mon, Jul 20, 2026 at 02:32:46PM +0000, Nandakumar Raghavan wrote:
> >> During journal replay, e2fsck writes the primary superblock back to disk
> >> in multiple I/O operations. The payload lands before the checksum, leaving
> >> a transient window where the on-disk superblock has a bad checksum.
> >>
> >> If udevd processes a change uevent during this window, libblkid probes the
> >> primary superblock, finds a checksum mismatch, and concludes the partition
> >> has no recognisable filesystem. udev then fires a remove event, wiping all
> >> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> >> that depends on those symlinks will fail.
> >>
> >> udevd already serialises its own partition probes against whole-disk device
> >> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> >> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> >> whole-disk device before opening the filesystem. This forces udevd to defer
> >> all probes on that disk until e2fsck exits and the lock is released, by
> >> which point the filesystem is fully consistent.
> >>
> >> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> >> so that the lock covers the same device node that udevd locks. If sysfs
> >> resolution fails, a warning is emitted and the lock falls back to the
> >> partition device itself.
> >>
> >> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
>
Thanks for the review, Andreas. Replies inline, v2 coming with all of these
addressed.
> This looks like it would unnecessarily emit an error message if run on any
> non-Linux platform (e.g. MacOS, BSD), so it needs to be conditionally built.
Agreed, fixed in v2 — lock_whole_disk(), its call site, the lock_fd
variable, and the close() cleanup are all now wrapped in #ifdef __linux__.
Nothing from this feature is compiled at all on non-Linux.
>
> >> ---
> >> e2fsck/unix.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 105 insertions(+)
> >>
> >> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> >> index 335ca377..12a9f50a 100644
> >> --- a/e2fsck/unix.c
> >> +++ b/e2fsck/unix.c
> >> @@ -36,6 +36,15 @@ extern int optind;
>
> Presumably this code is only useful on Linux, so the conditional headers are not needed,
> only a single `#ifdef __linux__`? Not really critical either way.
Fixed. collapsed the three HAVE_SYS_*_H guards into one #ifdef __linux__
block around sys/file.h, sys/stat.h, sys/sysmacros.h.
>
> >> #ifdef HAVE_SYS_IOCTL_H
> >> #include <sys/ioctl.h>
> >> #endif
> >> +#ifdef HAVE_SYS_FILE_H
> >> +#include <sys/file.h>
> >> +#endif
> >> +#ifdef HAVE_SYS_STAT_H
> >> +#include <sys/stat.h>
> >> +#endif
> >> +#ifdef HAVE_SYS_SYSMACROS_H
> >> +#include <sys/sysmacros.h>
> >> +#endif
> >> #ifdef HAVE_MALLOC_H
> >> #include <malloc.h>
> >> #endif
> >> @@ -1397,6 +1406,92 @@ err:
> >> +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
>
> This function should similarly be #ifdef'd out if not building on Linux.
Fixed. I have went with wrapping the whole function rather than guarding
just the body, so it's fully absent from non-Linux builds.
>
> >> +{
> >> + struct stat st;
> >> + char partition_attr[256]; /* sysfs 'partition' attribute path */
> >> + char parent_dev_path[256]; /* sysfs '../dev' of the parent disk */
> >> + char parent_devnum[32];
> >> + FILE *f;
> >> + unsigned int maj, min;
> >> + int parent_resolved = 0;
> >> + char lock_path[256];
> >> + const char *opened_path;
> >> + int fd;
> >> +
> >> + if (stat(dev_name, &st) < 0)
> >> + return -1;
> >> +
> >> + if (!S_ISBLK(st.st_mode))
> >> + return -1;
> >> +
> >> + maj = major(st.st_rdev);
> >> + min = minor(st.st_rdev);
> >> +
> >> + /*
> >> + * If dev_name is a partition (sysfs 'partition' attribute exists),
> >> + * resolve the parent whole-disk device so the flock covers the same
> >> + * device node that udevd locks before probing any partition on it.
> >> + */
> >> + snprintf(partition_attr, sizeof(partition_attr),
> >> + "/sys/dev/block/%u:%u/partition", maj, min);
> >> +
> >> + if (access(partition_attr, F_OK) == 0) {
> >> + snprintf(parent_dev_path, sizeof(parent_dev_path),
> >> + "/sys/dev/block/%u:%u/../dev", maj, min);
>
> (minor) this could reuse `partition_attr` here?
>
> >> + f = fopen(parent_dev_path, "r");
> >> + if (f) {
> >> + if (fscanf(f, "%31s", parent_devnum) == 1) {
> >> + unsigned int pmaj, pmin;
> >> + if (sscanf(parent_devnum, "%u:%u",
> >> + &pmaj, &pmin) == 2) {
> >> + maj = pmaj;
> >> + min = pmin;
> >> + parent_resolved = 1;
> >> + }
> >> + }
> >> + fclose(f);
> >> + }
> >> + if (!parent_resolved)
> >> + log_err(ctx, _("Warning: could not resolve whole-disk "
> >> + "device for %s; lock may not prevent "
> >> + "udev races\n"), dev_name);
> >> + }
> >> +
> >> + snprintf(lock_path, sizeof(lock_path),
> >> + "/dev/block/%u:%u", maj, min);
>
> (style) could fit on a single line?
>
> (style) could re-use `partition_attr` here (maybe renamed to `dev_path` or similar)
> to avoid having 3 single-use pathnames on the stack.
Fixed — same dev_path buffer reused for all three paths, and the /dev/block/%u:%u
snprintf is now a single line
>
> >> +
> >> + opened_path = lock_path;
> >> + fd = open(lock_path, O_RDONLY | O_CLOEXEC, 0);
> >> + if (fd < 0) {
> >> + fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
> >> + if (fd < 0)
> >> + return -1;
> >> + opened_path = dev_name;
> >> + if (parent_resolved)
> >> + log_err(ctx, _("Warning: %s not found; locking %s "
> >> + "instead, lock may not prevent "
> >> + "udev races\n"), lock_path, dev_name);
>
> (style) shouldn't split error messages across lines, even if > 80 columns
Done — both warning strings are now single-line literals.
>
> >> + }
> >> +
> >> + while (flock(fd, LOCK_EX) != 0) {
> >> + if (errno == EINTR) {
> >> + if (ctx->flags & E2F_FLAG_CANCEL) {
> >> + close(fd);
> >> + return -1;
> >> + }
> >> + continue;
>
> Does this loop/hang forever if you try to kill it with CTRL-C?
No. I traced it. The SIGINT/SIGTERM handler is installed earlier in main(),
before lock_whole_disk() runs, and isn't SA_RESTART (that's only applied
to SIGUSR1/SIGUSR2 afterward). So a blocked flock() gets EINTR on Ctrl-C,
E2F_FLAG_CANCEL is already set by the handler, and the existing EINTR
branch in the retry loop breaks out and returns -1 cleanly. So there is
no hang.
One more thing found while testing v2: the original patch printed "Warning:
could not lock %s; proceeding without block device lock" unconditionally
whenever lock_whole_disk() returned -1 — including the very common case of
running e2fsck against a regular file rather than a block device (which is
how virtually every e2fsprogs test image is checked). This caused issues
with test since the warning message went into stderr. v2 drops that blanket message
entirely; lock_whole_disk() now only warns/errors at the point of a genuine
failure
>
> >> + }
> >> + com_err(ctx->program_name, errno,
> >> + _("while trying to lock %s"), opened_path);
> >> + close(fd);
> >> + return -1;
> >> + }
> >> +
> >> + return fd;
> >> +}
> >> +
> >> int main (int argc, char *argv[])
> >> {
> >> errcode_t retval = 0, retval2 = 0, orig_retval = 0;
> >> @@ -1413,6 +1508,7 @@ int main (int argc, char *argv[])
> >> int journal_size;
> >> int sysval, sys_page_size = 4096;
> >> int old_bitmaps;
> >> + int lock_fd = -1;
> >> __u32 features[3];
> >> char *cp;
> >> enum quota_type qtype;
> >> @@ -1488,6 +1584,12 @@ int main (int argc, char *argv[])
> >>
> >> check_mount(ctx);
> >>
> >> + lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> >> + if (lock_fd < 0)
> >> + log_err(ctx, _("Warning: could not lock %s; "
> >> + "proceeding without block device lock\n"),
> >> + ctx->filesystem_name);
> >> +
> >> if (!(ctx->options & E2F_OPT_PREEN) &&
> >> !(ctx->options & E2F_OPT_NO) &&
> >> !(ctx->options & E2F_OPT_YES)) {
> >> @@ -2169,6 +2271,9 @@ skip_write:
> >> ext2fs_close_free(&ctx->fs);
> >> free(ctx->journal_name);
> >>
> >> + if (lock_fd >= 0)
> >> + close(lock_fd);
> >> +
> >> if (ctx->logf)
> >> fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> >> e2fsck_free_context(ctx);
> >> --
> >> 2.54.0
> >
>
>
> Cheers, Andreas
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
2026-08-24 13:24 ` Nandakumar Raghavan
@ 2026-08-24 16:15 ` Nandakumar Raghavan
2026-08-24 22:20 ` Andreas Dilger
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Nandakumar Raghavan @ 2026-08-24 16:15 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, adilger, srivatsa, Nandakumar Raghavan
During journal replay, e2fsck writes the primary superblock back to disk
in multiple I/O operations. The payload lands before the checksum, leaving
a transient window where the on-disk superblock has a bad checksum.
If udevd processes a change uevent during this window, libblkid probes the
primary superblock, finds a checksum mismatch, and concludes the partition
has no recognisable filesystem. udev then fires a remove event, wiping all
symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
that depends on those symlinks will fail.
udevd already serialises its own partition probes against whole-disk device
access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
whole-disk device before opening the filesystem. This forces udevd to defer
all probes on that disk until e2fsck exits and the lock is released, by
which point the filesystem is fully consistent.
The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
so that the lock covers the same device node that udevd locks. If sysfs
resolution fails, a warning is emitted and the lock falls back to the
partition device itself.
This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
on block devices) and is compiled out entirely on non-Linux platforms.
Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
---
Changes in v2:
- Guard the whole feature (function, call site, lock_fd, cleanup) with
#ifdef __linux__ instead of just the function body, so it's fully
absent from non-Linux builds
- Collapse HAVE_SYS_*_H header guards into a single #ifdef __linux__
- Reuse a single dev_path[] buffer instead of three separate stack
buffers
- Keep translatable warning strings on one line each, even past 80
columns
- Drop the blanket "could not lock" warning that fired even when the
target wasn't a block device at all (e.g. a regular file), which was
causing issues with tests; lock_whole_disk() now only warns/errors
at the point of a genuine failure
- No change needed for the Ctrl-C/EINTR concern — traced and confirmed
the existing signal handling already unwinds cleanly
e2fsck/unix.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)
diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 335ca377..cf21752a 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -36,6 +36,11 @@ extern int optind;
#ifdef HAVE_SYS_IOCTL_H
#include <sys/ioctl.h>
#endif
+#ifdef __linux__
+#include <sys/file.h>
+#include <sys/stat.h>
+#include <sys/sysmacros.h>
+#endif
#ifdef HAVE_MALLOC_H
#include <malloc.h>
#endif
@@ -1397,6 +1402,90 @@ err:
return retval;
}
+#ifdef __linux__
+static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
+{
+ struct stat st;
+ char dev_path[256]; /* reused for each sysfs/device path we build */
+ char parent_devnum[32];
+ FILE *f;
+ unsigned int maj, min;
+ int parent_resolved = 0;
+ const char *opened_path;
+ int fd;
+
+ if (stat(dev_name, &st) < 0)
+ return -1;
+
+ if (!S_ISBLK(st.st_mode))
+ return -1;
+
+ maj = major(st.st_rdev);
+ min = minor(st.st_rdev);
+
+ /*
+ * If dev_name is a partition (sysfs 'partition' attribute exists),
+ * resolve the parent whole-disk device so the flock covers the same
+ * device node that udevd locks before probing any partition on it.
+ */
+ snprintf(dev_path, sizeof(dev_path),
+ "/sys/dev/block/%u:%u/partition", maj, min);
+
+ if (access(dev_path, F_OK) == 0) {
+ snprintf(dev_path, sizeof(dev_path),
+ "/sys/dev/block/%u:%u/../dev", maj, min);
+
+ f = fopen(dev_path, "r");
+ if (f) {
+ if (fscanf(f, "%31s", parent_devnum) == 1) {
+ unsigned int pmaj, pmin;
+ if (sscanf(parent_devnum, "%u:%u",
+ &pmaj, &pmin) == 2) {
+ maj = pmaj;
+ min = pmin;
+ parent_resolved = 1;
+ }
+ }
+ fclose(f);
+ }
+ if (!parent_resolved)
+ log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
+ }
+
+ snprintf(dev_path, sizeof(dev_path), "/dev/block/%u:%u", maj, min);
+
+ opened_path = dev_path;
+ fd = open(dev_path, O_RDONLY | O_CLOEXEC, 0);
+ if (fd < 0) {
+ fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
+ if (fd < 0) {
+ com_err(ctx->program_name, errno,
+ _("while trying to open %s for locking"), dev_name);
+ return -1;
+ }
+ opened_path = dev_name;
+ if (parent_resolved)
+ log_err(ctx, _("Warning: %s not found; locking %s instead, lock may not prevent udev races\n"), dev_path, dev_name);
+ }
+
+ while (flock(fd, LOCK_EX) != 0) {
+ if (errno == EINTR) {
+ if (ctx->flags & E2F_FLAG_CANCEL) {
+ close(fd);
+ return -1;
+ }
+ continue;
+ }
+ com_err(ctx->program_name, errno,
+ _("while trying to lock %s"), opened_path);
+ close(fd);
+ return -1;
+ }
+
+ return fd;
+}
+#endif /* __linux__ */
+
int main (int argc, char *argv[])
{
errcode_t retval = 0, retval2 = 0, orig_retval = 0;
@@ -1413,6 +1502,9 @@ int main (int argc, char *argv[])
int journal_size;
int sysval, sys_page_size = 4096;
int old_bitmaps;
+#ifdef __linux__
+ int lock_fd = -1;
+#endif
__u32 features[3];
char *cp;
enum quota_type qtype;
@@ -1488,6 +1580,10 @@ int main (int argc, char *argv[])
check_mount(ctx);
+#ifdef __linux__
+ lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
+#endif
+
if (!(ctx->options & E2F_OPT_PREEN) &&
!(ctx->options & E2F_OPT_NO) &&
!(ctx->options & E2F_OPT_YES)) {
@@ -2169,6 +2265,11 @@ skip_write:
ext2fs_close_free(&ctx->fs);
free(ctx->journal_name);
+#ifdef __linux__
+ if (lock_fd >= 0)
+ close(lock_fd);
+#endif
+
if (ctx->logf)
fprintf(ctx->logf, "Exit status: %d\n", exit_value);
e2fsck_free_context(ctx);
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
2026-08-24 16:15 ` [PATCH v2] " Nandakumar Raghavan
@ 2026-08-24 22:20 ` Andreas Dilger
2026-08-24 22:40 ` Darrick J. Wong
2026-08-28 21:30 ` ddstreet
2 siblings, 0 replies; 12+ messages in thread
From: Andreas Dilger @ 2026-08-24 22:20 UTC (permalink / raw)
To: Nandakumar Raghavan; +Cc: linux-ext4, tytso, srivatsa
On Aug 24, 2026, at 10:15, Nandakumar Raghavan <naraghavan@linux.microsoft.com> wrote:
>
> During journal replay, e2fsck writes the primary superblock back to disk
> in multiple I/O operations. The payload lands before the checksum, leaving
> a transient window where the on-disk superblock has a bad checksum.
>
> If udevd processes a change uevent during this window, libblkid probes the
> primary superblock, finds a checksum mismatch, and concludes the partition
> has no recognisable filesystem. udev then fires a remove event, wiping all
> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> that depends on those symlinks will fail.
>
> udevd already serialises its own partition probes against whole-disk device
> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> whole-disk device before opening the filesystem. This forces udevd to defer
> all probes on that disk until e2fsck exits and the lock is released, by
> which point the filesystem is fully consistent.
>
> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> so that the lock covers the same device node that udevd locks. If sysfs
> resolution fails, a warning is emitted and the lock falls back to the
> partition device itself.
>
> This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
> on block devices) and is compiled out entirely on non-Linux platforms.
>
> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
Looks much better, thanks.
Reviewed-by: Andreas Dilger <adilger@dilger.ca <mailto:adilger@dilger.ca>>
Cheers, Andreas
> ---
>
> Changes in v2:
> - Guard the whole feature (function, call site, lock_fd, cleanup) with
> #ifdef __linux__ instead of just the function body, so it's fully
> absent from non-Linux builds
> - Collapse HAVE_SYS_*_H header guards into a single #ifdef __linux__
> - Reuse a single dev_path[] buffer instead of three separate stack
> buffers
> - Keep translatable warning strings on one line each, even past 80
> columns
> - Drop the blanket "could not lock" warning that fired even when the
> target wasn't a block device at all (e.g. a regular file), which was
> causing issues with tests; lock_whole_disk() now only warns/errors
> at the point of a genuine failure
> - No change needed for the Ctrl-C/EINTR concern — traced and confirmed
> the existing signal handling already unwinds cleanly
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
2026-08-24 16:15 ` [PATCH v2] " Nandakumar Raghavan
2026-08-24 22:20 ` Andreas Dilger
@ 2026-08-24 22:40 ` Darrick J. Wong
2026-08-26 14:04 ` Nandakumar Raghavan
2026-08-28 21:30 ` ddstreet
2 siblings, 1 reply; 12+ messages in thread
From: Darrick J. Wong @ 2026-08-24 22:40 UTC (permalink / raw)
To: Nandakumar Raghavan; +Cc: linux-ext4, tytso, adilger, srivatsa
On Mon, Aug 24, 2026 at 09:15:11AM -0700, Nandakumar Raghavan wrote:
> During journal replay, e2fsck writes the primary superblock back to disk
> in multiple I/O operations. The payload lands before the checksum, leaving
> a transient window where the on-disk superblock has a bad checksum.
>
> If udevd processes a change uevent during this window, libblkid probes the
> primary superblock, finds a checksum mismatch, and concludes the partition
> has no recognisable filesystem. udev then fires a remove event, wiping all
> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> that depends on those symlinks will fail.
>
> udevd already serialises its own partition probes against whole-disk device
> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> whole-disk device before opening the filesystem. This forces udevd to defer
> all probes on that disk until e2fsck exits and the lock is released, by
> which point the filesystem is fully consistent.
>
> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> so that the lock covers the same device node that udevd locks. If sysfs
> resolution fails, a warning is emitted and the lock falls back to the
> partition device itself.
>
> This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
> on block devices) and is compiled out entirely on non-Linux platforms.
You're clearly implementing the "Locking Block Device Access" protocol
as documented by systemd:
https://systemd.io/BLOCK_DEVICE_LOCKING/
So why not use libsystemd like the example in that posting provides?
sd_device_get_parent_with_subsystem_devtype is much less gross than
open-coding the same logic here.
> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> ---
>
> Changes in v2:
> - Guard the whole feature (function, call site, lock_fd, cleanup) with
> #ifdef __linux__ instead of just the function body, so it's fully
> absent from non-Linux builds
> - Collapse HAVE_SYS_*_H header guards into a single #ifdef __linux__
> - Reuse a single dev_path[] buffer instead of three separate stack
> buffers
> - Keep translatable warning strings on one line each, even past 80
> columns
> - Drop the blanket "could not lock" warning that fired even when the
> target wasn't a block device at all (e.g. a regular file), which was
> causing issues with tests; lock_whole_disk() now only warns/errors
> at the point of a genuine failure
> - No change needed for the Ctrl-C/EINTR concern — traced and confirmed
> the existing signal handling already unwinds cleanly
>
> e2fsck/unix.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 101 insertions(+)
>
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index 335ca377..cf21752a 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -36,6 +36,11 @@ extern int optind;
> #ifdef HAVE_SYS_IOCTL_H
> #include <sys/ioctl.h>
> #endif
> +#ifdef __linux__
> +#include <sys/file.h>
> +#include <sys/stat.h>
> +#include <sys/sysmacros.h>
> +#endif
> #ifdef HAVE_MALLOC_H
> #include <malloc.h>
> #endif
> @@ -1397,6 +1402,90 @@ err:
> return retval;
> }
>
> +#ifdef __linux__
> +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> +{
> + struct stat st;
> + char dev_path[256]; /* reused for each sysfs/device path we build */
> + char parent_devnum[32];
> + FILE *f;
> + unsigned int maj, min;
> + int parent_resolved = 0;
> + const char *opened_path;
> + int fd;
> +
> + if (stat(dev_name, &st) < 0)
> + return -1;
> +
> + if (!S_ISBLK(st.st_mode))
> + return -1;
> +
> + maj = major(st.st_rdev);
> + min = minor(st.st_rdev);
> +
> + /*
> + * If dev_name is a partition (sysfs 'partition' attribute exists),
> + * resolve the parent whole-disk device so the flock covers the same
> + * device node that udevd locks before probing any partition on it.
> + */
> + snprintf(dev_path, sizeof(dev_path),
> + "/sys/dev/block/%u:%u/partition", maj, min);
> +
> + if (access(dev_path, F_OK) == 0) {
> + snprintf(dev_path, sizeof(dev_path),
> + "/sys/dev/block/%u:%u/../dev", maj, min);
> +
> + f = fopen(dev_path, "r");
> + if (f) {
> + if (fscanf(f, "%31s", parent_devnum) == 1) {
> + unsigned int pmaj, pmin;
> + if (sscanf(parent_devnum, "%u:%u",
> + &pmaj, &pmin) == 2) {
> + maj = pmaj;
> + min = pmin;
> + parent_resolved = 1;
> + }
> + }
> + fclose(f);
> + }
> + if (!parent_resolved)
> + log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
> + }
> +
> + snprintf(dev_path, sizeof(dev_path), "/dev/block/%u:%u", maj, min);
> +
> + opened_path = dev_path;
> + fd = open(dev_path, O_RDONLY | O_CLOEXEC, 0);
> + if (fd < 0) {
> + fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
> + if (fd < 0) {
> + com_err(ctx->program_name, errno,
> + _("while trying to open %s for locking"), dev_name);
> + return -1;
> + }
> + opened_path = dev_name;
> + if (parent_resolved)
> + log_err(ctx, _("Warning: %s not found; locking %s instead, lock may not prevent udev races\n"), dev_path, dev_name);
> + }
> +
> + while (flock(fd, LOCK_EX) != 0) {
> + if (errno == EINTR) {
> + if (ctx->flags & E2F_FLAG_CANCEL) {
> + close(fd);
> + return -1;
> + }
> + continue;
> + }
> + com_err(ctx->program_name, errno,
> + _("while trying to lock %s"), opened_path);
> + close(fd);
> + return -1;
> + }
> +
> + return fd;
> +}
> +#endif /* __linux__ */
> +
> int main (int argc, char *argv[])
> {
> errcode_t retval = 0, retval2 = 0, orig_retval = 0;
> @@ -1413,6 +1502,9 @@ int main (int argc, char *argv[])
> int journal_size;
> int sysval, sys_page_size = 4096;
> int old_bitmaps;
> +#ifdef __linux__
> + int lock_fd = -1;
> +#endif
> __u32 features[3];
> char *cp;
> enum quota_type qtype;
> @@ -1488,6 +1580,10 @@ int main (int argc, char *argv[])
>
> check_mount(ctx);
>
> +#ifdef __linux__
> + lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
If the filesystem has an external journal device, doesn't that also need
locking?
Why isn't this implemented as part of the unixio manager?
--D
> +#endif
> +
> if (!(ctx->options & E2F_OPT_PREEN) &&
> !(ctx->options & E2F_OPT_NO) &&
> !(ctx->options & E2F_OPT_YES)) {
> @@ -2169,6 +2265,11 @@ skip_write:
> ext2fs_close_free(&ctx->fs);
> free(ctx->journal_name);
>
> +#ifdef __linux__
> + if (lock_fd >= 0)
> + close(lock_fd);
> +#endif
> +
> if (ctx->logf)
> fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> e2fsck_free_context(ctx);
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
2026-08-24 22:40 ` Darrick J. Wong
@ 2026-08-26 14:04 ` Nandakumar Raghavan
2026-08-27 7:33 ` Andreas Dilger
2026-08-27 21:01 ` Theodore Tso
0 siblings, 2 replies; 12+ messages in thread
From: Nandakumar Raghavan @ 2026-08-26 14:04 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-ext4, tytso, adilger, srivatsa
On Mon, Aug 24, 2026 at 03:40:06PM -0700, Darrick J. Wong wrote:
> On Mon, Aug 24, 2026 at 09:15:11AM -0700, Nandakumar Raghavan wrote:
> > During journal replay, e2fsck writes the primary superblock back to disk
> > in multiple I/O operations. The payload lands before the checksum, leaving
> > a transient window where the on-disk superblock has a bad checksum.
> >
> > If udevd processes a change uevent during this window, libblkid probes the
> > primary superblock, finds a checksum mismatch, and concludes the partition
> > has no recognisable filesystem. udev then fires a remove event, wiping all
> > symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> > that depends on those symlinks will fail.
> >
> > udevd already serialises its own partition probes against whole-disk device
> > access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> > event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> > whole-disk device before opening the filesystem. This forces udevd to defer
> > all probes on that disk until e2fsck exits and the lock is released, by
> > which point the filesystem is fully consistent.
> >
> > The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> > so that the lock covers the same device node that udevd locks. If sysfs
> > resolution fails, a warning is emitted and the lock falls back to the
> > partition device itself.
> >
> > This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
> > on block devices) and is compiled out entirely on non-Linux platforms.
>
> You're clearly implementing the "Locking Block Device Access" protocol
> as documented by systemd:
> https://systemd.io/BLOCK_DEVICE_LOCKING/
>
> So why not use libsystemd like the example in that posting provides?
> sd_device_get_parent_with_subsystem_devtype is much less gross than
> open-coding the same logic here.
This was on purpose and not something I missed. The sd_device_get_parent_with_subsystem_devtype
requires libsystemd v251+, and e2fsck needs to keep working in contexts where
that dependency isn't available or isn't guaranteed to be new enough - non-systemd distros, older
LTS systems doing root-fs repair before userspace is fully up, embedded/recovery images, etc.
Taking a hard runtime dependency on a versioned libsystemd API for a tool this fundamental felt
like a much bigger cost than by parsing (/sys/dev/block/MAJ:MIN/partition, .../dev), which only
relies on a stable kernel ABI with no library and no version floor. Happy to reconsider if
the project wants to take the dependency, but wanted to flag the tradeoff explicitly rather
than silently pick one side.
>
> > Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> > ---
> >
> > Changes in v2:
> > - Guard the whole feature (function, call site, lock_fd, cleanup) with
> > #ifdef __linux__ instead of just the function body, so it's fully
> > absent from non-Linux builds
> > - Collapse HAVE_SYS_*_H header guards into a single #ifdef __linux__
> > - Reuse a single dev_path[] buffer instead of three separate stack
> > buffers
> > - Keep translatable warning strings on one line each, even past 80
> > columns
> > - Drop the blanket "could not lock" warning that fired even when the
> > target wasn't a block device at all (e.g. a regular file), which was
> > causing issues with tests; lock_whole_disk() now only warns/errors
> > at the point of a genuine failure
> > - No change needed for the Ctrl-C/EINTR concern — traced and confirmed
> > the existing signal handling already unwinds cleanly
> >
> > e2fsck/unix.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 101 insertions(+)
> >
> > diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> > index 335ca377..cf21752a 100644
> > --- a/e2fsck/unix.c
> > +++ b/e2fsck/unix.c
> > @@ -36,6 +36,11 @@ extern int optind;
> > #ifdef HAVE_SYS_IOCTL_H
> > #include <sys/ioctl.h>
> > #endif
> > +#ifdef __linux__
> > +#include <sys/file.h>
> > +#include <sys/stat.h>
> > +#include <sys/sysmacros.h>
> > +#endif
> > #ifdef HAVE_MALLOC_H
> > #include <malloc.h>
> > #endif
> > @@ -1397,6 +1402,90 @@ err:
> > return retval;
> > }
> >
> > +#ifdef __linux__
> > +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> > +{
> > + struct stat st;
> > + char dev_path[256]; /* reused for each sysfs/device path we build */
> > + char parent_devnum[32];
> > + FILE *f;
> > + unsigned int maj, min;
> > + int parent_resolved = 0;
> > + const char *opened_path;
> > + int fd;
> > +
> > + if (stat(dev_name, &st) < 0)
> > + return -1;
> > +
> > + if (!S_ISBLK(st.st_mode))
> > + return -1;
> > +
> > + maj = major(st.st_rdev);
> > + min = minor(st.st_rdev);
> > +
> > + /*
> > + * If dev_name is a partition (sysfs 'partition' attribute exists),
> > + * resolve the parent whole-disk device so the flock covers the same
> > + * device node that udevd locks before probing any partition on it.
> > + */
> > + snprintf(dev_path, sizeof(dev_path),
> > + "/sys/dev/block/%u:%u/partition", maj, min);
> > +
> > + if (access(dev_path, F_OK) == 0) {
> > + snprintf(dev_path, sizeof(dev_path),
> > + "/sys/dev/block/%u:%u/../dev", maj, min);
> > +
> > + f = fopen(dev_path, "r");
> > + if (f) {
> > + if (fscanf(f, "%31s", parent_devnum) == 1) {
> > + unsigned int pmaj, pmin;
> > + if (sscanf(parent_devnum, "%u:%u",
> > + &pmaj, &pmin) == 2) {
> > + maj = pmaj;
> > + min = pmin;
> > + parent_resolved = 1;
> > + }
> > + }
> > + fclose(f);
> > + }
> > + if (!parent_resolved)
> > + log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
> > + }
> > +
> > + snprintf(dev_path, sizeof(dev_path), "/dev/block/%u:%u", maj, min);
> > +
> > + opened_path = dev_path;
> > + fd = open(dev_path, O_RDONLY | O_CLOEXEC, 0);
> > + if (fd < 0) {
> > + fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
> > + if (fd < 0) {
> > + com_err(ctx->program_name, errno,
> > + _("while trying to open %s for locking"), dev_name);
> > + return -1;
> > + }
> > + opened_path = dev_name;
> > + if (parent_resolved)
> > + log_err(ctx, _("Warning: %s not found; locking %s instead, lock may not prevent udev races\n"), dev_path, dev_name);
> > + }
> > +
> > + while (flock(fd, LOCK_EX) != 0) {
> > + if (errno == EINTR) {
> > + if (ctx->flags & E2F_FLAG_CANCEL) {
> > + close(fd);
> > + return -1;
> > + }
> > + continue;
> > + }
> > + com_err(ctx->program_name, errno,
> > + _("while trying to lock %s"), opened_path);
> > + close(fd);
> > + return -1;
> > + }
> > +
> > + return fd;
> > +}
> > +#endif /* __linux__ */
> > +
> > int main (int argc, char *argv[])
> > {
> > errcode_t retval = 0, retval2 = 0, orig_retval = 0;
> > @@ -1413,6 +1502,9 @@ int main (int argc, char *argv[])
> > int journal_size;
> > int sysval, sys_page_size = 4096;
> > int old_bitmaps;
> > +#ifdef __linux__
> > + int lock_fd = -1;
> > +#endif
> > __u32 features[3];
> > char *cp;
> > enum quota_type qtype;
> > @@ -1488,6 +1580,10 @@ int main (int argc, char *argv[])
> >
> > check_mount(ctx);
> >
> > +#ifdef __linux__
> > + lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
>
> If the filesystem has an external journal device, doesn't that also need
> locking?
Good point, and no, it doesn't currently - lock_whole_disk() is only called on
ctx->filesystem_name. An external journal device (-O journal_dev) is a separate
block device with its own by-uuid symlinks, and I don't have a strong argument
that it's immune to the same class of race during replay. I'd like to treat
this as a follow-up rather than block on it, since it needs a bit more digging
into whether the external journal's own superblock write pattern actually
creates the same transient-bad-checksum window - but I don't want to just wave
it away either.
>
> Why isn't this implemented as part of the unixio manager?
Agreed this is architecturally the more correct home for it - mke2fs, tune2fs,
and resize2fs go through the same unix_io manager and do similar multi-step
writes to live block devices, so they're exposed to the same race and get none
of this protection today. I scoped this patch to e2fsck specifically because
that's the most common/severe case in practice and wanted a narrow, reviewable
first fix rather than a bigger refactor touching every tool at once.
I can look at moving this into unix_io_open()/unix_io_close() as a follow-up
so mke2fs/tune2fs/resize2fs pick it up automatically via ext2fs_open()/close()
>
> --D
>
> > +#endif
> > +
> > if (!(ctx->options & E2F_OPT_PREEN) &&
> > !(ctx->options & E2F_OPT_NO) &&
> > !(ctx->options & E2F_OPT_YES)) {
> > @@ -2169,6 +2265,11 @@ skip_write:
> > ext2fs_close_free(&ctx->fs);
> > free(ctx->journal_name);
> >
> > +#ifdef __linux__
> > + if (lock_fd >= 0)
> > + close(lock_fd);
> > +#endif
> > +
> > if (ctx->logf)
> > fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> > e2fsck_free_context(ctx);
> > --
> > 2.54.0
> >
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
2026-08-26 14:04 ` Nandakumar Raghavan
@ 2026-08-27 7:33 ` Andreas Dilger
2026-08-27 12:20 ` Nandakumar Raghavan
2026-08-27 21:01 ` Theodore Tso
1 sibling, 1 reply; 12+ messages in thread
From: Andreas Dilger @ 2026-08-27 7:33 UTC (permalink / raw)
To: Nandakumar Raghavan; +Cc: Darrick J. Wong, linux-ext4, tytso, srivatsa
On Aug 26, 2026, at 08:04, Nandakumar Raghavan <naraghavan@linux.microsoft.com> wrote:
>
> On Mon, Aug 24, 2026 at 03:40:06PM -0700, Darrick J. Wong wrote:
>> On Mon, Aug 24, 2026 at 09:15:11AM -0700, Nandakumar Raghavan wrote:
>>> During journal replay, e2fsck writes the primary superblock back to disk
>>> in multiple I/O operations. The payload lands before the checksum, leaving
>>> a transient window where the on-disk superblock has a bad checksum.
>>>
>>> If udevd processes a change uevent during this window, libblkid probes the
>>> primary superblock, finds a checksum mismatch, and concludes the partition
>>> has no recognisable filesystem. udev then fires a remove event, wiping all
>>> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
>>> that depends on those symlinks will fail.
>>>
>>> udevd already serialises its own partition probes against whole-disk device
>>> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
>>> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
>>> whole-disk device before opening the filesystem. This forces udevd to defer
>>> all probes on that disk until e2fsck exits and the lock is released, by
>>> which point the filesystem is fully consistent.
>>>
>>> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
>>> so that the lock covers the same device node that udevd locks. If sysfs
>>> resolution fails, a warning is emitted and the lock falls back to the
>>> partition device itself.
>>>
>>> This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
>>> on block devices) and is compiled out entirely on non-Linux platforms.
>>
>> You're clearly implementing the "Locking Block Device Access" protocol
>> as documented by systemd:
>> https://systemd.io/BLOCK_DEVICE_LOCKING/
>>
>> So why not use libsystemd like the example in that posting provides?
>> sd_device_get_parent_with_subsystem_devtype is much less gross than
>> open-coding the same logic here.
> This was on purpose and not something I missed. The sd_device_get_parent_with_subsystem_devtype
> requires libsystemd v251+, and e2fsck needs to keep working in contexts where
> that dependency isn't available or isn't guaranteed to be new enough -
> non-systemd distros, older LTS systems doing root-fs repair before userspace
> is fully up, embedded/recovery images, etc.
> Taking a hard runtime dependency on a versioned libsystemd API for a tool
> this fundamental felt like a much bigger cost than by parsing (/sys/dev/block/MAJ:MIN/partition, .../dev), which only relies on a stable
> kernel ABI with no library and no version floor. Happy to reconsider if
> the project wants to take the dependency, but wanted to flag the tradeoff
> explicitly rather than silently pick one side.
I for one am not eager to have e2fsprogs depend on libsystemd to build.
>>
>>> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
>>> ---
>>>
>>> @@ -1488,6 +1580,10 @@ int main (int argc, char *argv[])
>>>
>>> check_mount(ctx);
>>>
>>> +#ifdef __linux__
>>> + lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
>>
>> If the filesystem has an external journal device, doesn't that also need
>> locking?
>
> Good point, and no, it doesn't currently - lock_whole_disk() is only called on
> ctx->filesystem_name. An external journal device (-O journal_dev) is a separate
> block device with its own by-uuid symlinks, and I don't have a strong argument
> that it's immune to the same class of race during replay. I'd like to treat
> this as a follow-up rather than block on it, since it needs a bit more digging
> into whether the external journal's own superblock write pattern actually
> creates the same transient-bad-checksum window - but I don't want to just wave
> it away either.
At one point I was working on having jbd2 "mount" the journal device, and then
ext4 would connect to the running journal as a "service", rather than having
the journal device be mounted exclusively as subpart of the ext4 filesystem.
I might have even posted patches for this to the list (maybe 15 years ago?)
This was intended to solve a number of issues:
- ensuring that the jbd2 block device was visibly "in use" to userspace
- allow a single jbd2 device (e.g. on NVMe) to be shared among multiple ext4
filesystems (on HDDs)
That shared device would have been much more useful when HDDs were prevalent
and an NVMe device was harder to share (e.g. before LVM/DM). It had some other
issues like what to do if there are journaled blocks to recover but one of the
using filesystems does not mount and recover them? Maybe copy those blocks to
the end of the journal and shrink the usable journal size?
It still has some appeal from the visibility POV, and for sharing it avoids
stranding dedicated NVMe journal space on one inactive filesystem when other
busy filesystems could be sharing the space.
Cheers, Andreas
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
2026-08-27 7:33 ` Andreas Dilger
@ 2026-08-27 12:20 ` Nandakumar Raghavan
0 siblings, 0 replies; 12+ messages in thread
From: Nandakumar Raghavan @ 2026-08-27 12:20 UTC (permalink / raw)
To: Andreas Dilger; +Cc: Darrick J. Wong, linux-ext4, tytso, srivatsa
On Thu, Aug 27, 2026 at 01:33:45AM -0600, Andreas Dilger wrote:
> On Aug 26, 2026, at 08:04, Nandakumar Raghavan <naraghavan@linux.microsoft.com> wrote:
> >
> > On Mon, Aug 24, 2026 at 03:40:06PM -0700, Darrick J. Wong wrote:
> >> On Mon, Aug 24, 2026 at 09:15:11AM -0700, Nandakumar Raghavan wrote:
> >>> During journal replay, e2fsck writes the primary superblock back to disk
> >>> in multiple I/O operations. The payload lands before the checksum, leaving
> >>> a transient window where the on-disk superblock has a bad checksum.
> >>>
> >>> If udevd processes a change uevent during this window, libblkid probes the
> >>> primary superblock, finds a checksum mismatch, and concludes the partition
> >>> has no recognisable filesystem. udev then fires a remove event, wiping all
> >>> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> >>> that depends on those symlinks will fail.
> >>>
> >>> udevd already serialises its own partition probes against whole-disk device
> >>> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> >>> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> >>> whole-disk device before opening the filesystem. This forces udevd to defer
> >>> all probes on that disk until e2fsck exits and the lock is released, by
> >>> which point the filesystem is fully consistent.
> >>>
> >>> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> >>> so that the lock covers the same device node that udevd locks. If sysfs
> >>> resolution fails, a warning is emitted and the lock falls back to the
> >>> partition device itself.
> >>>
> >>> This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
> >>> on block devices) and is compiled out entirely on non-Linux platforms.
> >>
> >> You're clearly implementing the "Locking Block Device Access" protocol
> >> as documented by systemd:
> >> https://systemd.io/BLOCK_DEVICE_LOCKING/
> >>
> >> So why not use libsystemd like the example in that posting provides?
> >> sd_device_get_parent_with_subsystem_devtype is much less gross than
> >> open-coding the same logic here.
> > This was on purpose and not something I missed. The sd_device_get_parent_with_subsystem_devtype
> > requires libsystemd v251+, and e2fsck needs to keep working in contexts where
> > that dependency isn't available or isn't guaranteed to be new enough -
> > non-systemd distros, older LTS systems doing root-fs repair before userspace
> > is fully up, embedded/recovery images, etc.
> > Taking a hard runtime dependency on a versioned libsystemd API for a tool
> > this fundamental felt like a much bigger cost than by parsing (/sys/dev/block/MAJ:MIN/partition, .../dev), which only relies on a stable
> > kernel ABI with no library and no version floor. Happy to reconsider if
> > the project wants to take the dependency, but wanted to flag the tradeoff
> > explicitly rather than silently pick one side.
>
> I for one am not eager to have e2fsprogs depend on libsystemd to build.
>
> >>
> >>> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> >>> ---
> >>>
> >>> @@ -1488,6 +1580,10 @@ int main (int argc, char *argv[])
> >>>
> >>> check_mount(ctx);
> >>>
> >>> +#ifdef __linux__
> >>> + lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> >>
> >> If the filesystem has an external journal device, doesn't that also need
> >> locking?
> >
> > Good point, and no, it doesn't currently - lock_whole_disk() is only called on
> > ctx->filesystem_name. An external journal device (-O journal_dev) is a separate
> > block device with its own by-uuid symlinks, and I don't have a strong argument
> > that it's immune to the same class of race during replay. I'd like to treat
> > this as a follow-up rather than block on it, since it needs a bit more digging
> > into whether the external journal's own superblock write pattern actually
> > creates the same transient-bad-checksum window - but I don't want to just wave
> > it away either.
>
> At one point I was working on having jbd2 "mount" the journal device, and then
> ext4 would connect to the running journal as a "service", rather than having
> the journal device be mounted exclusively as subpart of the ext4 filesystem.
> I might have even posted patches for this to the list (maybe 15 years ago?)
>
> This was intended to solve a number of issues:
> - ensuring that the jbd2 block device was visibly "in use" to userspace
> - allow a single jbd2 device (e.g. on NVMe) to be shared among multiple ext4
> filesystems (on HDDs)
>
> That shared device would have been much more useful when HDDs were prevalent
> and an NVMe device was harder to share (e.g. before LVM/DM). It had some other
> issues like what to do if there are journaled blocks to recover but one of the
> using filesystems does not mount and recover them? Maybe copy those blocks to
> the end of the journal and shrink the usable journal size?
>
> It still has some appeal from the visibility POV, and for sharing it avoids
> stranding dedicated NVMe journal space on one inactive filesystem when other
> busy filesystems could be sharing the space.
>
> Cheers, Andreas
Thanks for the history, Andreas and I can see how it would solve the visibility
problem more fundamentally than one-off locking external journal devices case by case.
That said, it sounds like a much bigger architectural change than what I'm
trying to do here. I'll keep it in mind as background, but for this patch
I'd still like to treat "should the external journal device also get
locked" as a narrower, separate follow-up rather than pull in a jbd2
redesign.
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
2026-08-26 14:04 ` Nandakumar Raghavan
2026-08-27 7:33 ` Andreas Dilger
@ 2026-08-27 21:01 ` Theodore Tso
1 sibling, 0 replies; 12+ messages in thread
From: Theodore Tso @ 2026-08-27 21:01 UTC (permalink / raw)
To: Nandakumar Raghavan; +Cc: Darrick J. Wong, linux-ext4, adilger, srivatsa
On Wed, Aug 26, 2026 at 07:04:48AM -0500, Nandakumar Raghavan wrote:
> This was on purpose and not something I missed. The sd_device_get_parent_with_subsystem_devtype
> requires libsystemd v251+, and e2fsck needs to keep working in contexts where
> that dependency isn't available or isn't guaranteed to be new enough - non-systemd distros, older
> LTS systems doing root-fs repair before userspace is fully up, embedded/recovery images, etc.
Cqn you name any other OS that has implemented
https://systemd.io/BLOCK_DEVICE_LOCKING/
*other* than systemd? So for non-systemd distros, who cares if we
don't have the functionality? So what we could do is to use autoconf
with something like this:
AC_CHECK_LIB(libsystemd, sd_device_get_parent_with_subsystem_devtype,
AC_DEFINE(HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE, 1,
[Define to 1 if libsystemd has sd_device_get_parent_with_subsystem_devtype]))
We would only do the flock(LOCK_EX) if and only if the function is
present. If it isn't, say on MacOS, FreeBSD, or RHEL 7 we won't play
the systemd locking game --- which is fine, because it doesn't matter
on those platforms.
If you care about running e2fsck built on RHEL 8 on RHEL 7, that's an
easy problem to solve. A executable linked using shared libraries
won't work already since the shared libraries on RHEL 8 are too new.
But we have a solution already, which is to use e2fsck.static, which
is statically linked.
> Taking a hard runtime dependency on a versioned libsystemd API for a tool this fundamental felt
> like a much bigger cost than by parsing (/sys/dev/block/MAJ:MIN/partition, .../dev), which only
> relies on a stable kernel ABI with no library and no version floor. Happy to reconsider if
> the project wants to take the dependency, but wanted to flag the tradeoff explicitly rather
> than silently pick one side.
If we really cared about taking a binary built on a system with
libsystemd. and running on a system without libsystemd, another
solution that we've used is dlopen.
For an example of that's done, see misc/create_inode_libarchve.c and
the associated autoconf tests in configure.ac in the e2fsprogs
sources. This is how "mke2fs -d embed_rootfs.tar.gz -t ext4 embed.img
1G" is handled without needing a hard dependency on libarchive being
installed. If it's not installed then you'll just get the error
message "you need libarchive to be able to process tarballs".
I don't think the dlopen() approach is needed here, though.
Cheers,
- Ted
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check
2026-08-24 16:15 ` [PATCH v2] " Nandakumar Raghavan
2026-08-24 22:20 ` Andreas Dilger
2026-08-24 22:40 ` Darrick J. Wong
@ 2026-08-28 21:30 ` ddstreet
2 siblings, 0 replies; 12+ messages in thread
From: ddstreet @ 2026-08-28 21:30 UTC (permalink / raw)
To: Nandakumar Raghavan; +Cc: linux-ext4, tytso, adilger, srivatsa, ddstreet
On Mon, 24 Aug 2026, Nandakumar Raghavan wrote:
> udevd already serialises its own partition probes against whole-disk device
> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> whole-disk device before opening the filesystem. This forces udevd to defer
> all probes on that disk until e2fsck exits and the lock is released, by
> which point the filesystem is fully consistent.
I believe this was already part of fsck, and it conflicted with udevd and so was removed:
https://bugs.freedesktop.org/show_bug.cgi?id=79576
https://github.com/util-linux/util-linux/commit/3bbdae633f4a1dda5f95ee6c61f18a1c8ef12250
That particular problem was already fixed in systemd-udevd:
https://github.com/systemd/systemd/commit/5d354e525a5
But in general, does it really make sense for _only_ e2fsck to lock
the device? Wouldn't it make more sense to add the device flock back
into fsck itself? Or if the problem is during boot between
systemd-fsck and systemd-udevd, wouldn't it be better to fix the issue
in systemd-fsck?
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-08-28 21:31 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20 14:32 [PATCH] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check Nandakumar Raghavan
2026-08-15 5:29 ` Nandakumar Raghavan
2026-08-18 22:44 ` Andreas Dilger
2026-08-24 13:24 ` Nandakumar Raghavan
2026-08-24 16:15 ` [PATCH v2] " Nandakumar Raghavan
2026-08-24 22:20 ` Andreas Dilger
2026-08-24 22:40 ` Darrick J. Wong
2026-08-26 14:04 ` Nandakumar Raghavan
2026-08-27 7:33 ` Andreas Dilger
2026-08-27 12:20 ` Nandakumar Raghavan
2026-08-27 21:01 ` Theodore Tso
2026-08-28 21:30 ` ddstreet
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.