[PATCH v16 0/9] Cache insensitive cleanup for ext4/f2fs

Eugen Hristev posted 9 patches 1 year, 8 months ago
fs/ext4/crypto.c   |  10 +---
fs/ext4/ext4.h     |  35 +++++++-----
fs/ext4/namei.c    | 129 ++++++++++++++++-----------------------------
fs/ext4/super.c    |   4 +-
fs/f2fs/dir.c      | 108 ++++++++++++-------------------------
fs/f2fs/f2fs.h     |  16 +++++-
fs/f2fs/namei.c    |  10 ++--
fs/f2fs/recovery.c |   9 +---
fs/f2fs/super.c    |   8 +--
fs/libfs.c         |  73 +++++++++++++++++++++++++
include/linux/fs.h |   4 ++
11 files changed, 206 insertions(+), 200 deletions(-)
[PATCH v16 0/9] Cache insensitive cleanup for ext4/f2fs
Posted by Eugen Hristev 1 year, 8 months ago
Hello,

I am trying to respin the series here :
https://www.spinics.net/lists/linux-ext4/msg85081.html

I resent some of the v9 patches and got some reviews from Gabriel,
I did changes as requested and here is v16.

Changes in v16:
- rewrote patch 2/9 without `match`
- changed to return value in generic_ci_match coming from utf8 compare only in
strict mode.
- changed f2fs_warn to *_ratelimited in 7/9
- removed the declaration of f2fs_cf_name_slab in recovery.c as it's no longer
needed.

Changes in v15:
- fix wrong check `ret<0` in 7/9
- fix memleak reintroduced in 8/9

Changes in v14:
- fix wrong kfree unchecked call
- changed the return code in 3/8

Changes in v13:
- removed stray wrong line in 2/8
- removed old R-b as it's too long since they were given
- removed check for null buff in 2/8
- added new patch `f2fs: Log error when lookup of encoded dentry fails` as suggested
- rebased on unicode.git for-next branch

Changes in v12:
- revert to v10 comparison with propagating the error code from utf comparison

Changes in v11:
- revert to the original v9 implementation for the comparison helper.

Changes in v10:
- reworked a bit the comparison helper to improve performance by
first performing the exact lookup.


* Original commit letter

The case-insensitive implementations in f2fs and ext4 have quite a bit
of duplicated code.  This series simplifies the ext4 version, with the
goal of extracting ext4_ci_compare into a helper library that can be
used by both filesystems.  It also reduces the clutter from many
codeguards for CONFIG_UNICODE; as requested by Linus, they are part of
the codeflow now.

While there, I noticed we can leverage the utf8 functions to detect
encoded names that are corrupted in the filesystem. Therefore, it also
adds an ext4 error on that scenario, to mark the filesystem as
corrupted.

This series survived passes of xfstests -g quick.

Eugen Hristev (1):
  f2fs: Log error when lookup of encoded dentry fails

Gabriel Krisman Bertazi (8):
  ext4: Simplify the handling of cached insensitive names
  f2fs: Simplify the handling of cached insensitive names
  libfs: Introduce case-insensitive string comparison helper
  ext4: Reuse generic_ci_match for ci comparisons
  f2fs: Reuse generic_ci_match for ci comparisons
  ext4: Log error when lookup of encoded dentry fails
  ext4: Move CONFIG_UNICODE defguards into the code flow
  f2fs: Move CONFIG_UNICODE defguards into the code flow

 fs/ext4/crypto.c   |  10 +---
 fs/ext4/ext4.h     |  35 +++++++-----
 fs/ext4/namei.c    | 129 ++++++++++++++++-----------------------------
 fs/ext4/super.c    |   4 +-
 fs/f2fs/dir.c      | 108 ++++++++++++-------------------------
 fs/f2fs/f2fs.h     |  16 +++++-
 fs/f2fs/namei.c    |  10 ++--
 fs/f2fs/recovery.c |   9 +---
 fs/f2fs/super.c    |   8 +--
 fs/libfs.c         |  73 +++++++++++++++++++++++++
 include/linux/fs.h |   4 ++
 11 files changed, 206 insertions(+), 200 deletions(-)

-- 
2.34.1
Re: [PATCH v16 0/9] Cache insensitive cleanup for ext4/f2fs
Posted by Matthew Wilcox 1 year, 8 months ago
On Fri, Apr 05, 2024 at 03:13:23PM +0300, Eugen Hristev wrote:
> Hello,
> 
> I am trying to respin the series here :
> https://www.spinics.net/lists/linux-ext4/msg85081.html

