Re: [PATCH v2] libfrog: make cmn_err() emit each message atomically to avoid torn output

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Jul 16, 2026 at 09:45:49PM +0200, Avinesh Kumar wrote:
> From: Avinesh Kumar <avinesh.kumar@suse.com>
> 
> fstests xfs/033 fails sporadically with a spurious blank line in the
> xfs_repair output:
> 
> - output mismatch (see /opt/xfstests/results//xfs/033.out.bad)
>     --- tests/xfs/033.out	2026-06-24 15:52:51.000000000 -0400
>     +++ /opt/xfstests/results//xfs/033.out.bad	2026-07-14 18:54:46.582495041 -0400
>     @@ -103,6 +103,7 @@
>      Phase 3 - for each AG...
>              - scan and clear agi unlinked lists...
>              - process known inodes and perform inode discovery...
>     +
>      bad magic number 0xffff on inode INO
>      bad version number 0xffffffff on inode INO
>      inode identifier 18446744073709551615 mismatch on inode INO
> 
> Root cause is in cmn_err() (libfrog/util.c), which emits a message
> and its newline as two separate writes to unbuffered stderr.
> xfs_repair's threads all share stderr. If one is preempted between the
> two writes, another thread's line lands in between:
> 
> (snips from `cat -A 033.raw`) -
> Phase 3 - for each AG...$
>         - scan and clear agi unlinked lists...$
>         - process known inodes and perform inode discovery...$
> Metadata corruption detected at 0x445dd3, xfs_inode block 0x80/0x4000        - agno = 0$
> $
> bad CRC for inode 128$
> bad magic number 0x0 on inode 128$
> 
> which should be like -
> 
> Phase 3 - for each AG...$
>         - scan and clear agi unlinked lists...$
>         - process known inodes and perform inode discovery...$
> Metadata corruption detected at 0x445dd3, xfs_inode block 0x80/0x4000$
>         - agno = 0$
> bad CRC for inode 130$
> bad magic number 0x0 on inode 130$
> 
> _filter_repair() strips the noise line it glued onto but not the lone
> newline, which then fails the golden diff.
> 
> Make the two writes atomic.
> 
> do_error() in xfs_repair.c also has same issue, fix that too.
> 
> Signed-off-by: Avinesh Kumar <avinesh.kumar@suse.com>

Looks good to me, hopefully this will cut down on the random xfs_repair
errors in fstests.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
> v2: fix the do_error() in xfs_repair.c too.
> 
>  libfrog/util.c      | 2 ++
>  repair/xfs_repair.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/libfrog/util.c b/libfrog/util.c
> index 5bae5bab..3b6df917 100644
> --- a/libfrog/util.c
> +++ b/libfrog/util.c
> @@ -116,8 +116,10 @@ cmn_err(int level, char *fmt, ...)
>  	va_list	ap;
>  
>  	va_start(ap, fmt);
> +	flockfile(stderr);
>  	vfprintf(stderr, fmt, ap);
>  	fputs("\n", stderr);
> +	funlockfile(stderr);
>  	va_end(ap);
>  }
>  
> diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
> index 6b97a806..dd758ebc 100644
> --- a/repair/xfs_repair.c
> +++ b/repair/xfs_repair.c
> @@ -458,10 +458,12 @@ do_error(char const *msg, ...)
>  {
>  	va_list args;
>  
> +	flockfile(stderr);
>  	fprintf(stderr, _("\nfatal error -- "));
>  
>  	va_start(args, msg);
>  	vfprintf(stderr, msg, args);
> +	funlockfile(stderr);
>  	if (dumpcore)
>  		abort();
>  	exit(1);
> -- 
> 2.55.0
>