On 5/22/24 7:43 PM, libao...@huaweicloud.com wrote:
> From: Baokun Li <libaok...@huawei.com>
>
> The err_put_fd label is only used once, so remove it to make the code
> more readable. In addition, the logic for deleting error request and
> CLOSE request is merged to simplify the code.
>
> Signed-off-by: Baokun Li <libaok...@huawei.com>
> Acked-by: Jeff Layton <jlay...@kernel.org>
> Reviewed-by: Jia Zhu <zhujia...@bytedance.com>
> Reviewed-by: Gao Xiang <hsiang...@linux.alibaba.com>
LGTM.
Reviewed-by: Jingbo Xu <jeffl...@linux.alibaba.com>
> ---
> fs/cachefiles/ondemand.c | 45 ++++++++++++++--------------------------
> 1 file changed, 16 insertions(+), 29 deletions(-)
>
> diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c
> index 3dd002108a87..bb94ef6a6f61 100644
> --- a/fs/cachefiles/ondemand.c
> +++ b/fs/cachefiles/ondemand.c
> @@ -305,7 +305,6 @@ ssize_t cachefiles_ondemand_daemon_read(struct
> cachefiles_cache *cache,
> {
> struct cachefiles_req *req;
> struct cachefiles_msg *msg;
> - unsigned long id = 0;
> size_t n;
> int ret = 0;
> XA_STATE(xas, &cache->reqs, cache->req_id_next);
> @@ -340,49 +339,37 @@ ssize_t cachefiles_ondemand_daemon_read(struct
> cachefiles_cache *cache,
> cachefiles_grab_object(req->object, cachefiles_obj_get_read_req);
> xa_unlock(&cache->reqs);
>
> - id = xas.xa_index;
> -
> if (msg->opcode == CACHEFILES_OP_OPEN) {
> ret = cachefiles_ondemand_get_fd(req);
> if (ret) {
> cachefiles_ondemand_set_object_close(req->object);
> - goto error;
> + goto out;
> }
> }
>
> - msg->msg_id = id;
> + msg->msg_id = xas.xa_index;
> msg->object_id = req->object->ondemand->ondemand_id;
>
> if (copy_to_user(_buffer, msg, n) != 0) {
> ret = -EFAULT;
> - goto err_put_fd;
> - }
> -
> - cachefiles_put_object(req->object, cachefiles_obj_put_read_req);
> - /* CLOSE request has no reply */
> - if (msg->opcode == CACHEFILES_OP_CLOSE) {
> - xa_erase(&cache->reqs, id);
> - complete(&req->done);
> + if (msg->opcode == CACHEFILES_OP_OPEN)
> + close_fd(((struct cachefiles_open *)msg->data)->fd);
> }
> -
> - cachefiles_req_put(req);
> - return n;
> -
> -err_put_fd:
> - if (msg->opcode == CACHEFILES_OP_OPEN)
> - close_fd(((struct cachefiles_open *)msg->data)->fd);
> -error:
> +out:
> cachefiles_put_object(req->object, cachefiles_obj_put_read_req);
> - xas_reset(&xas);
> - xas_lock(&xas);
> - if (xas_load(&xas) == req) {
> - req->error = ret;
> - complete(&req->done);
> - xas_store(&xas, NULL);
> + /* Remove error request and CLOSE request has no reply */
> + if (ret || msg->opcode == CACHEFILES_OP_CLOSE) {
> + xas_reset(&xas);
> + xas_lock(&xas);
> + if (xas_load(&xas) == req) {
> + req->error = ret;
> + complete(&req->done);
> + xas_store(&xas, NULL);
> + }
> + xas_unlock(&xas);
> }
> - xas_unlock(&xas);
> cachefiles_req_put(req);
> - return ret;
> + return ret ? ret : n;
> }
>
> typedef int (*init_req_fn)(struct cachefiles_req *req, void *private);
--
Thanks,
Jingbo
