From 7be5be431ca0e54c9015b70a4d290fec5f7795a5 Mon Sep 17 00:00:00 2001 From: Jingbo Xu Date: Fri, 18 Nov 2022 14:25:10 +0800 Subject: [PATCH 1/6] anolis: cachefiles: replace BUG_ON() with WARN_ON() ANBZ: #3210 BUG_ON() is extremely user unfriendly. Keep moving forward if things wern't so bad. Fixes: 8fc28945e193 ("cachefiles: notify the user daemon when looking up cookie") Fixes: e0f54fb64c0a ("cachefiles: implement on-demand read") Signed-off-by: Jingbo Xu --- fs/cachefiles/daemon.c | 3 ++- fs/cachefiles/ondemand.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c index c34e53a8cada..86024089e551 100644 --- a/fs/cachefiles/daemon.c +++ b/fs/cachefiles/daemon.c @@ -160,7 +160,8 @@ static void cachefiles_flush_reqs(struct cachefiles_cache *cache) radix_tree_for_each_slot(slot, &cache->reqs, &iter, 0) { req = radix_tree_deref_slot_protected(slot, &cache->reqs.xa_lock); - BUG_ON(!req); + if (WARN_ON(!req)) + continue; radix_tree_delete(&cache->reqs, iter.index); req->error = -EIO; complete(&req->done); diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c index a7c3e5a6e0b9..7056fc5da8a2 100644 --- a/fs/cachefiles/ondemand.c +++ b/fs/cachefiles/ondemand.c @@ -27,8 +27,8 @@ static int cachefiles_ondemand_fd_release(struct inode *inode, radix_tree_for_each_slot(slot, &cache->reqs, &iter, 0) { req = radix_tree_deref_slot_protected(slot, &cache->reqs.xa_lock); - BUG_ON(!req); - + if (WARN_ON(!req)) + continue; if (req->msg.object_id == object_id && req->msg.opcode == CACHEFILES_OP_READ) { req->error = -EIO; -- Gitee From 83df48db240d2b379467f07bc3f2939fe299ff15 Mon Sep 17 00:00:00 2001 From: Jingbo Xu Date: Fri, 4 Nov 2022 17:00:32 +0800 Subject: [PATCH 2/6] anolis: cachefiles: fix volume key setup for cachefiles_open ANBZ: #3210 Prior to fscache refactoring, the volume key in fscache_volume is a string with trailing NUL; while after refactoring, the volume key in fscache_cookie is actually a string without trailing NUL. Thus the current volume key setup for cachefiles_open may cause oops since it attempts to access volume key from the bad address. This can be reproduced by specifying "fsid" larger than or equal to 16 characters, e.g. "-o fsid=bigdomain". Fix this by determining if volume key is stored in volume->key or volume->inline_key by checking volume->key_len, rather than volume_key_size (which is actually volume->key_len plus 1). Reported-by: Jia Zhu Fixes: 8fc28945e193 ("cachefiles: notify the user daemon when looking up cookie") Signed-off-by: Jingbo Xu --- fs/cachefiles/ondemand.c | 14 ++++++++++---- include/uapi/linux/cachefiles.h | 5 +++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c index 7056fc5da8a2..9b48cf16c10b 100644 --- a/fs/cachefiles/ondemand.c +++ b/fs/cachefiles/ondemand.c @@ -391,21 +391,27 @@ static int cachefiles_ondemand_init_open_req(struct cachefiles_req *req, { struct cachefiles_object *object = req->object; struct fscache_cookie *cookie = object->fscache.cookie; - struct fscache_cookie *volume; + struct fscache_cookie *volume = object->fscache.parent->cookie; struct cachefiles_open *load = (void *)req->msg.data; size_t volume_key_size, cookie_key_size; char *cookie_key, *volume_key; - /* Cookie key is binary data, which is netfs specific. */ + /* + * cookie_key is a string without trailing '\0', while cachefiles_open + * expects cookie key a string without trailing '\0'. + */ cookie_key_size = cookie->key_len; if (cookie->key_len <= sizeof(cookie->inline_key)) cookie_key = cookie->inline_key; else cookie_key = cookie->key; - volume = object->fscache.parent->cookie; + /* + * volume_key is a string without trailing '\0', while cachefiles_open + * expects volume key a string with trailing '\0'. + */ volume_key_size = volume->key_len + 1; - if (volume_key_size <= sizeof(cookie->inline_key)) + if (volume->key_len <= sizeof(volume->inline_key)) volume_key = volume->inline_key; else volume_key = volume->key; diff --git a/include/uapi/linux/cachefiles.h b/include/uapi/linux/cachefiles.h index 78caa73e5343..b6746a2fe57c 100644 --- a/include/uapi/linux/cachefiles.h +++ b/include/uapi/linux/cachefiles.h @@ -37,8 +37,9 @@ struct cachefiles_msg { /* * @data contains the volume_key followed directly by the cookie_key. volume_key * is a NUL-terminated string; @volume_key_size indicates the size of the volume - * key in bytes. cookie_key is binary data, which is netfs specific; - * @cookie_key_size indicates the size of the cookie key in bytes. + * key in bytes (with trailing NUL). cookie_key is a string without trailing + * NUL; @cookie_key_size indicates the size of the cookie key in bytes (without + * trailing NUL). * * @fd identifies an anon_fd referring to the cache file. */ -- Gitee From 91decd94eec8dce7fc05d9f04806c441dfaf7ddf Mon Sep 17 00:00:00 2001 From: Jingbo Xu Date: Thu, 17 Nov 2022 16:49:01 +0800 Subject: [PATCH 3/6] anolis: erofs: update page for .readpages() in fscache mode ANBZ: #3210 Update page to be handled for unmapped and meta mapped condition. Reported-by: Jia Zhu Fixes: f167f3665c5a ("anolis: erofs: implement fscache-based data readahead") Signed-off-by: Jingbo Xu --- fs/erofs/fscache.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c index dcb4e1e22479..1357adef75aa 100644 --- a/fs/erofs/fscache.c +++ b/fs/erofs/fscache.c @@ -214,6 +214,7 @@ static int erofs_fscache_readpages(struct file *filp, struct address_space *mapp return ret; if (!(map.m_flags & EROFS_MAP_MAPPED)) { + page = lru_to_page(pages); list_del(&page->lru); ret = add_to_page_cache_lru(page, mapping, page->index, readahead_gfp_mask(mapping)); @@ -230,6 +231,7 @@ static int erofs_fscache_readpages(struct file *filp, struct address_space *mapp } if (map.m_flags & EROFS_MAP_META) { + page = lru_to_page(pages); list_del(&page->lru); ret = add_to_page_cache_lru(page, mapping, page->index, readahead_gfp_mask(mapping)); -- Gitee From 921c4a2b1d46bfd87d868bb4e572e2b5036bd1ad Mon Sep 17 00:00:00 2001 From: Sun Ke Date: Fri, 26 Aug 2022 10:35:15 +0800 Subject: [PATCH 4/6] cachefiles: fix error return code in cachefiles_ondemand_copen() ANBZ: #3210 commit c93ccd63b18c8d108c57b2bb0e5f3b058b9d2029 upstream. The cache_size field of copen is specified by the user daemon. If cache_size < 0, then the OPEN request is expected to fail, while copen itself shall succeed. However, returning 0 is indeed unexpected when cache_size is an invalid error code. Fix this by returning error when cache_size is an invalid error code. Changes ======= v4: update the code suggested by Dan v3: update the commit log suggested by Jingbo. Fixes: c8383054506c ("cachefiles: notify the user daemon when looking up cookie") Signed-off-by: Sun Ke Suggested-by: Jeffle Xu Suggested-by: Dan Carpenter Signed-off-by: David Howells Reviewed-by: Gao Xiang Reviewed-by: Jingbo Xu Reviewed-by: Dan Carpenter Link: https://lore.kernel.org/r/20220818111935.1683062-1-sunke32@huawei.com/ # v2 Link: https://lore.kernel.org/r/20220818125038.2247720-1-sunke32@huawei.com/ # v3 Link: https://lore.kernel.org/r/20220826023515.3437469-1-sunke32@huawei.com/ # v4 Signed-off-by: Jingbo Xu --- fs/cachefiles/ondemand.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c index 9b48cf16c10b..7eab24367944 100644 --- a/fs/cachefiles/ondemand.c +++ b/fs/cachefiles/ondemand.c @@ -150,9 +150,13 @@ int cachefiles_ondemand_copen(struct cachefiles_cache *cache, char *args) /* fail OPEN request if daemon reports an error */ if (size < 0) { - if (!IS_ERR_VALUE(size)) - size = -EINVAL; - req->error = size; + if (!IS_ERR_VALUE(size)) { + req->error = -EINVAL; + ret = -EINVAL; + } else { + req->error = size; + ret = 0; + } goto out; } -- Gitee From 3378f24ab7e72711513c469f8c91bbe255e4f15d Mon Sep 17 00:00:00 2001 From: Jingbo Xu Date: Thu, 17 Nov 2022 17:33:39 +0800 Subject: [PATCH 5/6] anolis: cachefiles: refactor cachefiles_ondemand_daemon_read() ANBZ: #3210 Refactor the code arrangement of cachefiles_ondemand_daemon_read() a bit to make it more consistent with that of upstream. This is in prep for the following fix which makes on-demand request distribution fairer. Signed-off-by: Jingbo Xu --- fs/cachefiles/ondemand.c | 68 +++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c index 7eab24367944..430e7c2fa17c 100644 --- a/fs/cachefiles/ondemand.c +++ b/fs/cachefiles/ondemand.c @@ -237,7 +237,7 @@ static int cachefiles_ondemand_get_fd(struct cachefiles_req *req) ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, char __user *_buffer, size_t buflen, loff_t *pos) { - struct cachefiles_req *req; + struct cachefiles_req *req = NULL; struct cachefiles_msg *msg; unsigned long id = 0; size_t n; @@ -252,46 +252,50 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, xa_lock(&cache->reqs); radix_tree_for_each_tagged(slot, &cache->reqs, &iter, 0, CACHEFILES_REQ_NEW) { - req = radix_tree_deref_slot_protected(slot, - &cache->reqs.xa_lock); + req = radix_tree_deref_slot_protected(slot, &cache->reqs.xa_lock); + WARN_ON(!req); + break; + } - msg = &req->msg; - n = msg->len; + /* no request tagged with CACHEFILES_REQ_NEW found */ + if (!req) { + xa_unlock(&cache->reqs); + return 0; + } - if (n > buflen) { - xa_unlock(&cache->reqs); - return -EMSGSIZE; - } + msg = &req->msg; + n = msg->len; - radix_tree_iter_tag_clear(&cache->reqs, &iter, - CACHEFILES_REQ_NEW); + if (n > buflen) { xa_unlock(&cache->reqs); + return -EMSGSIZE; + } - id = iter.index; - msg->msg_id = id; + radix_tree_iter_tag_clear(&cache->reqs, &iter, CACHEFILES_REQ_NEW); + xa_unlock(&cache->reqs); - if (msg->opcode == CACHEFILES_OP_OPEN) { - ret = cachefiles_ondemand_get_fd(req); - if (ret) - goto error; - } + id = iter.index; + msg->msg_id = id; - if (copy_to_user(_buffer, msg, n) != 0) { - ret = -EFAULT; - goto err_put_fd; - } + if (msg->opcode == CACHEFILES_OP_OPEN) { + ret = cachefiles_ondemand_get_fd(req); + if (ret) + goto error; + } - /* CLOSE request has no reply */ - if (msg->opcode == CACHEFILES_OP_CLOSE) { - xa_lock(&cache->reqs); - radix_tree_delete(&cache->reqs, id); - xa_unlock(&cache->reqs); - complete(&req->done); - } - return n; + if (copy_to_user(_buffer, msg, n) != 0) { + ret = -EFAULT; + goto err_put_fd; } - xa_unlock(&cache->reqs); - return 0; + + /* CLOSE request has no reply */ + if (msg->opcode == CACHEFILES_OP_CLOSE) { + xa_lock(&cache->reqs); + radix_tree_delete(&cache->reqs, id); + xa_unlock(&cache->reqs); + complete(&req->done); + } + return n; err_put_fd: if (msg->opcode == CACHEFILES_OP_OPEN) -- Gitee From 26d8efff8ed199fe85a9441564c8a6621b094ccd Mon Sep 17 00:00:00 2001 From: Xin Yin Date: Thu, 25 Aug 2022 10:09:45 +0800 Subject: [PATCH 6/6] cachefiles: make on-demand request distribution fairer ANBZ: #3210 commit 1122f40072731525c06b1371cfa30112b9b54d27 upstream. For now, enqueuing and dequeuing on-demand requests all start from idx 0, this makes request distribution unfair. In the weighty concurrent I/O scenario, the request stored in higher idx will starve. Searching requests cyclically in cachefiles_ondemand_daemon_read, makes distribution fairer. Fixes: c8383054506c ("cachefiles: notify the user daemon when looking up cookie") Reported-by: Yongqing Li Signed-off-by: Xin Yin Signed-off-by: David Howells Reviewed-by: Jeffle Xu Reviewed-by: Gao Xiang Link: https://lore.kernel.org/r/20220817065200.11543-1-yinxin.x@bytedance.com/ # v1 Link: https://lore.kernel.org/r/20220825020945.2293-1-yinxin.x@bytedance.com/ # v2 --- fs/cachefiles/internal.h | 1 + fs/cachefiles/ondemand.c | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h index 1f486df1d7e1..b6633b2a6541 100644 --- a/fs/cachefiles/internal.h +++ b/fs/cachefiles/internal.h @@ -102,6 +102,7 @@ struct cachefiles_cache { char *tag; /* cache binding tag */ refcount_t unbind_pincount;/* refcount to do daemon unbind */ struct radix_tree_root reqs; /* xarray of pending on-demand requests */ + unsigned long req_id_next; struct idr ondemand_ids; /* xarray for ondemand_id allocation */ u32 ondemand_id_next; }; diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c index 430e7c2fa17c..250b98e9820c 100644 --- a/fs/cachefiles/ondemand.c +++ b/fs/cachefiles/ondemand.c @@ -246,17 +246,29 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, void **slot; /* - * Search for a request that has not ever been processed, to prevent - * requests from being processed repeatedly. + * Cyclically search for a request that has not ever been processed, + * to prevent requests from being processed repeatedly, and make + * request distribution fair. */ xa_lock(&cache->reqs); - radix_tree_for_each_tagged(slot, &cache->reqs, &iter, 0, + radix_tree_for_each_tagged(slot, &cache->reqs, &iter, cache->req_id_next, CACHEFILES_REQ_NEW) { req = radix_tree_deref_slot_protected(slot, &cache->reqs.xa_lock); WARN_ON(!req); break; } + if (!req && cache->req_id_next > 0) { + radix_tree_for_each_tagged(slot, &cache->reqs, &iter, 0, + CACHEFILES_REQ_NEW) { + if (iter.index >= cache->req_id_next) + break; + req = radix_tree_deref_slot_protected(slot, &cache->reqs.xa_lock); + WARN_ON(!req); + break; + } + } + /* no request tagged with CACHEFILES_REQ_NEW found */ if (!req) { xa_unlock(&cache->reqs); @@ -272,6 +284,7 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, } radix_tree_iter_tag_clear(&cache->reqs, &iter, CACHEFILES_REQ_NEW); + cache->req_id_next = iter.index + 1; xa_unlock(&cache->reqs); id = iter.index; -- Gitee