lkml.org 
[lkml]   [2022]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v9 02/21] cachefiles: notify user daemon when looking up cookie
Date
Jeffle Xu <jefflexu@linux.alibaba.com> wrote:

> + help
> + This permits on-demand read mode of cachefiles. In this mode, when
> + cache miss, the cachefiles backend instead of netfs, is responsible
> + for fetching data, e.g. through user daemon.

How about:

help
This permits userspace to enable the cachefiles on-demand read mode.
In this mode, when a cache miss occurs, responsibility for fetching
the data lies with the cachefiles backend instead of with the netfs
and is delegated to userspace.

> + /*
> + * 1) Cache has been marked as dead state, and then 2) flush all
> + * pending requests in @reqs xarray. The barrier inside set_bit()
> + * will ensure that above two ops won't be reordered.
> + */

What set_bit()? What "above two ops"? And that's not how barriers work; they
provide a partial ordering relative to another pair of barriered ops.

Also, set_bit() can't be relied upon to imply a barrier - see
Documentation/memory-barriers.txt.

> + if (IS_ENABLED(CONFIG_CACHEFILES_ONDEMAND) &&
> + test_bit(CACHEFILES_ONDEMAND_MODE, &cache->flags)) {

It might be worth abstracting this into an inline function in internal.h:

static inline bool cachefiles_in_ondemand_mode(cache)
{
return IS_ENABLED(CONFIG_CACHEFILES_ONDEMAND) &&
test_bit(CACHEFILES_ONDEMAND_MODE, &cache->flags)
}

> +#ifdef CONFIG_CACHEFILES_ONDEMAND

This looks like it ought to be superfluous, given the preceding test - though
I can see why you need it:

> +#ifdef CONFIG_CACHEFILES_ONDEMAND
> + struct xarray reqs; /* xarray of pending on-demand requests */
> + struct xarray ondemand_ids; /* xarray for ondemand_id allocation */
> + u32 ondemand_id_next;
> +#endif

I'm tempted to say that you should just make them non-conditional. It's not
like there's likely to be more than one or two cachefiles_cache structs on a
system.

David

\
 
 \ /
  Last update: 2022-04-21 15:58    [W:0.235 / U:0.748 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site