linux-kernel - Re: [PATCH 3/6] xfs: replace kthread freezing with auto fs freezing

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: <20250401-baubeginn-ausdehnen-3a7387b756aa@brauner>
Date: Tue, 1 Apr 2025 09:17:12 +0200
From: Christian Brauner <brauner@...nel.org>
To: Dave Chinner <david@...morbit.com>
Cc: linux-fsdevel@...r.kernel.org, jack@...e.cz, 
	Ard Biesheuvel <ardb@...nel.org>, linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org, 
	James Bottomley <James.Bottomley@...senpartnership.com>, mcgrof@...nel.org, hch@...radead.org, rafael@...nel.org, 
	djwong@...nel.org, pavel@...nel.org, peterz@...radead.org, mingo@...hat.com, 
	will@...nel.org, boqun.feng@...il.com
Subject: Re: [PATCH 3/6] xfs: replace kthread freezing with auto fs freezing

On Tue, Apr 01, 2025 at 12:11:04PM +1100, Dave Chinner wrote:
> On Tue, Apr 01, 2025 at 02:32:48AM +0200, Christian Brauner wrote:
> > From: Luis Chamberlain <mcgrof@...nel.org>
> > 
> > The kernel power management now supports allowing the VFS
> > to handle filesystem freezing freezes and thawing. Take advantage
> > of that and remove the kthread freezing. This is needed so that we
> > properly really stop IO in flight without races after userspace
> > has been frozen. Without this we rely on kthread freezing and
> > its semantics are loose and error prone.
> > 
> > The filesystem therefore is in charge of properly dealing with
> > quiescing of the filesystem through its callbacks if it thinks
> > it knows better than how the VFS handles it.
> > 
> .....
> 
> > diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> > index 0fcb1828e598..ad8183db0780 100644
> > --- a/fs/xfs/xfs_trans_ail.c
> > +++ b/fs/xfs/xfs_trans_ail.c
> > @@ -636,7 +636,6 @@ xfsaild(
> >  	unsigned int	noreclaim_flag;
> >  
> >  	noreclaim_flag = memalloc_noreclaim_save();
> > -	set_freezable();
> >  
> >  	while (1) {
> >  		/*
> > @@ -695,8 +694,6 @@ xfsaild(
> >  
> >  		__set_current_state(TASK_RUNNING);
> >  
> > -		try_to_freeze();
> > -
> >  		tout = xfsaild_push(ailp);
> >  	}
> >  
> 
> So what about the TASK_FREEZABLE flag that is set in this code
> before sleeping?
> 
> i.e. this code before we schedule():
> 
>                 if (tout && tout <= 20)
>                         set_current_state(TASK_KILLABLE|TASK_FREEZABLE);
>                 else
>                         set_current_state(TASK_INTERRUPTIBLE|TASK_FREEZABLE);
> 
> Shouldn't TASK_FREEZABLE go away, too?

Thanks for spotting! Yes, yesterday late at night I just took Luis
patches as they are and had only gotten around to testing btrfs. The
coccinelle scripts seemed to have missed those. I'll wait for comments
and will do another pass and send out v2.

> > diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> > index c5136ea9bb1d..1875b6551ab0 100644
> > --- a/fs/xfs/xfs_zone_gc.c
> > +++ b/fs/xfs/xfs_zone_gc.c
> > @@ -993,7 +993,6 @@ xfs_zone_gc_handle_work(
> >  	}
> >  
> >  	__set_current_state(TASK_RUNNING);
> > -	try_to_freeze();
> >  
> >  	if (reset_list)
> >  		xfs_zone_gc_reset_zones(data, reset_list);
> > @@ -1041,7 +1040,6 @@ xfs_zoned_gcd(
> >  	unsigned int		nofs_flag;
> >  
> >  	nofs_flag = memalloc_nofs_save();
> > -	set_freezable();
> >  
> >  	for (;;) {
> >  		set_current_state(TASK_INTERRUPTIBLE | TASK_FREEZABLE);
> 
> Same question here for this newly merged code, too...

I'm not sure if this is supposed to be a snipe or not but just in case
this is a hidden question: This isn't merged. Per the cover letter this
is in a work.* branch. Anything that is considered mergable is in
vfs-6.16.* branches. But since we're pre -rc1 even those branches are
not yet showing up in -next.