linux-kernel - Re: [RFC PATCH 4/4] vfs: add filesystem freeze/thaw callbacks for power management

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <018ff37dde2bacf47f5d5f6aacd7f560795385b3.camel@HansenPartnership.com>
Date: Fri, 28 Mar 2025 12:15:55 -0400
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: Christian Brauner <brauner@...nel.org>
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
 mcgrof@...nel.org, jack@...e.cz, hch@...radead.org, david@...morbit.com, 
 rafael@...nel.org, djwong@...nel.org, pavel@...nel.org,
 peterz@...radead.org,  mingo@...hat.com, will@...nel.org,
 boqun.feng@...il.com
Subject: Re: [RFC PATCH 4/4] vfs: add filesystem freeze/thaw callbacks for
 power management

On Fri, 2025-03-28 at 16:52 +0100, Christian Brauner wrote:
[...]
> 
> > various operations from races.  Taken exclusively with down_write
> > and shared with down_read. Private functions internal to super.c
> > wrap this with grab_super and super_lock_shared/excl() wrappers.
> 
> See also the Documentation/filesystems/lock I added.

you mean locking.rst which covers s_umount?  It would be nice to add
the others as well.

> > The explicit freeze/thaw_super() functions require the s_umount
> > rwsem in down_write or exclusive mode and take it as the first step
> > in their operation.  Looking at the locking in
> > fs_bdev_freeze/thaw() implies that the super_operations
> > freeze_super/thaw_super *don't* need this taken (presumably they
> > handle it internally).
> 
> Block device locking cannot acquire the s_umount as that would cause
> lock inversion with the block device open_mutex. The locking scheme
> using sb_lock and the holder mutex allow safely acquiring the
> superblock. It's orthogonal to what you're doing though.

OK, but based on the above and the fact that the code has to call
either the super op freeze/thaw_super or the global call, I think this
can be handled in the callback as something like rather than trying to
thread an exclusive s_umount:

static void filesystems_thaw_callback(struct super_block *sb)
{
	if (unlikely(!atomic_inc_not_zero(&sb->s_active)))
		return;

	if (sb->s_op->thaw_super)
		sb->s_op->thaw_super(sb, FREEZE_MAY_NEST
				     | FREEZE_HOLDER_KERNEL
				     | freeze_flags);
	else if (sb->s_bdev)
		thaw_super(sb,	FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL
			   | freeze_flags);

	deactivate_super(sb);
}


Regards,

James