Re: [PATCH] Write the inode itself in block_fsync()

[Bart Samwel <bart@samwel.tk>]

OGAWA Hirofumi wrote:
> Bart Samwel <bart@samwel.tk> writes:
> 
>> Andrew Morton wrote:
>>> Sam Vilain <sam@vilain.net> wrote:
>>>> OGAWA Hirofumi wrote:
>>  >>
>>>>  Ouch... won't that halve performance of database transaction logs?
>>> Yes, it could well cause a lot more seeking to do atime and/or mtime
>>> writes.   Which aren't terribly important, really.
>>>
>>> Unless I'm missing something, I suspect we'd be better off without this,
>>> even though it's a correctness fix :(
>> Maybe atime/mtime aren't important, but I would be unhappy if a file 
>> size change wasn't written to disk on fsync.
> 
> Please don't worry, we should be doing a right thing for normal files
> already. This patch is just for block device file.

Ahhh, I missed that. I interpreted:

 >For block device's inode, we don't write a inode's meta data
 >itself. But, I think we should write inode's meta data for fsync().

as "for block devices we don't, for normal files, yes", but apparently 
that's not what you meant. :-)

>> Anyway, shouldn't databases be using a combination of fixed-size files 
>> and fdatasync? fsync doesn't perform well by definition, and I guess the 
>> only reason databases still use it is because the kernel failed to 
>> implement the sucky part of the behaviour.
> 
> Yes, I agree. The changes of atime/mtime only sets I_DIRTY_SYNC, so,
> usually this patch doesn't change fdatasync() at all.
> 
> Umm... however, I also can understand what akpm says.... check some databases.
> 
> 	berkeley db 4.4: use fdatasync() if available
>         mysql 5.0:	 use fdatasync() if available (innobase)
> 			 use fsync() (bdb)
> 	postgresql:	 use fdatasync() if available
> 	sqlite:		 use fsync

Nice piece of info. Apparently all of the "large" database engines can 
use fdatasync, only the smaller ones (bdb, sqlite) don't. I've done some 
extra research:

* From a quick look at the docs it seems to me that bdb can't be 
configured to put its transaction log directly on a block device, so bdb 
won't be affected.

* SQLite definitely can't write logs to a block device, the docs 
explicitly say that the transaction log is a regular file with a 
specific name, so we can write off sqlite as well. (It does seem to use 
fdatasync btw, since version 3.2.6, see http://www.sqlite.org/changes.html.)

If we've missed none, that leaves only proprietary databases at risk. 
But I would be genuinely surprised if a database like Oracle would use 
fsync. If we assume that Oracle et al. are not a problem, the risks of 
this patch are very low.

Cheers,
Bart