The subject here is "Cache insensitive cleanup for ext4/f2fs".
Cache insensitive means something entirely different
https://en.wikipedia.org/wiki/Cache-oblivious_algorithm

I suspect you mean "Case insensitive".
Re: [PATCH v16 0/9] Cache insensitive cleanup for ext4/f2fs
Posted by Eugen Hristev 1 year, 8 months ago
On 4/5/24 15:18, Matthew Wilcox wrote:
> On Fri, Apr 05, 2024 at 03:13:23PM +0300, Eugen Hristev wrote:
>> Hello,
>>
>> I am trying to respin the series here :
>> https://www.spinics.net/lists/linux-ext4/msg85081.html
> 
> The subject here is "Cache insensitive cleanup for ext4/f2fs".
> Cache insensitive means something entirely different
> https://en.wikipedia.org/wiki/Cache-oblivious_algorithm
> 
> I suspect you mean "Case insensitive".

You are correct, I apologize for the typo.

> _______________________________________________
> Kernel mailing list -- kernel@mailman.collabora.com
> To unsubscribe send an email to kernel-leave@mailman.collabora.com
> This list is managed by https://mailman.collabora.com
Re: [PATCH v16 0/9] Cache insensitive cleanup for ext4/f2fs
Posted by Gabriel Krisman Bertazi 1 year, 8 months ago
Eugen Hristev <eugen.hristev@collabora.com> writes:

> On 4/5/24 15:18, Matthew Wilcox wrote:
>> On Fri, Apr 05, 2024 at 03:13:23PM +0300, Eugen Hristev wrote:
>>> Hello,
>>>
>>> I am trying to respin the series here :
>>> https://www.spinics.net/lists/linux-ext4/msg85081.html
>> 
>> The subject here is "Cache insensitive cleanup for ext4/f2fs".
>> Cache insensitive means something entirely different
>> https://en.wikipedia.org/wiki/Cache-oblivious_algorithm
>> 
>> I suspect you mean "Case insensitive".
>
> You are correct, I apologize for the typo.

Heh. I completely missed it in the previous submissions. I guess we both
just mentally auto-corrected.

Since we are here, I think I contributed to the typo in the cover letter
with the summary lines of patch 1 and 2.  Differently from the rest of
the series, these two are actually working on a "cache of
casefolded strings".  But their summary lines are misleading.

Can you rename them to:

[PATCH v16 1/9] ext4: Simplify the handling of cached casefolded names
[PATCH v16 2/9] f2fs: Simplify the handling of cached casefolded names

From a quick look, the series is looking good and the strict mode issue
pointed in the last iteration seems fixed, though I didn't run it yet.
I'll take a closer look later today and fully review.

-- 
Gabriel Krisman Bertazi
Re: [PATCH v16 0/9] Cache insensitive cleanup for ext4/f2fs
Posted by Eugen Hristev 1 year, 7 months ago
Hello Krisman,

On 4/5/24 19:37, Gabriel Krisman Bertazi wrote:
> Eugen Hristev <eugen.hristev@collabora.com> writes:
> 
>> On 4/5/24 15:18, Matthew Wilcox wrote:
>>> On Fri, Apr 05, 2024 at 03:13:23PM +0300, Eugen Hristev wrote:
>>>> Hello,
>>>>
>>>> I am trying to respin the series here :
>>>> https://www.spinics.net/lists/linux-ext4/msg85081.html
>>>
>>> The subject here is "Cache insensitive cleanup for ext4/f2fs".
>>> Cache insensitive means something entirely different
>>> https://en.wikipedia.org/wiki/Cache-oblivious_algorithm
>>>
>>> I suspect you mean "Case insensitive".
>>
>> You are correct, I apologize for the typo.
> 
> Heh. I completely missed it in the previous submissions. I guess we both
> just mentally auto-corrected.
> 
> Since we are here, I think I contributed to the typo in the cover letter
> with the summary lines of patch 1 and 2.  Differently from the rest of
> the series, these two are actually working on a "cache of
> casefolded strings".  But their summary lines are misleading.
> 
> Can you rename them to:
> 
> [PATCH v16 1/9] ext4: Simplify the handling of cached casefolded names
> [PATCH v16 2/9] f2fs: Simplify the handling of cached casefolded names
> 
> From a quick look, the series is looking good and the strict mode issue
> pointed in the last iteration seems fixed, though I didn't run it yet.
> I'll take a closer look later today and fully review.
> 

Have you managed to take a look ? What would be the future of the series ? I didn't
want to send another version for just a subject change, but I can if that's the
only change required .

Thanks,
Eugen