Re: [f2fs-dev] [PATCH v3 09/12] f2fs: cache: use compress cache

[Daeho Jeong]

On Wed, Aug 26, 2026 at 8:24 PM Chao Yu <chao@xxxxxxxxxx> wrote:
>
> On 8/27/26 04:04, Daeho Jeong wrote:
> >> -static void f2fs_truncate_cache(struct f2fs_cached_block *entry,
> >> +void f2fs_truncate_cache(struct f2fs_cached_block *entry,
> >>                                         bool drop_dirty)
> >
> > -> __f2fs_truncate_cache() without locking?
> Or, I guess you mean we need a cleanup here?
>
> What about:
>
> void f2fs_truncate_cache(struct f2fs_cached_block *entry, bool drop_dirty)
> {
>         if (!entry->cache)
>                 return;
>         f2fs_do_truncate_cache(entry, drop_dirty);
> }
>
> static void f2fs_truncate_cache_locked(struct f2fs_cached_block *entry, bool drop_dirty)
> {
>         f2fs_lock_cache(entry);
>         f2fs_truncate_cache(entry, drop_dirty);
>         f2fs_unlock_cache(entry);
> }
>
> f2fs_truncate_cache() was exported for using in other .c file, in the meantime,
> static f2fs_truncate_cache_locked() can be used inside cache.c.

Splitting them into two helpers (one with locking, one without) makes
total sense.

However, in the Linux kernel convention, having `_locked` or `__`
usually indicates that the caller is expected to ALREADY hold the lock
(e.g. `__list_add()`, `__folio_mark_dirty()`).

If an exported function is named `f2fs_truncate_cache()` without `__`,
callers would naturally assume it handles locking internally.

So I'd suggest following the standard kernel pattern:

/* Caller must hold f2fs_lock_cache(entry) */
void __f2fs_truncate_cache(struct f2fs_cached_block *entry, bool drop_dirty)
{
if (!entry->cache)
return;
f2fs_do_truncate_cache(entry, drop_dirty);
}

/* Safe wrapper that acquires entry lock internally */
void f2fs_truncate_cache(struct f2fs_cached_block *entry, bool drop_dirty)
{
f2fs_lock_cache(entry);
__f2fs_truncate_cache(entry, drop_dirty);
f2fs_unlock_cache(entry);
}

Then compress.c (which already holds the lock to check entry->ino) can
call `__f2fs_truncate_cache()`, while other callers can safely use
`f2fs_truncate_cache()`.

What do you think?

Thanks,

>
> Thanks,