On Tue, 2022-01-18 at 13:53 +0000, David Howells wrote:
> Cachefiles keeps track of how much space is available on the backing
> filesystem and refuses new writes permission to start if there isn't enough
> (we especially don't want ENOSPC happening). It also tracks the amount of
> data pending in DIO writes (cache->b_writing) and reduces the amount of
> free space available by this amount before deciding if it can set up a new
> write.
>
> However, the old fscache I/O API was very much page-granularity dependent
> and, as such, cachefiles's cache->bshift was meant to be a multiplier to
> get from PAGE_SIZE to block size (ie. a blocksize of 512 would give a shift
> of 3 for a 4KiB page) - and this was incorrectly being used to turn the
> number of bytes in a DIO write into a number of blocks, leading to a
> massive over estimation of the amount of data in flight.
>
> Fix this by changing cache->bshift to be a multiplier from bytes to
> blocksize and deal with quantities of blocks, not quantities of pages.
>
> Fix also the rounding in the calculation in cachefiles_write() which needs
> a "- 1" inserting.
>
> Fixes: 047487c947e8 ("cachefiles: Implement the I/O routines")
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> cc: linux-cachefs@xxxxxxxxxx
> ---
>
> fs/cachefiles/cache.c | 7 ++-----
> fs/cachefiles/internal.h | 2 +-
> fs/cachefiles/io.c | 2 +-
> 3 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/fs/cachefiles/cache.c b/fs/cachefiles/cache.c
> index ce4d4785003c..1e9c71666c6a 100644
> --- a/fs/cachefiles/cache.c
> +++ b/fs/cachefiles/cache.c
> @@ -84,9 +84,7 @@ int cachefiles_add_cache(struct cachefiles_cache *cache)
> goto error_unsupported;
>
> cache->bsize = stats.f_bsize;
> - cache->bshift = 0;
> - if (stats.f_bsize < PAGE_SIZE)
> - cache->bshift = PAGE_SHIFT - ilog2(stats.f_bsize);
> + cache->bshift = ilog2(stats.f_bsize);
>
> _debug("blksize %u (shift %u)",
> cache->bsize, cache->bshift);
> @@ -106,7 +104,6 @@ int cachefiles_add_cache(struct cachefiles_cache *cache)
> (unsigned long long) cache->fcull,
> (unsigned long long) cache->fstop);
>
> - stats.f_blocks >>= cache->bshift;
> do_div(stats.f_blocks, 100);
> cache->bstop = stats.f_blocks * cache->bstop_percent;
> cache->bcull = stats.f_blocks * cache->bcull_percent;
> @@ -209,7 +206,7 @@ int cachefiles_has_space(struct cachefiles_cache *cache,
> return ret;
> }
>
> - b_avail = stats.f_bavail >> cache->bshift;
> + b_avail = stats.f_bavail;
> b_writing = atomic_long_read(&cache->b_writing);
> if (b_avail > b_writing)
> b_avail -= b_writing;
> diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h
> index 8dd54d9375b6..c793d33b0224 100644
> --- a/fs/cachefiles/internal.h
> +++ b/fs/cachefiles/internal.h
> @@ -86,7 +86,7 @@ struct cachefiles_cache {
> unsigned bcull_percent; /* when to start culling (% blocks) */
> unsigned bstop_percent; /* when to stop allocating (% blocks) */
> unsigned bsize; /* cache's block size */
> - unsigned bshift; /* min(ilog2(PAGE_SIZE / bsize), 0) */
> + unsigned bshift; /* ilog2(bsize) */
> uint64_t frun; /* when to stop culling */
> uint64_t fcull; /* when to start culling */
> uint64_t fstop; /* when to stop allocating */
> diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c
> index 60b1eac2ce78..04eb52736990 100644
> --- a/fs/cachefiles/io.c
> +++ b/fs/cachefiles/io.c
> @@ -264,7 +264,7 @@ static int cachefiles_write(struct netfs_cache_resources *cres,
> ki->term_func = term_func;
> ki->term_func_priv = term_func_priv;
> ki->was_async = true;
> - ki->b_writing = (len + (1 << cache->bshift)) >> cache->bshift;
> + ki->b_writing = (len + (1 << cache->bshift) - 1) >> cache->bshift;
>
> if (ki->term_func)
> ki->iocb.ki_complete = cachefiles_write_complete;
>
>
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
