Re: [f2fs-dev] [PATCH 1/3] f2fs: turn off atomic writes when deteting abnormal behaviors

[Jaegeuk Kim]

On 07/23, Chao Yu wrote:
> On 2018/7/23 21:03, Jaegeuk Kim wrote:
> > On 07/16, Chao Yu wrote:
> >> On 2018/7/15 9:11, Jaegeuk Kim wrote:
> >>> In order to prevent abusing atomic writes by abnormal users, we've added a
> >>> threshold, 20% over memory footprint, which disallows further atomic writes.
> >>> Previously, however, SQLite doesn't know the files became normal, so that
> >>> it could write stale data and commit on revoked normal database file.
> >>>
> >>> Once f2fs detects such the abnormal behavior, this patch simply disables
> >>> all the atomic operations such as:
> >>> - write_begin() gives EINVAL to avoid stale data writes, and SQLite will call
> >>>   F2FS_IOC_ABORT_VOLATILE_WRITE to notify aborting the transaction,
> >>> - F2FS_IOC_START_ATOMIC_WRITE gives EINVAL for SQLite to fall back normal
> >>>   journal_mode,
> >>> - F2FS_IOC_COMMIT_ATOMIC_WRITE gives EINVAL, if the file was revoked, so that
> >>>   Framework retries to submit the transaction given propagated SQLite error.
> >>>
> >>> Note that, this patch also turns off atomic operations, if foreground GC tries
> >>> to move atomic files too frequently.
> >>
> >> Well, how about just keeping original implementation: shutdown atomic write for
> >> those files which are really affect fggc? Since intention of the original
> >> behavior is targeting to abnormal atomic write usage, e.g. open atomic_write
> >> file for very long time, then fggc will be blocked each time when moving its
> >> block. So shutdown it, fggc will recover.
> > 
> > The point here is stopping sqlite to keep going wrong data writes even after
> > we already revoked blocks.
> 
> Yes, that's correct, what I mean is that if we can do that with smaller
> granularity like just revoke blocks for file which is blocking fggc, it will
> affect system/sqlite flow much less than forcing closing all atomic_write.

How about this?