Hi Park,
On 2018/3/30 21:13, Ju Hyung Park wrote:
> Hi Chao,
>
>> As I said, now we'd better to add a option in mkfs to disable default list
>> first. If you have time to work on this, I'm glad to review the patch.;)
>
> Yeah, I'll add that to my TODOs list :)
>
>> What I mean is we'd better to unify the list into one place
>
> Ahhhh..
> Now I understand why you had issues with the idea of kernelspace extension
> list.
>
> I've just checked the implementation of sysfs extension list update.
> I thought it'd be just a separate list handled within kernelspace f2fs
> volatile memory.
>
> Looks like it's an update to the existing superblock list, then you
> would be right,
> it'll get much more complicated.
>
> Now, the best idea I can come up with is:
> - Move the default list from mkfs.f2fs to kernelspace and remove the
> list from mkfs.
> - Store user's added/removed extensions separately in the superblock.
>
> This will break compatibility with older f2fs and f2fs-tools, so I
> don't think this is a good idea.
> Can you come up with a better idea?
Not yet, except supporting a optional full list in mkfs... :P
>
> In summary, we need:
> - Always up-to-date default list, not fixed at the time of mkfs
> - Maintaining compatibility with different f2fs and f2fs-tools versions
> - Respecting user's choice to add or remove extensions
Yeah, I just think we need to simply extension list usage for user, also simply
kernel/tools implementation.
Thanks,
>
> Thanks.
> (This is more complicated than I originally anticipated, xD)
>
>
> On Fri, Mar 30, 2018 at 8:19 PM, Chao Yu <yuch...@huawei.com> wrote:
>> Hi Park,
>>
>> On 2018/3/29 23:54, Ju Hyung Park wrote:
>>> Hi Chao,
>>>
>>>> I think this is real hardcoded one
>>>
>>> Agreed, but I can't figure out a better way of doing this.
>>> I still don't think fixing it at mkfs point isn't a good idea.
>>>
>>> Doing this entirely on the userspace also won't make much sense since
>>> I would not trust the distros to ship "good extension lists".
>>
>> Hmm, if user can not trust these recommended extension list, we can introduce
>> mount option to let user to choose to disable all these default extensions,
>> and
>> do the configuration with -e to add their own extensions. Once they want to
>> update the list, sysfs entry will be recommended to make change.
>>
>>>
>>>> which will be hard to configure with if user
>>>> don't want this list at all.
>>>
>>> The new list is very conservative.
>>> Stuffs like .tar and .zip, which most users won't randomly write to,
>>> were excluded intentionally just by the fact that
>>> those are *capable* of random updates.
>>> Even compression methods like .bz2 were excluded since those are
>>> consisted of independent compressed blocks,
>>> which would mean random updates might be possible.
>>>
>>> I don't see why anyone would want to disable any of those,
>>> (even video editors won't be able to randomly write on mp4/mkv files)
>>> but they are still free to do so with the sysfs interface.
>>
>> Of course, but I doubt that in some private system, people may define .mp4 as
>> own extension of one kind of non-audio file, and do not need to add this type
>> extension, I think our filesystem should consider to allow user's custom
>> extension, instead of current hardcoded one.
>>
>> As I said, now we'd better to add a option in mkfs to disable default list
>> first. If you have time to work on this, I'm glad to review the patch.;)
>>
>>>
>>> The way I see it is that doing this in the kernel's default
>>> should be fine with almost every single users.
>>>
>>>> we have to use mount option
>>>
>>> Adding a mount option to bypass and disable
>>> any sort of kernelspace / mkfs list would be also nice.
>>>
>>>> We can add an new mkfs option to disable old common list,
>>>
>>> This would be also nice.
>>> The current code only appends user's list on the existing list.
>>>
>>>> then user can
>>>> enable/disable cold/hot file type as they prefer on mkfs/run-time.
>>>
>>> My point was that most users won't even know that the list has
>>> changed from f2fs-tools.
>>> Some users won't even aware of cold/hot concept in f2fs.
>>> If we do this from the kernel, just using the latest version of f2fs
>>> will bring their extension list up-to-date.
>>>
>>>> One question, how can you handle the conflict in between old list generated
>>>> during mkfs and new common list in run-time kernel?
>>>
>>> Valid point, but I think the added overhead of comparing both lists,
>>> possibly re-comparing the same extension, would be negligible.
>>> Can strcmp'ing ~70 entries be a considerable amount of overhead?
>>
>> What I mean is we'd better to unify the list into one place, now, I think
>> default place in super block is not bad. If we add one hardcoded list, we
>> will
>> hard to know which one is deleted by user.
>>
>> Before:
>> thin: .mp4, .mp3...
>>
>> User disable .mp4 extension
>> thin list: .mp3...
>> .mp4 will not be treated as cold file.
>>
>> After:
>> thin: .mp4, .mp3...
>> rich: .mp4, .mp3...
>>
>> User disable .mp4 extension
>> thin list: .mp3...
>> rich list: .mp4, .mp3...
>>
>> .mp4 will still be treated as cold file, since it is in rich list
>>
>> Although, we can tag .mp4 is non-cold file in thin list, but after that, the
>> code logic become more complicate, and layout is be more mess.
>>
>> Thanks,
>>
>>>
>>> Thanks.
>>>
>>> On Thu, Mar 29, 2018 at 1:04 PM, Chao Yu <yuch...@huawei.com> wrote:
>>>> Hi Park,
>>>>
>>>> On 2018/3/29 10:18, Ju Hyung Park wrote:
>>>>> Hi Chao.
>>>>>
>>>>>> Do you mean we need add a new option to enable common list?
>>>>>
>>>>> No. I meant to move the list to the kernelspace's fs/f2fs or
>>>>> include/linux/f2fs_fs.h.
>>>>
>>>> I think this is real hardcoded one, which will be hard to configure with
>>>> if user
>>>> don't want this list at all. How can you disable partial type in this
>>>> common
>>>> list? we have to use mount option or sysfs entry to do this at every mount
>>>> time...
>>>>
>>>>> This eliminates the concern of the list getting too big and exceeding
>>>>> the 64 limit,
>>>>> and we can modify the list upon shipping new versions of f2fs.
>>>>>
>>>>> As I complained multiple times before,
>>>>> I simply don't think that this list should be hardcoded at the time of
>>>>> mkfs.
>>>>>
>>>>> While users can still dynamically add/remove some extensions,
>>>>> they can still have an older version of the "common list",
>>>>> which then they have to use the sysfs interface everytime
>>>>> whenever we update this list on mkfs.f2fs.
>>>>
>>>> We can add an new mkfs option to disable old common list, then user can
>>>> enable/disable cold/hot file type as they prefer on mkfs/run-time.
>>>>
>>>>>
>>>>> I'm not proposing to remove the list altogether in mkfs.f2fs.
>>>>> I'm proposing to leave the old list in f2fs-tools, and write the new
>>>>> list to the kernelspace code.
>>>>> This way, older version of f2fs will still remain properly working.
>>>>
>>>> One question, how can you handle the conflict in between old list generated
>>>> during mkfs and new common list in run-time kernel?
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> I'd love to hear Jaegeuk's thoughts on this.
>>>>>
>>>>>> -o extlist=classic means we enable thin cold/hot file type list, mostly
>>>>>> we
>>>>>> recommended enable this in android system.
>>>>>
>>>>> I strongly disagree with this.
>>>>> Even more so with the case of Android,
>>>>> the users/OEMs won't typically add more entries.
>>>>>
>>>>> And the new list is very relevant on Android as well imo.
>>>>>
>>>>> Thanks.
>>>>>
>>>>> On Wed, Mar 28, 2018 at 11:40 AM, Chao Yu <yuch...@huawei.com> wrote:
>>>>>> Hi Park,
>>>>>>
>>>>>> On 2018/3/24 14:55, Ju Hyung Park wrote:
>>>>>>> Hi Chao,
>>>>>>>
>>>>>>> Sorry myself as well for the late reply :)
>>>>>>> Got caught up with something.
>>>>>>>
>>>>>>>> Like .so?
>>>>>>>
>>>>>>> Yeah, that makes sense.
>>>>>>> I've reran the command without the size limit.
>>>>>>>
>>>>>>> >From Ubuntu, I think we can add '*.pyc' files as it's compiled Python
>>>>>>> bytecode format.
>>>>>>> >From Android, I saw some new '*.cnt' files which are just jpeg files.
>>>>>>> Notably, Facebook, Skype, Tumblr and Twitter uses it.
>>>>>>>
>>>>>>> Everything else seemed not that much interesting.
>>>>>>>
>>>>>>>> I agree that we'd better support the superset list of common static
>>>>>>>> file, but
>>>>>>>> also I hope there is flexible usage of common list, old list and self
>>>>>>>> defined
>>>>>>>> list, so I think we'd better leave enough free space of cold list to
>>>>>>>> let user
>>>>>>>> define private cold file type extension as they wish, meanwhile
>>>>>>>> support an
>>>>>>>> option to make user have a chance to choose the common list or old
>>>>>>>> list.
>>>>>>>
>>>>>>> If I understood you correctly, you want to leave an option for the old
>>>>>>> list
>>>>>>> so that users have more room to add many more extensions, correct?
>>>>>>
>>>>>> Yup.
>>>>>>
>>>>>>>
>>>>>>> If so, how about just leaving the old list and move the new ones to
>>>>>>> the kernel code?
>>>>>>
>>>>>> Do you mean we need add a new option to enable common list? e.g.
>>>>>>
>>>>>> -o extlist=classic means we enable thin cold/hot file type list, mostly
>>>>>> we
>>>>>> recommended enable this in android system.
>>>>>> -o extlist=full means we enable full cold/hot file type list, it is
>>>>>> recommended
>>>>>> in other system, like server or pc.
>>>>>>
>>>>>> Like this?
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>> As I said in the previous comment, I don't think it makes sense to
>>>>>>> ship new lists only to at the time of mkfs.
>>>>>>> We can ship new lists as we update f2fs kernel code.
>>>>>>>
>>>>>>> While the new version of f2fs allows users to dynamically add or
>>>>>>> remove extensions,
>>>>>>> it doesn't ship the new common lists.
>>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>> On Wed, Mar 21, 2018 at 9:00 PM, Chao Yu <yuch...@huawei.com> wrote:
>>>>>>>> Hi Park,
>>>>>>>>
>>>>>>>> Sorry for late replying.
>>>>>>>>
>>>>>>>> On 2018/3/19 11:53, Ju Hyung Park wrote:
>>>>>>>>> Hi Chao,
>>>>>>>>>
>>>>>>>>>> Do you run this script in android environment to get the cold type?
>>>>>>>>> Yes, both on Ubuntu and Android(on /data with root permission).
>>>>>>>>>
>>>>>>>>>> Actually, I doubt that '+1M' condition can't indicate that the file
>>>>>>>>>> is cold or
>>>>>>>>>> not, and after run this script in my cell phone,
>>>>>>>>> Would it make sense to set a file that's < 1M as cold?
>>>>>>>>
>>>>>>>> Like .so?
>>>>>>>>
>>>>>>>>> I didn't think so. Please let me know if I'm wrong.
>>>>>>>>>
>>>>>>>>>> I didn't see so many type as your patch adds.
>>>>>>>>> Of course, most of those were added from vlc and p7zip.
>>>>>>>>> There are tons more, but I added ones that are most common.
>>>>>>>>> While I personally don't have that much many types myself as well,
>>>>>>>>> I can easily see one having those extensions stored under f2fs.
>>>>>>>>>
>>>>>>>>> Previous list was not enough, imo.
>>>>>>>>> (After running the command, I've added exo and ?dex files for
>>>>>>>>> Android.)
>>>>>>>>>
>>>>>>>>>> If that is a common cold file type list that user may not do random
>>>>>>>>>> updates in
>>>>>>>>>> the file after its creation,
>>>>>>>>> That's exactly what I intended.
>>>>>>>>>
>>>>>>>>>> I suggest that we can add one common list instead
>>>>>>>>>> of changing old one controlled by mkfs option
>>>>>>>>> The new list is superset of the old list.
>>>>>>>>> A few extensions were removed as those are mostly deprecated formats
>>>>>>>>> and to make room for much more important extensions to be added such
>>>>>>>>> as m4a.
>>>>>>>>
>>>>>>>> I agree that we'd better support the superset list of common static
>>>>>>>> file, but
>>>>>>>> also I hope there is flexible usage of common list, old list and self
>>>>>>>> defined
>>>>>>>> list, so I think we'd better leave enough free space of cold list to
>>>>>>>> let user
>>>>>>>> define private cold file type extension as they wish, meanwhile
>>>>>>>> support an
>>>>>>>> option to make user have a chance to choose the common list or old
>>>>>>>> list.
>>>>>>>>
>>>>>>>> How do you think?
>>>>>>>>
>>>>>>>> Hi Jaegeuk, what's your opinion?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Mon, Mar 19, 2018 at 12:42 PM, Chao Yu <yuch...@huawei.com> wrote:
>>>>>>>>>> Hi Park,
>>>>>>>>>>
>>>>>>>>>> On 2018/3/17 23:02, Park Ju Hyung wrote:
>>>>>>>>>>> Those formats are large in size and rarely updated.
>>>>>>>>>>>
>>>>>>>>>>> Formats such as tar and zip were intentionally excluded as
>>>>>>>>>>> those are capable of random updates.
>>>>>>>>>>>
>>>>>>>>>>> (Added from vlc, p7zip and running
>>>>>>>>>>> 'find . -type f -size +1M |
>>>>>>>>>>> while read FILE; do echo ${FILE##*.}; done |
>>>>>>>>>>> sort | uniq -c | sort -nr'
>>>>>>>>>>> manually)
>>>>>>>>>>
>>>>>>>>>> Do you run this script in android environment to get the cold type?
>>>>>>>>>>
>>>>>>>>>> Actually, I doubt that '+1M' condition can't indicate that the file
>>>>>>>>>> is cold or
>>>>>>>>>> not, and after run this script in my cell phone, I didn't see so
>>>>>>>>>> many type as
>>>>>>>>>> your patch adds.
>>>>>>>>>>
>>>>>>>>>> If that is a common cold file type list that user may not do random
>>>>>>>>>> updates in
>>>>>>>>>> the file after its creation, I suggest that we can add one common
>>>>>>>>>> list instead
>>>>>>>>>> of changing old one controlled by mkfs option, anyway, to use which
>>>>>>>>>> one, the
>>>>>>>>>> option can be decided by user.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Park Ju Hyung <qkrwngud...@gmail.com>
>>>>>>>>>>> ---
>>>>>>>>>>> mkfs/f2fs_format.c | 86
>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++++++--------------
>>>>>>>>>>> 1 file changed, 64 insertions(+), 22 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
>>>>>>>>>>> index 65692bb..3c7ce16 100644
>>>>>>>>>>> --- a/mkfs/f2fs_format.c
>>>>>>>>>>> +++ b/mkfs/f2fs_format.c
>>>>>>>>>>> @@ -37,34 +37,76 @@ struct f2fs_checkpoint *cp;
>>>>>>>>>>>
>>>>>>>>>>> static unsigned int quotatype_bits = 0;
>>>>>>>>>>>
>>>>>>>>>>> -const char *media_ext_lists[] = {
>>>>>>>>>>> - "jpg",
>>>>>>>>>>> - "gif",
>>>>>>>>>>> - "png",
>>>>>>>>>>> +const char *cold_ext_lists[] = {
>>>>>>>>>>> + /* video */
>>>>>>>>>>> "avi",
>>>>>>>>>>> "divx",
>>>>>>>>>>> - "mp4",
>>>>>>>>>>> - "mp3",
>>>>>>>>>>> - "3gp",
>>>>>>>>>>> - "wmv",
>>>>>>>>>>> - "wma",
>>>>>>>>>>> - "mpeg",
>>>>>>>>>>> + "flv",
>>>>>>>>>>> + "m2ts",
>>>>>>>>>>> + "m4p",
>>>>>>>>>>> + "m4v",
>>>>>>>>>>> "mkv",
>>>>>>>>>>> "mov",
>>>>>>>>>>> - "asx",
>>>>>>>>>>> - "asf",
>>>>>>>>>>> - "wmx",
>>>>>>>>>>> - "svi",
>>>>>>>>>>> - "wvx",
>>>>>>>>>>> - "wm",
>>>>>>>>>>> + "mp4",
>>>>>>>>>>> + "mpeg",
>>>>>>>>>>> + "mpeg4",
>>>>>>>>>>> "mpg",
>>>>>>>>>>> - "mpe",
>>>>>>>>>>> - "rm",
>>>>>>>>>>> "ogg",
>>>>>>>>>>> + "ogm",
>>>>>>>>>>> + "ogv",
>>>>>>>>>>> + "ts",
>>>>>>>>>>> + "vob",
>>>>>>>>>>> + "wmb",
>>>>>>>>>>> + "wmv",
>>>>>>>>>>> + "webm",
>>>>>>>>>>> +
>>>>>>>>>>> + /* audio */
>>>>>>>>>>> + "aac",
>>>>>>>>>>> + "ac3",
>>>>>>>>>>> + "dts",
>>>>>>>>>>> + "flac",
>>>>>>>>>>> + "m4a",
>>>>>>>>>>> + "mka",
>>>>>>>>>>> + "mp3",
>>>>>>>>>>> + "oga",
>>>>>>>>>>> + "wav",
>>>>>>>>>>> + "wma",
>>>>>>>>>>> +
>>>>>>>>>>> + /* image */
>>>>>>>>>>> + "bmp",
>>>>>>>>>>> + "gif",
>>>>>>>>>>> + "jpg",
>>>>>>>>>>> "jpeg",
>>>>>>>>>>> - "video",
>>>>>>>>>>> - "apk", /* for android system */
>>>>>>>>>>> - "so", /* for android system */
>>>>>>>>>>> + "png",
>>>>>>>>>>> + "svg",
>>>>>>>>>>> + "webp",
>>>>>>>>>>> +
>>>>>>>>>>> + /* archive */
>>>>>>>>>>> + "7z",
>>>>>>>>>>> + "a",
>>>>>>>>>>> + "deb",
>>>>>>>>>>> + "gz",
>>>>>>>>>>> + "gzip",
>>>>>>>>>>> + "iso",
>>>>>>>>>>> + "jar",
>>>>>>>>>>> + "lzma",
>>>>>>>>>>> + "rar",
>>>>>>>>>>> + "tgz",
>>>>>>>>>>> + "txz",
>>>>>>>>>>> + "udf",
>>>>>>>>>>> + "xz",
>>>>>>>>>>> +
>>>>>>>>>>> + /* other */
>>>>>>>>>>> + "pdf",
>>>>>>>>>>> + "ttf",
>>>>>>>>>>> + "ttc",
>>>>>>>>>>> +
>>>>>>>>>>> + /* android */
>>>>>>>>>>> + "apk",
>>>>>>>>>>> + "exo", // YouTube
>>>>>>>>>>> + "odex", // Android RunTime
>>>>>>>>>>> + "vdex", // Android RunTime
>>>>>>>>>>> + "so",
>>>>>>>>>>> NULL
>>>>>>>>>>> };
>>>>>>>>>>>
>>>>>>>>>>> @@ -74,7 +116,7 @@ const char *hot_ext_lists[] = {
>>>>>>>>>>> };
>>>>>>>>>>>
>>>>>>>>>>> const char **default_ext_list[] = {
>>>>>>>>>>> - media_ext_lists,
>>>>>>>>>>> + cold_ext_lists,
>>>>>>>>>>> hot_ext_lists
>>>>>>>>>>> };
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> .
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>
>>> .
>>>
>>
>
> .
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
