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

Reply via email to