Re: [PATCH] ubifs: Add new mount option to force fdatasync before rename

[Richard Weinberger <richard@nod.at>]

Hi!

Am 28.09.2015 um 22:38 schrieb Nikhilesh Reddy:
>> ...and these applications are buggy by design.
>> ext4 has some hacks to detect some misuses (IIRC replace by rename and replace by truncate)
>> as these applications worked by chance on ext3.
>> Adding such a hack now to UBIFS needs a bit more justification.
>> Especially as your new mount option is a sledgehammer.
>>
>> Which application triggers this issue?
>> I'm asking because UBIFS is more or less an embedded filesystem.
>> On ext4 mostly broken GUI programs like eclipse or kwrite forgot to fsync().
> 
> Thanks Richard for lookign into this patch.
> I completely agree with you on the fact that these applications are indeed buggy.
> But yes the issues were seen on embedded systems.
> We saw this issue when debugging a few applications that used an xml parser library.
> to write  data.

libxml2?

> There were a few other applications as well but i dont have access to their source.
> Fixing all the applications is not exactly feasible since they may have bugs in multiple places.
> And sometimes we dont have a legal go ahead to fix code that is from thirdparties who may never fix their code... or just distribute a s binaries.
> This change was made due to multiple requests that came from our customers who ran into this issue on the applications that they run on their products.
> 
> We could  use "-o sync" mount option. But this makes UBIFS perform badly that just syncing the old inode data alone.
> The idea was to have a mount point option that could be enabled only as needed and taking a performance hit during a rename.
> All the tests showed no real performance degradation.

Hmm, I'd have assumed that programs with heavy rename() usage would degrade.

> Since it would be disabled by default the normal mount without this would have no impact what so ever to the current behavior.
> Only on filesystems that are mounted with this option will this new behavior kick in.
> 
> Please do consider applying the patch.
> If you have any suggestions on improving this patch to you liking please do let me know and I am happy to make any chances that you deem necessary.

Instead of making all rename() synchronous it would be a good start to detect broken
patterns like ext4 does.
I'll happily test and review those. :-)

Thanks,
//richard