Previously, @ondemand_id field was used not only to identify ondemand
state of the object, but also to represent the index of the xarray.
This commit introduces @state field to decouple the role of @ondemand_id
and adds helpers to access it.
Signed-off-by: Jia Zhu <[email protected]>
Reviewed-by: Xin Yin <[email protected]>
Reviewed-by: Jingbo Xu <[email protected]>
---
fs/cachefiles/internal.h | 21 +++++++++++++++++++++
fs/cachefiles/ondemand.c | 21 +++++++++------------
2 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h
index 2ad58c465208..b9c76a935ecd 100644
--- a/fs/cachefiles/internal.h
+++ b/fs/cachefiles/internal.h
@@ -44,6 +44,11 @@ struct cachefiles_volume {
struct dentry *fanout[256]; /* Fanout subdirs */
};
+enum cachefiles_object_state {
+ CACHEFILES_ONDEMAND_OBJSTATE_close, /* Anonymous fd closed by daemon or initial state */
+ CACHEFILES_ONDEMAND_OBJSTATE_open, /* Anonymous fd associated with object is available */
+};
+
/*
* Backing file state.
*/
@@ -62,6 +67,7 @@ struct cachefiles_object {
#define CACHEFILES_OBJECT_USING_TMPFILE 0 /* Have an unlinked tmpfile */
#ifdef CONFIG_CACHEFILES_ONDEMAND
int ondemand_id;
+ enum cachefiles_object_state state;
#endif
};
@@ -296,6 +302,21 @@ extern void cachefiles_ondemand_clean_object(struct cachefiles_object *object);
extern int cachefiles_ondemand_read(struct cachefiles_object *object,
loff_t pos, size_t len);
+#define CACHEFILES_OBJECT_STATE_FUNCS(_state) \
+static inline bool \
+cachefiles_ondemand_object_is_##_state(const struct cachefiles_object *object) \
+{ \
+ return object->state == CACHEFILES_ONDEMAND_OBJSTATE_##_state; \
+} \
+ \
+static inline void \
+cachefiles_ondemand_set_object_##_state(struct cachefiles_object *object) \
+{ \
+ object->state = CACHEFILES_ONDEMAND_OBJSTATE_##_state; \
+}
+
+CACHEFILES_OBJECT_STATE_FUNCS(open);
+CACHEFILES_OBJECT_STATE_FUNCS(close);
#else
static inline ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
char __user *_buffer, size_t buflen)
diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c
index 0254ed39f68c..90456b8a4b3e 100644
--- a/fs/cachefiles/ondemand.c
+++ b/fs/cachefiles/ondemand.c
@@ -15,6 +15,7 @@ static int cachefiles_ondemand_fd_release(struct inode *inode,
xa_lock(&cache->reqs);
object->ondemand_id = CACHEFILES_ONDEMAND_ID_CLOSED;
+ cachefiles_ondemand_set_object_close(object);
/*
* Flush all pending READ requests since their completion depends on
@@ -176,6 +177,8 @@ int cachefiles_ondemand_copen(struct cachefiles_cache *cache, char *args)
set_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags);
trace_cachefiles_ondemand_copen(req->object, id, size);
+ cachefiles_ondemand_set_object_open(req->object);
+
out:
complete(&req->done);
return ret;
@@ -363,7 +366,8 @@ static int cachefiles_ondemand_send_req(struct cachefiles_object *object,
/* coupled with the barrier in cachefiles_flush_reqs() */
smp_mb();
- if (opcode != CACHEFILES_OP_OPEN && object->ondemand_id <= 0) {
+ if (opcode != CACHEFILES_OP_OPEN &&
+ !cachefiles_ondemand_object_is_open(object)) {
WARN_ON_ONCE(object->ondemand_id == 0);
xas_unlock(&xas);
ret = -EIO;
@@ -430,18 +434,11 @@ static int cachefiles_ondemand_init_close_req(struct cachefiles_req *req,
void *private)
{
struct cachefiles_object *object = req->object;
- int object_id = object->ondemand_id;
- /*
- * It's possible that object id is still 0 if the cookie looking up
- * phase failed before OPEN request has ever been sent. Also avoid
- * sending CLOSE request for CACHEFILES_ONDEMAND_ID_CLOSED, which means
- * anon_fd has already been closed.
- */
- if (object_id <= 0)
+ if (!cachefiles_ondemand_object_is_open(object))
return -ENOENT;
- req->msg.object_id = object_id;
+ req->msg.object_id = object->ondemand_id;
trace_cachefiles_ondemand_close(object, &req->msg);
return 0;
}
@@ -460,7 +457,7 @@ static int cachefiles_ondemand_init_read_req(struct cachefiles_req *req,
int object_id = object->ondemand_id;
/* Stop enqueuing requests when daemon has closed anon_fd. */
- if (object_id <= 0) {
+ if (!cachefiles_ondemand_object_is_open(object)) {
WARN_ON_ONCE(object_id == 0);
pr_info_once("READ: anonymous fd closed prematurely.\n");
return -EIO;
@@ -485,7 +482,7 @@ int cachefiles_ondemand_init_object(struct cachefiles_object *object)
* creating a new tmpfile as the cache file. Reuse the previously
* allocated object ID if any.
*/
- if (object->ondemand_id > 0)
+ if (cachefiles_ondemand_object_is_open(object))
return 0;
volume_key_size = volume->key[0] + 1;
--
2.20.1
Jia Zhu <[email protected]> wrote:
> +enum cachefiles_object_state {
> + CACHEFILES_ONDEMAND_OBJSTATE_close, /* Anonymous fd closed by daemon or initial state */
> + CACHEFILES_ONDEMAND_OBJSTATE_open, /* Anonymous fd associated with object is available */
That looks weird. Maybe make them all-lowercase?
> @@ -296,6 +302,21 @@ extern void cachefiles_ondemand_clean_object(struct cachefiles_object *object);
> extern int cachefiles_ondemand_read(struct cachefiles_object *object,
> loff_t pos, size_t len);
>
> +#define CACHEFILES_OBJECT_STATE_FUNCS(_state) \
> +static inline bool \
> +cachefiles_ondemand_object_is_##_state(const struct cachefiles_object *object) \
> +{ \
> + return object->state == CACHEFILES_ONDEMAND_OBJSTATE_##_state; \
> +} \
> + \
> +static inline void \
> +cachefiles_ondemand_set_object_##_state(struct cachefiles_object *object) \
> +{ \
> + object->state = CACHEFILES_ONDEMAND_OBJSTATE_##_state; \
> +}
> +
> +CACHEFILES_OBJECT_STATE_FUNCS(open);
> +CACHEFILES_OBJECT_STATE_FUNCS(close);
Or just get rid of the macroisation? If there are only two states, it doesn't
save you that much and it means that "make TAGS" won't generate refs for those
functions and grep won't find them.
David
Hi David,
Thanks for reviewing.
在 2023/3/28 21:52, David Howells 写道:
> Jia Zhu <[email protected]> wrote:
>
>> +enum cachefiles_object_state {
>> + CACHEFILES_ONDEMAND_OBJSTATE_close, /* Anonymous fd closed by daemon or initial state */
>> + CACHEFILES_ONDEMAND_OBJSTATE_open, /* Anonymous fd associated with object is available */
>
> That looks weird. Maybe make them all-lowercase?
I'll revise it in next version.
>
>> @@ -296,6 +302,21 @@ extern void cachefiles_ondemand_clean_object(struct cachefiles_object *object);
>> extern int cachefiles_ondemand_read(struct cachefiles_object *object,
>> loff_t pos, size_t len);
>>
>> +#define CACHEFILES_OBJECT_STATE_FUNCS(_state) \
>> +static inline bool \
>> +cachefiles_ondemand_object_is_##_state(const struct cachefiles_object *object) \
>> +{ \
>> + return object->state == CACHEFILES_ONDEMAND_OBJSTATE_##_state; \
>> +} \
>> + \
>> +static inline void \
>> +cachefiles_ondemand_set_object_##_state(struct cachefiles_object *object) \
>> +{ \
>> + object->state = CACHEFILES_ONDEMAND_OBJSTATE_##_state; \
>> +}
>> +
>> +CACHEFILES_OBJECT_STATE_FUNCS(open);
>> +CACHEFILES_OBJECT_STATE_FUNCS(close);
>
> Or just get rid of the macroisation? If there are only two states, it doesn't
> save you that much and it means that "make TAGS" won't generate refs for those
> functions and grep won't find them.
Actually there is one more state <reopening> will be introduced in
patch3 and 30+ loc for repeated functions will be added if we drop the
macro.
Shall I keep using the macro or replace it?
> David