Re: [PATCH -mm] swsusp: Freeze filesystems during suspend

[Pavel Machek <pavel@ucw.cz>]

Hi!

> Freeze all filesystems during the suspend by calling freeze_bdev() for each of
> them and thaw them during the resume using thaw_bdev().
> 
> This is needed by swsusp, because some filesystems (eg. XFS) use work queues
> and worker_threads run with PF_NOFREEZE set, so they can cause some writes
> to be performed after the suspend image has been created which may corrupt
> the filesystem.  The additional benefit of it is that if the resume fails, the
> filesystems will be in a consistent state and there won't be any journal replays
> needed.
> 
> The freezing of filesystems is carried out when processes are being frozen, so
> on the majority of architectures it also will happen during a
> suspend to RAM.


> @@ -119,7 +120,7 @@ int freeze_processes(void)
>  		read_unlock(&tasklist_lock);
>  		todo += nr_user;
>  		if (!user_frozen && !nr_user) {
> -			sys_sync();
> +			freeze_filesystems();
>  			start_time = jiffies;
>  		}
>  		user_frozen = !nr_user;


Do all filesystems implement freeze? If not, we may want to keep that
sync...


> @@ -156,28 +157,43 @@ int freeze_processes(void)
>  void thaw_some_processes(int all)
>  {
>  	struct task_struct *g, *p;
> -	int pass = 0; /* Pass 0 = Kernel space, 1 = Userspace */
>  
>  	printk("Restarting tasks... ");
>  	read_lock(&tasklist_lock);
> -	do {
> -		do_each_thread(g, p) {
> -			/*
> -			 * is_user = 0 if kernel thread or borrowed mm,
> -			 * 1 otherwise.
> -			 */
> -			int is_user = !!(p->mm && !(p->flags & PF_BORROWED_MM));
> -			if (!freezeable(p) || (is_user != pass))
> -				continue;
> -			if (!thaw_process(p))
> -				printk(KERN_INFO
> -					"Strange, %s not stopped\n", p->comm);
> -		} while_each_thread(g, p);
>  
> -		pass++;
> -	} while (pass < 2 && all);
> +	do_each_thread(g, p) {
> +		if (!freezeable(p))
> +			continue;
> +
> +		/* Don't thaw userland processes, for now */
> +		if (p->mm && !(p->flags & PF_BORROWED_MM))
> +			continue;
> +
> +		if (!thaw_process(p))
> +			printk(KERN_INFO " Strange, %s not stopped\n", p->comm );
> +	} while_each_thread(g, p);
> +
> +	read_unlock(&tasklist_lock);
> +	if (!all)
> +		goto Exit;
> +
> +	thaw_filesystems();
> +	read_lock(&tasklist_lock);
> +
> +	do_each_thread(g, p) {
> +		if (!freezeable(p))
> +			continue;
> +
> +		/* Kernel threads should have been thawed already */
> +		if (!p->mm || (p->flags & PF_BORROWED_MM))
> +			continue;
> +
> +		if (!thaw_process(p))
> +			printk(KERN_INFO " Strange, %s not stopped\n", p->comm );
> +	} while_each_thread(g, p);
>  
>  	read_unlock(&tasklist_lock);
> +Exit:
>  	schedule();
>  	printk("done.\n");


Could we do without the code duplication?
>  
> +/**
> + * freeze_filesystems - lock all filesystems and force them into a consistent
> + * state
> + */
> +void freeze_filesystems(void)
> +{
> +	struct super_block *sb;
> +
> +	lockdep_off();

You should not just turn off lockdep because you don't like its
output.

Perhaps tasklist_lock does not nest with whatever freeze_bdev needs?

						Pavel
-- 
Thanks, Sharp!