2021-05-19 17:44:54

by Chao Yu

[permalink] [raw]
Subject: [PATCH 1/2] f2fs: compress: fix to disallow wildcard extension for hot/cold file

If all files are hot or cold, hot/cold separation is not needed anymore,
so let's disallow configure wildcard extension.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/f2fs.h | 1 +
fs/f2fs/namei.c | 12 ++++++++----
fs/f2fs/sysfs.c | 3 +++
3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index b753cc88c77e..ecb13c3b458d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3322,6 +3322,7 @@ void f2fs_handle_failed_inode(struct inode *inode);
/*
* namei.c
*/
+bool f2fs_is_wildcard_char(const char *ext);
int f2fs_update_extension_list(struct f2fs_sb_info *sbi, const char *name,
bool hot, bool set);
struct dentry *f2fs_get_parent(struct dentry *child);
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index a9cd9cf97229..8d78b96a8b3b 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -153,15 +153,17 @@ static struct inode *f2fs_new_inode(struct inode *dir, umode_t mode)
return ERR_PTR(err);
}

+bool f2fs_is_wildcard_char(const char *ext)
+{
+ return *ext == '*' && strlen(ext) == 1;
+}
+
static inline int is_extension_exist(const unsigned char *s, const char *sub)
{
size_t slen = strlen(s);
size_t sublen = strlen(sub);
int i;

- if (sublen == 1 && *sub == '*')
- return 1;
-
/*
* filename format of multimedia file should be defined as:
* "filename + '.' + extension + (optional: '.' + temp extension)".
@@ -306,9 +308,11 @@ static void set_compress_inode(struct f2fs_sb_info *sbi, struct inode *inode,
ext = F2FS_OPTION(sbi).extensions;

for (i = 0; i < ext_cnt; i++) {
+ if (f2fs_is_wildcard_char(ext[i]))
+ goto set_compress;
if (!is_extension_exist(name, ext[i]))
continue;
-
+set_compress:
set_compress_context(inode);
return;
}
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index dc71bc968c72..ff5acb4de1b6 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -343,6 +343,9 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
if (strlen(name) >= F2FS_EXTENSION_LEN)
return -EINVAL;

+ if (f2fs_is_wildcard_char(name))
+ return -EINVAL;
+
down_write(&sbi->sb_lock);

ret = f2fs_update_extension_list(sbi, name, hot, set);
--
2.29.2



2021-05-19 17:45:34

by Chao Yu

[permalink] [raw]
Subject: [PATCH 2/2] f2fs: compress: fix to disallow temp extension

This patch restricts to configure compress extension as format of:

[filename + '.' + extension]

rather than:

[filename + '.' + extension + (optional: '.' + temp extension)]

in order to avoid to enable compression incorrectly:

1. compress_extension=so
2. touch file.soa
3. touch file.so.tmp

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/namei.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 8d78b96a8b3b..36e04148e8f8 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -158,7 +158,8 @@ bool f2fs_is_wildcard_char(const char *ext)
return *ext == '*' && strlen(ext) == 1;
}

-static inline int is_extension_exist(const unsigned char *s, const char *sub)
+static inline int is_extension_exist(const unsigned char *s, const char *sub,
+ bool tmp_ext)
{
size_t slen = strlen(s);
size_t sublen = strlen(sub);
@@ -171,6 +172,13 @@ static inline int is_extension_exist(const unsigned char *s, const char *sub)
if (slen < sublen + 2)
return 0;

+ if (!tmp_ext) {
+ /* file has no temp extension */
+ if (s[slen - sublen - 1] != '.')
+ return 0;
+ return !strncasecmp(s + slen - sublen, sub, sublen);
+ }
+
for (i = 1; i < slen - sublen; i++) {
if (s[i] != '.')
continue;
@@ -196,7 +204,7 @@ static inline void set_file_temperature(struct f2fs_sb_info *sbi, struct inode *
hot_count = sbi->raw_super->hot_ext_count;

for (i = 0; i < cold_count + hot_count; i++) {
- if (is_extension_exist(name, extlist[i]))
+ if (is_extension_exist(name, extlist[i], true))
break;
}

@@ -297,7 +305,7 @@ static void set_compress_inode(struct f2fs_sb_info *sbi, struct inode *inode,
hot_count = sbi->raw_super->hot_ext_count;

for (i = cold_count; i < cold_count + hot_count; i++) {
- if (is_extension_exist(name, extlist[i])) {
+ if (is_extension_exist(name, extlist[i], false)) {
up_read(&sbi->sb_lock);
return;
}
@@ -310,7 +318,7 @@ static void set_compress_inode(struct f2fs_sb_info *sbi, struct inode *inode,
for (i = 0; i < ext_cnt; i++) {
if (f2fs_is_wildcard_char(ext[i]))
goto set_compress;
- if (!is_extension_exist(name, ext[i]))
+ if (!is_extension_exist(name, ext[i], false))
continue;
set_compress:
set_compress_context(inode);
--
2.29.2


2021-05-20 03:20:09

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH 1/2] f2fs: compress: fix to disallow wildcard extension for hot/cold file

On 2021/5/20 10:21, Jaegeuk Kim wrote:
> On 05/18, Chao Yu wrote:
>> If all files are hot or cold, hot/cold separation is not needed anymore,
>> so let's disallow configure wildcard extension.
>
> We need to be able to set hot/cold on the fly?

I got your concern, so do you prefer to keep "*" extension configuration
for hot/cold file?

Thanks,

>
>>
>> Fixes: 4c8ff7095bef ("f2fs: support data compression")
>> Signed-off-by: Chao Yu <[email protected]>
>> ---
>> fs/f2fs/f2fs.h | 1 +
>> fs/f2fs/namei.c | 12 ++++++++----
>> fs/f2fs/sysfs.c | 3 +++
>> 3 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index b753cc88c77e..ecb13c3b458d 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -3322,6 +3322,7 @@ void f2fs_handle_failed_inode(struct inode *inode);
>> /*
>> * namei.c
>> */
>> +bool f2fs_is_wildcard_char(const char *ext);
>> int f2fs_update_extension_list(struct f2fs_sb_info *sbi, const char *name,
>> bool hot, bool set);
>> struct dentry *f2fs_get_parent(struct dentry *child);
>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
>> index a9cd9cf97229..8d78b96a8b3b 100644
>> --- a/fs/f2fs/namei.c
>> +++ b/fs/f2fs/namei.c
>> @@ -153,15 +153,17 @@ static struct inode *f2fs_new_inode(struct inode *dir, umode_t mode)
>> return ERR_PTR(err);
>> }
>>
>> +bool f2fs_is_wildcard_char(const char *ext)
>> +{
>> + return *ext == '*' && strlen(ext) == 1;
>> +}
>> +
>> static inline int is_extension_exist(const unsigned char *s, const char *sub)
>> {
>> size_t slen = strlen(s);
>> size_t sublen = strlen(sub);
>> int i;
>>
>> - if (sublen == 1 && *sub == '*')
>> - return 1;
>> -
>> /*
>> * filename format of multimedia file should be defined as:
>> * "filename + '.' + extension + (optional: '.' + temp extension)".
>> @@ -306,9 +308,11 @@ static void set_compress_inode(struct f2fs_sb_info *sbi, struct inode *inode,
>> ext = F2FS_OPTION(sbi).extensions;
>>
>> for (i = 0; i < ext_cnt; i++) {
>> + if (f2fs_is_wildcard_char(ext[i]))
>> + goto set_compress;
>> if (!is_extension_exist(name, ext[i]))
>> continue;
>> -
>> +set_compress:
>> set_compress_context(inode);
>> return;
>> }
>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>> index dc71bc968c72..ff5acb4de1b6 100644
>> --- a/fs/f2fs/sysfs.c
>> +++ b/fs/f2fs/sysfs.c
>> @@ -343,6 +343,9 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
>> if (strlen(name) >= F2FS_EXTENSION_LEN)
>> return -EINVAL;
>>
>> + if (f2fs_is_wildcard_char(name))
>> + return -EINVAL;
>> +
>> down_write(&sbi->sb_lock);
>>
>> ret = f2fs_update_extension_list(sbi, name, hot, set);
>> --
>> 2.29.2
> .
>

2021-05-25 12:30:18

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH 1/2] f2fs: compress: fix to disallow wildcard extension for hot/cold file

On 05/20, Chao Yu wrote:
> On 2021/5/20 10:21, Jaegeuk Kim wrote:
> > On 05/18, Chao Yu wrote:
> > > If all files are hot or cold, hot/cold separation is not needed anymore,
> > > so let's disallow configure wildcard extension.
> >
> > We need to be able to set hot/cold on the fly?
>
> I got your concern, so do you prefer to keep "*" extension configuration
> for hot/cold file?

I'm fine with "*".

>
> Thanks,
>
> >
> > >
> > > Fixes: 4c8ff7095bef ("f2fs: support data compression")
> > > Signed-off-by: Chao Yu <[email protected]>
> > > ---
> > > fs/f2fs/f2fs.h | 1 +
> > > fs/f2fs/namei.c | 12 ++++++++----
> > > fs/f2fs/sysfs.c | 3 +++
> > > 3 files changed, 12 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index b753cc88c77e..ecb13c3b458d 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -3322,6 +3322,7 @@ void f2fs_handle_failed_inode(struct inode *inode);
> > > /*
> > > * namei.c
> > > */
> > > +bool f2fs_is_wildcard_char(const char *ext);
> > > int f2fs_update_extension_list(struct f2fs_sb_info *sbi, const char *name,
> > > bool hot, bool set);
> > > struct dentry *f2fs_get_parent(struct dentry *child);
> > > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> > > index a9cd9cf97229..8d78b96a8b3b 100644
> > > --- a/fs/f2fs/namei.c
> > > +++ b/fs/f2fs/namei.c
> > > @@ -153,15 +153,17 @@ static struct inode *f2fs_new_inode(struct inode *dir, umode_t mode)
> > > return ERR_PTR(err);
> > > }
> > > +bool f2fs_is_wildcard_char(const char *ext)
> > > +{
> > > + return *ext == '*' && strlen(ext) == 1;
> > > +}
> > > +
> > > static inline int is_extension_exist(const unsigned char *s, const char *sub)
> > > {
> > > size_t slen = strlen(s);
> > > size_t sublen = strlen(sub);
> > > int i;
> > > - if (sublen == 1 && *sub == '*')
> > > - return 1;
> > > -
> > > /*
> > > * filename format of multimedia file should be defined as:
> > > * "filename + '.' + extension + (optional: '.' + temp extension)".
> > > @@ -306,9 +308,11 @@ static void set_compress_inode(struct f2fs_sb_info *sbi, struct inode *inode,
> > > ext = F2FS_OPTION(sbi).extensions;
> > > for (i = 0; i < ext_cnt; i++) {
> > > + if (f2fs_is_wildcard_char(ext[i]))
> > > + goto set_compress;
> > > if (!is_extension_exist(name, ext[i]))
> > > continue;
> > > -
> > > +set_compress:
> > > set_compress_context(inode);
> > > return;
> > > }
> > > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> > > index dc71bc968c72..ff5acb4de1b6 100644
> > > --- a/fs/f2fs/sysfs.c
> > > +++ b/fs/f2fs/sysfs.c
> > > @@ -343,6 +343,9 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
> > > if (strlen(name) >= F2FS_EXTENSION_LEN)
> > > return -EINVAL;
> > > + if (f2fs_is_wildcard_char(name))
> > > + return -EINVAL;
> > > +
> > > down_write(&sbi->sb_lock);
> > > ret = f2fs_update_extension_list(sbi, name, hot, set);
> > > --
> > > 2.29.2
> > .
> >