| Message ID | 20220406075612.60298-5-jefflexu@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fscache,erofs: fscache-based on-demand read semantics | expand |
Jeffle Xu <jefflexu@linux.alibaba.com> wrote: > +static int init_close_req(struct cachefiles_req *req, void *private) "cachefiles_" prefix please. > + /* > + * It's possible if the cookie looking up phase failed before READ > + * request has ever been sent. > + */ What "it" is possible? You might want to say "It's possible that the cookie..." > + if (fd == 0) > + return -ENOENT; 0 is a valid fd. David
On 4/11/22 8:35 PM, David Howells wrote: > Jeffle Xu <jefflexu@linux.alibaba.com> wrote: > >> +static int init_close_req(struct cachefiles_req *req, void *private) > > "cachefiles_" prefix please. Okay. > >> + /* >> + * It's possible if the cookie looking up phase failed before READ >> + * request has ever been sent. >> + */ > > What "it" is possible? You might want to say "It's possible that the > cookie..." "It's possible that the following if (fd == 0) condition is triggered when cookie looking up phase failed before READ request has ever been sent." Anyway I will fix this comment then. > >> + if (fd == 0) >> + return -ENOENT; > > 0 is a valid fd. Yeah, but IMHO fd 0 is always for stdin? I think the allocated anon_fd won't install at fd 0. Please correct me if I'm wrong. In fact I wanna use "fd == 0" as the initial state as struct cachefiles_object is allocated with kmem_cache_zalloc().
JeffleXu <jefflexu@linux.alibaba.com> wrote: > > > >> + if (fd == 0) > >> + return -ENOENT; > > > > 0 is a valid fd. > > Yeah, but IMHO fd 0 is always for stdin? I think the allocated anon_fd > won't install at fd 0. Please correct me if I'm wrong. If someone has closed 0, then you'll get 0 next, I'm pretty sure. Try it and see. > In fact I wanna use "fd == 0" as the initial state as struct > cachefiles_object is allocated with kmem_cache_zalloc(). I would suggest presetting it to something like -2 to avoid confusion. David
On 4/11/22 9:42 PM, David Howells wrote: > JeffleXu <jefflexu@linux.alibaba.com> wrote: > >>> >>>> + if (fd == 0) >>>> + return -ENOENT; >>> >>> 0 is a valid fd. >> >> Yeah, but IMHO fd 0 is always for stdin? I think the allocated anon_fd >> won't install at fd 0. Please correct me if I'm wrong. > > If someone has closed 0, then you'll get 0 next, I'm pretty sure. Try it and > see. Good catch. > >> In fact I wanna use "fd == 0" as the initial state as struct >> cachefiles_object is allocated with kmem_cache_zalloc(). > > I would suggest presetting it to something like -2 to avoid confusion. Okay, as described in the previous email, I'm going to replace @fd to @object_id. I will define some symbols to make it more readable, something like ``` struct cachefiles_object { ... #ifdef CONFIG_CACHEFILES_ONDEMAND #define CACHEFILES_OBJECT_ID_DEFAULT -2 #define CACHEFILES_OBJECT_ID_DEAD -1 int object_id; #endif ... } ``` Thanks for your time.
diff --git a/fs/cachefiles/interface.c b/fs/cachefiles/interface.c index ae93cee9d25d..a69073a1d3f0 100644 --- a/fs/cachefiles/interface.c +++ b/fs/cachefiles/interface.c @@ -362,6 +362,8 @@ static void cachefiles_withdraw_cookie(struct fscache_cookie *cookie) spin_unlock(&cache->object_list_lock); } + cachefiles_ondemand_clean_object(object); + if (object->file) { cachefiles_begin_secure(cache, &saved_cred); cachefiles_clean_up_object(object, cache); diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h index 7d5c7d391fdb..8a397d4da560 100644 --- a/fs/cachefiles/internal.h +++ b/fs/cachefiles/internal.h @@ -279,6 +279,7 @@ extern int cachefiles_ondemand_copen(struct cachefiles_cache *cache, char *args); extern int cachefiles_ondemand_init_object(struct cachefiles_object *object); +extern void cachefiles_ondemand_clean_object(struct cachefiles_object *object); #else static inline ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, @@ -291,6 +292,10 @@ static inline int cachefiles_ondemand_init_object(struct cachefiles_object *obje { return 0; } + +static inline void cachefiles_ondemand_clean_object(struct cachefiles_object *object) +{ +} #endif /* diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c index 75180d02af91..defd65124052 100644 --- a/fs/cachefiles/ondemand.c +++ b/fs/cachefiles/ondemand.c @@ -213,6 +213,12 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, goto err_put_fd; } + /* CLOSE request has no reply */ + if (msg->opcode == CACHEFILES_OP_CLOSE) { + xa_erase(&cache->reqs, id); + complete(&req->done); + } + return n; err_put_fd: @@ -334,6 +340,28 @@ static int init_open_req(struct cachefiles_req *req, void *private) return 0; } +static int init_close_req(struct cachefiles_req *req, void *private) +{ + struct cachefiles_object *object = req->object; + struct cachefiles_close *load = (void *)req->msg.data; + int fd = object->fd; + + if (fd == -1) { + pr_info_once("CLOSE: anonymous fd closed prematurely.\n"); + return -EIO; + } + + /* + * It's possible if the cookie looking up phase failed before READ + * request has ever been sent. + */ + if (fd == 0) + return -ENOENT; + + load->fd = fd; + return 0; +} + int cachefiles_ondemand_init_object(struct cachefiles_object *object) { struct fscache_cookie *cookie = object->cookie; @@ -358,3 +386,11 @@ int cachefiles_ondemand_init_object(struct cachefiles_object *object) CACHEFILES_OP_OPEN, data_len, init_open_req, NULL); } + +void cachefiles_ondemand_clean_object(struct cachefiles_object *object) +{ + cachefiles_ondemand_send_req(object, + CACHEFILES_OP_CLOSE, + sizeof(struct cachefiles_close), + init_close_req, NULL); +} diff --git a/include/uapi/linux/cachefiles.h b/include/uapi/linux/cachefiles.h index 41492f2653c9..73397e142ab3 100644 --- a/include/uapi/linux/cachefiles.h +++ b/include/uapi/linux/cachefiles.h @@ -12,6 +12,7 @@ enum cachefiles_opcode { CACHEFILES_OP_OPEN, + CACHEFILES_OP_CLOSE, }; /* @@ -46,4 +47,8 @@ struct cachefiles_open { __u8 data[]; }; +struct cachefiles_close { + __u32 fd; +}; + #endif
Notify user daemon that cookie is going to be withdrawn, providing a hint that the associated anon_fd can be closed. The anon_fd attached in the CLOSE request shall be same with that in the previous OPEN request. Be noted that this is only a hint. User daemon can close the anon_fd when receiving the CLOSE request, then it will receive another anon_fd if the cookie gets looked up. Or it can also ignore the CLOSE request, and keep writing data into the anon_fd. However the next time cookie gets looked up, the user daemon will still receive another anon_fd. Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com> --- fs/cachefiles/interface.c | 2 ++ fs/cachefiles/internal.h | 5 +++++ fs/cachefiles/ondemand.c | 36 +++++++++++++++++++++++++++++++++ include/uapi/linux/cachefiles.h | 5 +++++ 4 files changed, 48 insertions(+)