| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d99929c8-9bf2-4a99-8507-617eb3419b98@linux.alibaba.com>
Date: Mon, 22 Jan 2024 15:42:28 +0800
From: Gao Xiang <hsiangkao@...ux.alibaba.com>
To: Jingbo Xu <jefflexu@...ux.alibaba.com>, xiang@...nel.org,
chao@...nel.org, linux-erofs@...ts.ozlabs.org
Cc: huyue2@...lpad.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] erofs: make iov_iter describe target buffer when read
from fscache
Hi Jingbo,
On 2024/1/22 15:12, Jingbo Xu wrote:
> So far the fscache mode supports uncompressed data only, and the data
> read from fscache is put directly into the target page cache. As the
> support for compressed data in fscache mode is going to be introduced,
> refactor the interface of reading fscache so that the following
> compressed part could make the raw data read from fscache be directed to
> the target buffer it wants, decompress the raw data, and finally fill
> the page cache with the decompressed data.
>
> As the first step, a new structure, i.e. erofs_fscache_io (cio), is
I'd suggest just using io instead of cio here.
. i.e. erofs_fscache_io (io) ...
> introduced to describe a generic read request from the fscache, while
> the caller can specify the target buffer it wants in the iov_iter
> structure (cio->iter). Besides, the caller can also specify its
. structure (io->iter) ...
> completion callback and private data through cio, which will be called
> to make further handling, e.g. unlocking the page cache for uncompressed
> data or decompressing the read raw data, when the read request from the
> fscache completes. Now erofs_fscache_read_io_async() serves as a
> generic interface for reading raw data from fscache for both compressed
> and uncompressed data.
>
> The erofs_fscache_request structure is kept to describe a request to
> fill the page cache in the specified range.
>
> Signed-off-by: Jingbo Xu <jefflexu@...ux.alibaba.com>
> ---
> fs/erofs/fscache.c | 219 ++++++++++++++++++++++++---------------------
> 1 file changed, 118 insertions(+), 101 deletions(-)
>
> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
> index bc12030393b2..10709f20bef5 100644
> --- a/fs/erofs/fscache.c
> +++ b/fs/erofs/fscache.c
> @@ -13,8 +13,6 @@ static LIST_HEAD(erofs_domain_cookies_list);
> static struct vfsmount *erofs_pseudo_mnt;
>
> struct erofs_fscache_request {
> - struct erofs_fscache_request *primary;
> - struct netfs_cache_resources cache_resources;
> struct address_space *mapping; /* The mapping being accessed */
> loff_t start; /* Start position */
> size_t len; /* Length of the request */
> @@ -23,42 +21,13 @@ struct erofs_fscache_request {
> refcount_t ref;
> };
>
> -static struct erofs_fscache_request *erofs_fscache_req_alloc(struct address_space *mapping,
> - loff_t start, size_t len)
> -{
> - struct erofs_fscache_request *req;
> -
> - req = kzalloc(sizeof(struct erofs_fscache_request), GFP_KERNEL);
> - if (!req)
> - return ERR_PTR(-ENOMEM);
> -
> - req->mapping = mapping;
> - req->start = start;
> - req->len = len;
> - refcount_set(&req->ref, 1);
> -
> - return req;
> -}
> -
> -static struct erofs_fscache_request *erofs_fscache_req_chain(struct erofs_fscache_request *primary,
> - size_t len)
> -{
> - struct erofs_fscache_request *req;
> -
> - /* use primary request for the first submission */
> - if (!primary->submitted) {
> - refcount_inc(&primary->ref);
> - return primary;
> - }
> -
> - req = erofs_fscache_req_alloc(primary->mapping,
> - primary->start + primary->submitted, len);
> - if (!IS_ERR(req)) {
> - req->primary = primary;
> - refcount_inc(&primary->ref);
> - }
> - return req;
> -}
> +struct erofs_fscache_io {
> + struct netfs_cache_resources cache_resources;
struct netfs_cache_resources cres;
> + struct iov_iter iter;
> + netfs_io_terminated_t end_io;
> + void *private;
> + refcount_t ref;
> +};
>
> static void erofs_fscache_req_complete(struct erofs_fscache_request *req)
> {
> @@ -83,82 +52,116 @@ static void erofs_fscache_req_complete(struct erofs_fscache_request *req)
> static void erofs_fscache_req_put(struct erofs_fscache_request *req)
> {
> if (refcount_dec_and_test(&req->ref)) {
> - if (req->cache_resources.ops)
> - req->cache_resources.ops->end_operation(&req->cache_resources);
> - if (!req->primary)
> - erofs_fscache_req_complete(req);
> - else
> - erofs_fscache_req_put(req->primary);
> + erofs_fscache_req_complete(req);
> kfree(req);
> }
> }
>
> -static void erofs_fscache_subreq_complete(void *priv,
> +static struct erofs_fscache_request *erofs_fscache_req_alloc(struct address_space *mapping,
> + loff_t start, size_t len)
> +{
> + struct erofs_fscache_request *req;
> +
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> + if (req) {
> + req->mapping = mapping;
> + req->start = start;
> + req->len = len;
> + refcount_set(&req->ref, 1);
> + }
> + return req;
The following part may be better? to save an indentation:
req = kzalloc(sizeof(*req), GFP_KERNEL);
if (!req)
return NULL;
req->mapping = mapping;
req->start = start;
req->len = len;
refcount_set(&req->ref, 1);
return req;
> +}
> +
> +static bool erofs_fscache_io_put(struct erofs_fscache_io *cio)
> +{
> + if (refcount_dec_and_test(&cio->ref)) {
> + if (cio->cache_resources.ops)
> + cio->cache_resources.ops->end_operation(&cio->cache_resources);
> + kfree(cio);
> + return true;
> + }
> + return false;
if (!refcount_dec_and_test(&io->ref))
return false;
if (io->cres.ops)
io->cres.ops->end_operation(&io->cres);
kfree(io);
return true;
> +}
> +
> +static void erofs_fscache_req_io_put(struct erofs_fscache_io *cio)
cio -> io
> +{
> + struct erofs_fscache_request *req = cio->private;
> +
> + if (erofs_fscache_io_put(cio))
> + erofs_fscache_req_put(req);
> +}
> +
> +static void erofs_fscache_req_end_io(void *priv,
> ssize_t transferred_or_error, bool was_async)
> {
> - struct erofs_fscache_request *req = priv;
> + struct erofs_fscache_io *cio = priv;
> + struct erofs_fscache_request *req = cio->private;
> +
> + if (IS_ERR(transferred_or_error))
> + req->error = transferred_or_error;
> + erofs_fscache_req_io_put(cio);
> +}
> +
> +static struct erofs_fscache_io *erofs_fscache_req_io_alloc(struct erofs_fscache_request *req)
> +{
> + struct erofs_fscache_io *cio;
>
> - if (IS_ERR_VALUE(transferred_or_error)) {
> - if (req->primary)
> - req->primary->error = transferred_or_error;
> - else
> - req->error = transferred_or_error;
> + cio = kzalloc(sizeof(*cio), GFP_KERNEL);
> + if (cio) {
> + cio->end_io = erofs_fscache_req_end_io;
> + cio->private = req;
> + refcount_inc(&req->ref);
> + refcount_set(&cio->ref, 1);
> }
> - erofs_fscache_req_put(req);
> + return cio;
io = kzalloc(sizeof(*io), GFP_KERNEL);
if (!io)
return NULL;
io->end_io = erofs_fscache_req_end_io;
io->private = req;
refcount_inc(&req->ref);
refcount_set(&io->ref, 1);
return io;
Thanks,
Gao Xiang
Powered by blists - more mailing lists