2023-03-30 16:51:57

by 李扬韬

Subject: [PATCH v2] f2fs: merge lz4hc_compress_pages() to lz4_compress_pages()

Remove unnecessary lz4hc_compress_pages().

Signed-off-by: Yangtao Li <[email protected]>
---
v2:
-rebase
fs/f2fs/compress.c | 24 ++++--------------------
1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 3182e1506252..f8b15c932c97 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -264,34 +264,18 @@ static void lz4_destroy_compress_ctx(struct compress_ctx *cc)
cc->private = NULL;
}

-#ifdef CONFIG_F2FS_FS_LZ4HC
-static int lz4hc_compress_pages(struct compress_ctx *cc)
+static int lz4_compress_pages(struct compress_ctx *cc)
{
- unsigned char level = F2FS_I(cc->inode)->i_compress_level;
int len;
+#ifdef CONFIG_F2FS_FS_LZ4HC
+ unsigned char level = F2FS_I(cc->inode)->i_compress_level;

if (level)
len = LZ4_compress_HC(cc->rbuf, cc->cbuf->cdata, cc->rlen,
cc->clen, level, cc->private);
else
- len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,
- cc->clen, cc->private);
- if (!len)
- return -EAGAIN;
-
- cc->clen = len;
- return 0;
-}
-#endif
-
-static int lz4_compress_pages(struct compress_ctx *cc)
-{
- int len;
-
-#ifdef CONFIG_F2FS_FS_LZ4HC
- return lz4hc_compress_pages(cc);
#endif
- len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,
+ len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,
cc->clen, cc->private);
if (!len)
return -EAGAIN;
--
2.35.1

2023-04-05 02:50:05

by Chao Yu

Subject: Re: [PATCH v2] f2fs: merge lz4hc_compress_pages() to lz4_compress_pages()

On 2023/3/31 0:49, Yangtao Li wrote:
> +#ifdef CONFIG_F2FS_FS_LZ4HC
> + unsigned char level = F2FS_I(cc->inode)->i_compress_level;
>
> if (level)
> len = LZ4_compress_HC(cc->rbuf, cc->cbuf->cdata, cc->rlen,
> cc->clen, level, cc->private);
> else

[snip]

> #endif

[snip]

> + len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,

It's weired that whole else xxx; statement is split by #endif.

What about?

unsigned char level = 0;

#if
...
#endif
if (!level)
len = LZ4_compress_default()

Thanks,

> cc->clen, cc->private);
> if (!len)
> return -EAGAIN;

2023-04-05 16:14:54

by Jaegeuk Kim

Subject: Re: [PATCH v2] f2fs: merge lz4hc_compress_pages() to lz4_compress_pages()

On 04/05, Chao Yu wrote:
> On 2023/3/31 0:49, Yangtao Li wrote:
> > +#ifdef CONFIG_F2FS_FS_LZ4HC
> > + unsigned char level = F2FS_I(cc->inode)->i_compress_level;
> > if (level)
> > len = LZ4_compress_HC(cc->rbuf, cc->cbuf->cdata, cc->rlen,
> > cc->clen, level, cc->private);
> > else
>
> [snip]
>
> > #endif
>
> [snip]
>
> > + len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,
>
> It's weired that whole else xxx; statement is split by #endif.
>
> What about?
>
> unsigned char level = 0;
>
> #if
> ...
> #endif
> if (!level)
> len = LZ4_compress_default()

I modified like this.

--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -266,17 +266,19 @@ static void lz4_destroy_compress_ctx(struct compress_ctx *cc)

static int lz4_compress_pages(struct compress_ctx *cc)
{
- int len;
-#ifdef CONFIG_F2FS_FS_LZ4HC
+ int len = -EINVAL;
unsigned char level = F2FS_I(cc->inode)->i_compress_level;

- if (level)
+ if (!level)
+ len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,
+ cc->clen, cc->private);
+#ifdef CONFIG_F2FS_FS_LZ4HC
+ else
len = LZ4_compress_HC(cc->rbuf, cc->cbuf->cdata, cc->rlen,
cc->clen, level, cc->private);
- else
#endif
- len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,
- cc->clen, cc->private);
+ if (len < 0)
+ return len;
if (!len)
return -EAGAIN;

>
> Thanks,
>
> > cc->clen, cc->private);
> > if (!len)
> > return -EAGAIN;

2023-04-07 01:35:26

by Chao Yu

Subject: Re: [PATCH v2] f2fs: merge lz4hc_compress_pages() to lz4_compress_pages()

On 2023/4/6 0:13, Jaegeuk Kim wrote:
> On 04/05, Chao Yu wrote:
>> On 2023/3/31 0:49, Yangtao Li wrote:
>>> +#ifdef CONFIG_F2FS_FS_LZ4HC
>>> + unsigned char level = F2FS_I(cc->inode)->i_compress_level;
>>> if (level)
>>> len = LZ4_compress_HC(cc->rbuf, cc->cbuf->cdata, cc->rlen,
>>> cc->clen, level, cc->private);
>>> else
>>
>> [snip]
>>
>>> #endif
>>
>> [snip]
>>
>>> + len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,
>>
>> It's weired that whole else xxx; statement is split by #endif.
>>
>> What about?
>>
>> unsigned char level = 0;
>>
>> #if
>> ...
>> #endif
>> if (!level)
>> len = LZ4_compress_default()
>
> I modified like this.
>
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -266,17 +266,19 @@ static void lz4_destroy_compress_ctx(struct compress_ctx *cc)
>
> static int lz4_compress_pages(struct compress_ctx *cc)
> {
> - int len;
> -#ifdef CONFIG_F2FS_FS_LZ4HC
> + int len = -EINVAL;
> unsigned char level = F2FS_I(cc->inode)->i_compress_level;
>
> - if (level)
> + if (!level)
> + len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,
> + cc->clen, cc->private);
> +#ifdef CONFIG_F2FS_FS_LZ4HC
> + else
> len = LZ4_compress_HC(cc->rbuf, cc->cbuf->cdata, cc->rlen,
> cc->clen, level, cc->private);
> - else
> #endif
> - len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,
> - cc->clen, cc->private);
> + if (len < 0)
> + return len;
> if (!len)
> return -EAGAIN;
>

Better.

Reviewed-by: Chao Yu <[email protected]>

Thanks,