Jeffle Xu <jefflexu@xxxxxxxxxxxxxxxxx> 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
--
Linux-cachefs mailing list
Linux-cachefs@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/linux-cachefs
