Re: [PATCH] let fat handle MS_SYNCHRONOUS flag

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Colin Leroy <colin@colino.net> writes:

> @@ -764,6 +765,11 @@
>  	dir->i_atime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
>  	mark_inode_dirty(dir);
>  
> +	sb = dir->i_sb;
> +
> +	if (sb->s_flags & MS_SYNCHRONOUS)
> +		sync_dirty_buffer(bh);
> +

This bh is already released.

>  	retval = generic_file_write(filp, buf, count, ppos);
>  	if (retval > 0) {
>  		inode->i_mtime = inode->i_ctime = CURRENT_TIME;
>  		MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
>  		mark_inode_dirty(inode);
> +		if (sb->s_flags & MS_SYNCHRONOUS) {
> +			bh = sb_bread(sb, MSDOS_SB(sb)->fsinfo_sector);
> +			if (bh != NULL) {
> +				sync_dirty_buffer(bh);
> +				brelse(bh);
> +			} else {
> +				BUG_ON(1);
> +			}
> +		}

FAT12/16 doesn't have FSINFO sector.  And if sb_bread() returns NULL,
we should return the valid error.

> @@ -105,4 +118,8 @@
>  	unlock_kernel();
>  	inode->i_ctime = inode->i_mtime = CURRENT_TIME;
>  	mark_inode_dirty(inode);
> +	if (sb->s_flags & MS_SYNCHRONOUS) {
> +		bh = sb_bread(sb, sbi->fsinfo_sector);
> +		sync_dirty_buffer(bh);
> +	}

Ditto.

Aren't you forgetting to update the inode and various metadata (e.g. FAT)?
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>