Re: [PATCH v3] fat: editions to support fat_fallocate

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

Namjae Jeon <linkinjeon@gmail.com> writes:

>> If so, probably, I didn't clear my opinion/suggestion, or misled
>> you. Sorry about it.
>>
>> My opinion/suggestion is, "it should be before umount()".
>> I.e. fallocate() doesn't have any affect to FAT on clean state (clean
>> umount).
>>
>> To clear my state, I don't have strong opinion about implementation yet.
>> For example, about ->release() or ->evict_inode().
>>
>> So, if you had reason to use ->release() over "we discussed", it would
>> be good. (Or, if you still didn't have reasons, we would be better to
>> think about it)
> We considered all the possible points where we can release the
> pre-allocated blocks.
> As the "pre-allocation" is file based, we think it need to have
> release mechanism.
>
> In case of evict, Eviction will either happen when the file is removed
> or the inode is evicted from the cache by memory pressure when no
> references are present for that file.
> It is good that evict will be triggered in umount. but there is a risk
> that umount time can be increased.
> And It show users inconsistent result. e.g sometime user can not get
> file discarded fallocated blocks by memory pressure.
>
> Let me know your opinion.

Yes. My personal opinion is almost same, but I see some trade-off, and
it is why I can't decide easily.

Although I don't care about inconsistency on umount time. Because
inconsistency window is opening while user is holding reference to
inode, the window can already be enough big, and inconsistent only
happens with crash.  And if crashed, after all, both of ->release and
->evict requires fsck to recover FS.

Possibly longer umount time (batch modify) and longer close time (on
demand modify) are simply trade-off.

Predictable behavior would matter. Yes, evict time doesn't guarantee to
fallocate() is still available, however evict can be able to say it just
guarantees to available fallocated blocks between open() and
close(). Users have to call fallocate() again after close(), but
fallocate() may skip actual allocation if blocks still available.

I see optimization window with this evict behavior, because it can do
batch discard of fallocated blocks on multiple files, and update FAT at
once.

However, ->release() would be simpler, and would more predictable.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>