Is that a typo in ffs_softdep.c

Max Khon fjoe at iclub.nsu.ru
Tue Mar 2 11:08:44 PST 2004


Hello!

On Fri, Feb 13, 2004 at 05:16:17PM +0800, rjshang wrote:

> It looks like a typo in ffs_softdep.c softdep_sync_metadata().
> The second parameter of drain_output is used to tell if it is 
> protected by &lk.
> Is there other concern?
> Please check the following patch from 4.9 release.
>
> --- ffs_softdep.c	Wed Feb  6 02:46:53 2002
> +++ ffs_softdep.new.c	Fri Feb 13 16:48:52 2004
> @@ -4299,29 +4299,29 @@
>  		return (0);
>  	}
>  
>  	FREE_LOCK(&lk);
>  	/*
>  	 * If we are trying to sync a block device, some of its buffers may
>  	 * contain metadata that cannot be written until the contents of 
> some
>  	 * partially written files have been written to disk. The only easy
>  	 * way to accomplish this is to sync the entire filesystem (luckily
>  	 * this happens rarely).
>  	 *
>  	 * We must wait for any I/O in progress to finish so that
>  	 * all potential buffers on the dirty list will be visible.
>  	 */
> -	drain_output(vp, 1);
> +	drain_output(vp, 0);
>  	if (vn_isdisk(vp, NULL) && 
>  	    vp->v_specmountpoint && !VOP_ISLOCKED(vp, NULL) &&
>  	    (error = VFS_SYNC(vp->v_specmountpoint, MNT_WAIT, ap->a_cred,
>  	     ap->a_p)) != 0)
>  		return (error);
>  	return (0);
>  }
>  
>  /*
>   * Flush the dependencies associated with an inodedep.
>   * Called with splbio blocked.
>   */
>  static int
>  flush_inodedep_deps(fs, ino)

Nice catch!

drain_output() should be called earlier (before chechkig dirty list).
The bug was introduced in ffs_softdep.c rev. 1.57.2.10.
Correct patch is attached.
I sent this patch to Kirk for review and will commit it as soon as I get
an approval from him.

/fjoe


More information about the freebsd-fs mailing list