2021-06-21 21:45:33

by David Howells

[permalink] [raw]
Subject: [PATCH 00/12] fscache: Some prep work for fscache rewrite


Here are some patches that perform some preparatory work for the fscache
rewrite that's being worked on. These include:

(1) Always select netfs stats when enabling fscache stats since they're
displayed through the same procfile.

(2) Add a cookie debug ID that can be used in tracepoints instead of a
pointer and cache it in the netfs_cache_resources struct rather than
in the netfs_read_request struct to make it more available.

(3) Use file_inode() in cachefiles rather than dereferencing file->f_inode
directly.

(4) Provide a procfile to display fscache cookies.

(5) Remove the fscache and cachefiles histogram procfiles.

(6) Remove the fscache object list procfile.

(7) Avoid using %p in fscache and cachefiles as the value is hashed and
not comparable to the register dump in an oops trace.

(8) Fix the cookie hash function to actually achieve useful dispersion.

(9) Fix fscache_cookie_put() so that it doesn't dereference the cookie
pointer in the tracepoint after the refcount has been decremented
(we're only allowed to do that if we decremented it to zero).

(10) Use refcount_t rather than atomic_t for the fscache_cookie refcount.

The patches can be found on this branch:

http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=fscache-next

David
---
David Howells (12):
fscache: Select netfs stats if fscache stats are enabled
netfs: Move cookie debug ID to struct netfs_cache_resources
cachefiles: Use file_inode() rather than accessing ->f_inode
fscache: Add a cookie debug ID and use that in traces
fscache: Procfile to display cookies
fscache, cachefiles: Remove the histogram stuff
fscache: Remove the object list procfile
fscache: Change %p in format strings to something else
cachefiles: Change %p in format strings to something else
fscache: Fix cookie key hashing
fscache: Fix fscache_cookie_put() to not deref after dec
fscache: Use refcount_t for the cookie refcount instead of atomic_t


fs/cachefiles/Kconfig | 19 --
fs/cachefiles/Makefile | 2 -
fs/cachefiles/bind.c | 2 -
fs/cachefiles/interface.c | 6 +-
fs/cachefiles/internal.h | 25 --
fs/cachefiles/io.c | 6 +-
fs/cachefiles/key.c | 2 +-
fs/cachefiles/main.c | 7 -
fs/cachefiles/namei.c | 61 ++---
fs/cachefiles/proc.c | 114 --------
fs/cachefiles/xattr.c | 4 +-
fs/fscache/Kconfig | 24 --
fs/fscache/Makefile | 2 -
fs/fscache/cache.c | 11 +-
fs/fscache/cookie.c | 201 +++++++++++----
fs/fscache/fsdef.c | 3 +-
fs/fscache/histogram.c | 87 -------
fs/fscache/internal.h | 57 +---
fs/fscache/main.c | 39 +++
fs/fscache/netfs.c | 2 +-
fs/fscache/object-list.c | 414 ------------------------------
fs/fscache/object.c | 8 -
fs/fscache/operation.c | 3 -
fs/fscache/page.c | 6 -
fs/fscache/proc.c | 20 +-
include/linux/fscache-cache.h | 4 -
include/linux/fscache.h | 4 +-
include/linux/netfs.h | 2 +-
include/trace/events/cachefiles.h | 68 ++---
include/trace/events/fscache.h | 160 ++++++------
include/trace/events/netfs.h | 2 +-
31 files changed, 367 insertions(+), 998 deletions(-)
delete mode 100644 fs/cachefiles/proc.c
delete mode 100644 fs/fscache/histogram.c
delete mode 100644 fs/fscache/object-list.c



2021-06-21 21:45:47

by David Howells

[permalink] [raw]
Subject: [PATCH 01/12] fscache: Select netfs stats if fscache stats are enabled

Unconditionally select the stats produced by the netfs lib if fscache stats
are enabled as the former are displayed in the latter's procfile.

Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

fs/fscache/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/fs/fscache/Kconfig b/fs/fscache/Kconfig
index 427efa73b9bd..92c87d8e0913 100644
--- a/fs/fscache/Kconfig
+++ b/fs/fscache/Kconfig
@@ -14,6 +14,7 @@ config FSCACHE
config FSCACHE_STATS
bool "Gather statistical information on local caching"
depends on FSCACHE && PROC_FS
+ select NETFS_STATS
help
This option causes statistical information to be gathered on local
caching and exported through file:


2021-06-21 21:45:58

by David Howells

[permalink] [raw]
Subject: [PATCH 02/12] netfs: Move cookie debug ID to struct netfs_cache_resources

Move the cookie debug ID from struct netfs_read_request to struct
netfs_cache_resources and drop the 'cookie_' prefix. This makes it
available for things that want to use netfs_cache_resources without having
a netfs_read_request.

Signed-off-by: David Howells <[email protected]>
---

fs/cachefiles/io.c | 2 +-
include/linux/netfs.h | 2 +-
include/trace/events/netfs.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c
index b13fb45fc3f3..ca68bb97ca00 100644
--- a/fs/cachefiles/io.c
+++ b/fs/cachefiles/io.c
@@ -410,7 +410,7 @@ int cachefiles_begin_read_operation(struct netfs_read_request *rreq,
rreq->cache_resources.cache_priv = op;
rreq->cache_resources.cache_priv2 = file;
rreq->cache_resources.ops = &cachefiles_netfs_cache_ops;
- rreq->cookie_debug_id = object->fscache.debug_id;
+ rreq->cache_resources.debug_id = object->fscache.debug_id;
_leave("");
return 0;

diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index 9062adfa2fb9..5d6a4158a9a6 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -102,6 +102,7 @@ struct netfs_cache_resources {
const struct netfs_cache_ops *ops;
void *cache_priv;
void *cache_priv2;
+ unsigned int debug_id; /* Cookie debug ID */
};

/*
@@ -137,7 +138,6 @@ struct netfs_read_request {
struct list_head subrequests; /* Requests to fetch I/O from disk or net */
void *netfs_priv; /* Private data for the netfs */
unsigned int debug_id;
- unsigned int cookie_debug_id;
atomic_t nr_rd_ops; /* Number of read ops in progress */
atomic_t nr_wr_ops; /* Number of write ops in progress */
size_t submitted; /* Amount submitted for I/O so far */
diff --git a/include/trace/events/netfs.h b/include/trace/events/netfs.h
index de1c64635e42..4d470bffd9f1 100644
--- a/include/trace/events/netfs.h
+++ b/include/trace/events/netfs.h
@@ -139,7 +139,7 @@ TRACE_EVENT(netfs_read,

TP_fast_assign(
__entry->rreq = rreq->debug_id;
- __entry->cookie = rreq->cookie_debug_id;
+ __entry->cookie = rreq->cache_resources.debug_id;
__entry->start = start;
__entry->len = len;
__entry->what = what;


2021-06-21 21:46:14

by David Howells

[permalink] [raw]
Subject: [PATCH 03/12] cachefiles: Use file_inode() rather than accessing ->f_inode

Use the file_inode() helper rather than accessing ->f_inode directly.

Signed-off-by: David Howells <[email protected]>
---

fs/cachefiles/io.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c
index ca68bb97ca00..fac2e8e7b533 100644
--- a/fs/cachefiles/io.c
+++ b/fs/cachefiles/io.c
@@ -70,7 +70,7 @@ static int cachefiles_read(struct netfs_cache_resources *cres,

_enter("%pD,%li,%llx,%zx/%llx",
file, file_inode(file)->i_ino, start_pos, len,
- i_size_read(file->f_inode));
+ i_size_read(file_inode(file)));

/* If the caller asked us to seek for data before doing the read, then
* we should do that now. If we find a gap, we fill it with zeros.
@@ -194,7 +194,7 @@ static int cachefiles_write(struct netfs_cache_resources *cres,

_enter("%pD,%li,%llx,%zx/%llx",
file, file_inode(file)->i_ino, start_pos, len,
- i_size_read(file->f_inode));
+ i_size_read(file_inode(file)));

ki = kzalloc(sizeof(struct cachefiles_kiocb), GFP_KERNEL);
if (!ki)


2021-06-21 21:46:27

by David Howells

[permalink] [raw]
Subject: [PATCH 04/12] fscache: Add a cookie debug ID and use that in traces

Add a cookie debug ID and use that in traces and in procfiles rather than
displaying the (hashed) pointer to the cookie. This is easier to correlate
and we don't lose anything when interpreting oops output since that shows
unhashed addresses and registers that aren't comparable to the hashed
values.

Signed-off-by: David Howells <[email protected]>
---

fs/fscache/cookie.c | 29 ++++++---
fs/fscache/fsdef.c | 1
fs/fscache/object-list.c | 14 ++--
include/linux/fscache.h | 1
include/trace/events/cachefiles.h | 68 +++++++++++-----------
include/trace/events/fscache.h | 116 +++++++++++++++++++------------------
6 files changed, 121 insertions(+), 108 deletions(-)

diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
index 751bc5b1cddf..f2be98d2c64d 100644
--- a/fs/fscache/cookie.c
+++ b/fs/fscache/cookie.c
@@ -29,21 +29,29 @@ static int fscache_attach_object(struct fscache_cookie *cookie,

static void fscache_print_cookie(struct fscache_cookie *cookie, char prefix)
{
- struct hlist_node *object;
+ struct fscache_object *object;
+ struct hlist_node *o;
const u8 *k;
unsigned loop;

- pr_err("%c-cookie c=%p [p=%p fl=%lx nc=%u na=%u]\n",
- prefix, cookie, cookie->parent, cookie->flags,
+ pr_err("%c-cookie c=%08x [p=%08x fl=%lx nc=%u na=%u]\n",
+ prefix,
+ cookie->debug_id,
+ cookie->parent ? cookie->parent->debug_id : 0,
+ cookie->flags,
atomic_read(&cookie->n_children),
atomic_read(&cookie->n_active));
- pr_err("%c-cookie d=%p n=%p\n",
- prefix, cookie->def, cookie->netfs_data);
+ pr_err("%c-cookie d=%p{%s} n=%p\n",
+ prefix,
+ cookie->def,
+ cookie->def ? cookie->def->name : "?",
+ cookie->netfs_data);

- object = READ_ONCE(cookie->backing_objects.first);
- if (object)
- pr_err("%c-cookie o=%p\n",
- prefix, hlist_entry(object, struct fscache_object, cookie_link));
+ o = READ_ONCE(cookie->backing_objects.first);
+ if (o) {
+ object = hlist_entry(o, struct fscache_object, cookie_link);
+ pr_err("%c-cookie o=%u\n", prefix, object->debug_id);
+ }

pr_err("%c-key=[%u] '", prefix, cookie->key_len);
k = (cookie->key_len <= sizeof(cookie->inline_key)) ?
@@ -129,6 +137,8 @@ static long fscache_compare_cookie(const struct fscache_cookie *a,
return memcmp(ka, kb, a->key_len);
}

+static atomic_t fscache_cookie_debug_id = ATOMIC_INIT(1);
+
/*
* Allocate a cookie.
*/
@@ -163,6 +173,7 @@ struct fscache_cookie *fscache_alloc_cookie(

atomic_set(&cookie->usage, 1);
atomic_set(&cookie->n_children, 0);
+ cookie->debug_id = atomic_inc_return(&fscache_cookie_debug_id);

/* We keep the active count elevated until relinquishment to prevent an
* attempt to wake up every time the object operations queue quiesces.
diff --git a/fs/fscache/fsdef.c b/fs/fscache/fsdef.c
index 09ed8795ad86..5f8f6fe243fe 100644
--- a/fs/fscache/fsdef.c
+++ b/fs/fscache/fsdef.c
@@ -45,6 +45,7 @@ static struct fscache_cookie_def fscache_fsdef_index_def = {
};

struct fscache_cookie fscache_fsdef_index = {
+ .debug_id = 1,
.usage = ATOMIC_INIT(1),
.n_active = ATOMIC_INIT(1),
.lock = __SPIN_LOCK_UNLOCKED(fscache_fsdef_index.lock),
diff --git a/fs/fscache/object-list.c b/fs/fscache/object-list.c
index e106a1a1600d..1a0dc32c0a33 100644
--- a/fs/fscache/object-list.c
+++ b/fs/fscache/object-list.c
@@ -170,7 +170,7 @@ static int fscache_objlist_show(struct seq_file *m, void *v)
if ((unsigned long) v == 1) {
seq_puts(m, "OBJECT PARENT STAT CHLDN OPS OOP IPR EX READS"
" EM EV FL S"
- " | NETFS_COOKIE_DEF TY FL NETFS_DATA");
+ " | COOKIE NETFS_COOKIE_DEF TY FL NETFS_DATA");
if (config & (FSCACHE_OBJLIST_CONFIG_KEY |
FSCACHE_OBJLIST_CONFIG_AUX))
seq_puts(m, " ");
@@ -189,7 +189,7 @@ static int fscache_objlist_show(struct seq_file *m, void *v)
if ((unsigned long) v == 2) {
seq_puts(m, "======== ======== ==== ===== === === === == ====="
" == == == ="
- " | ================ == == ================");
+ " | ======== ================ == === ================");
if (config & (FSCACHE_OBJLIST_CONFIG_KEY |
FSCACHE_OBJLIST_CONFIG_AUX))
seq_puts(m, " ================");
@@ -231,9 +231,9 @@ static int fscache_objlist_show(struct seq_file *m, void *v)
}

seq_printf(m,
- "%8x %8x %s %5u %3u %3u %3u %2u %5u %2lx %2lx %2lx %1x | ",
+ "%08x %08x %s %5u %3u %3u %3u %2u %5u %2lx %2lx %2lx %1x | ",
obj->debug_id,
- obj->parent ? obj->parent->debug_id : -1,
+ obj->parent ? obj->parent->debug_id : UINT_MAX,
obj->state->short_name,
obj->n_children,
obj->n_ops,
@@ -246,7 +246,7 @@ static int fscache_objlist_show(struct seq_file *m, void *v)
obj->flags,
work_busy(&obj->work));

- if (fscache_use_cookie(obj)) {
+ if (obj->cookie) {
uint16_t keylen = 0, auxlen = 0;

switch (cookie->type) {
@@ -263,7 +263,8 @@ static int fscache_objlist_show(struct seq_file *m, void *v)
break;
}

- seq_printf(m, "%-16s %s %2lx %16p",
+ seq_printf(m, "%08x %-16s %s %3lx %16p",
+ cookie->debug_id,
cookie->def->name,
type,
cookie->flags,
@@ -292,7 +293,6 @@ static int fscache_objlist_show(struct seq_file *m, void *v)
}

seq_puts(m, "\n");
- fscache_unuse_cookie(obj);
} else {
seq_puts(m, "<no_netfs>\n");
}
diff --git a/include/linux/fscache.h b/include/linux/fscache.h
index abc1c4737fb8..ba58c427cf9a 100644
--- a/include/linux/fscache.h
+++ b/include/linux/fscache.h
@@ -126,6 +126,7 @@ struct fscache_cookie {
atomic_t usage; /* number of users of this cookie */
atomic_t n_children; /* number of children of this cookie */
atomic_t n_active; /* number of active users of netfs ptrs */
+ unsigned int debug_id;
spinlock_t lock;
spinlock_t stores_lock; /* lock on page store tree */
struct hlist_head backing_objects; /* object(s) backing this file/index */
diff --git a/include/trace/events/cachefiles.h b/include/trace/events/cachefiles.h
index 5d9de24cb9c0..9a448fe9355d 100644
--- a/include/trace/events/cachefiles.h
+++ b/include/trace/events/cachefiles.h
@@ -78,20 +78,20 @@ TRACE_EVENT(cachefiles_ref,

/* Note that obj may be NULL */
TP_STRUCT__entry(
- __field(struct cachefiles_object *, obj )
- __field(struct fscache_cookie *, cookie )
+ __field(unsigned int, obj )
+ __field(unsigned int, cookie )
__field(enum cachefiles_obj_ref_trace, why )
__field(int, usage )
),

TP_fast_assign(
- __entry->obj = obj;
- __entry->cookie = cookie;
+ __entry->obj = obj->fscache.debug_id;
+ __entry->cookie = cookie->debug_id;
__entry->usage = usage;
__entry->why = why;
),

- TP_printk("c=%p o=%p u=%d %s",
+ TP_printk("c=%08x o=%08x u=%d %s",
__entry->cookie, __entry->obj, __entry->usage,
__print_symbolic(__entry->why, cachefiles_obj_ref_traces))
);
@@ -104,18 +104,18 @@ TRACE_EVENT(cachefiles_lookup,
TP_ARGS(obj, de, inode),

TP_STRUCT__entry(
- __field(struct cachefiles_object *, obj )
+ __field(unsigned int, obj )
__field(struct dentry *, de )
__field(struct inode *, inode )
),

TP_fast_assign(
- __entry->obj = obj;
+ __entry->obj = obj->fscache.debug_id;
__entry->de = de;
__entry->inode = inode;
),

- TP_printk("o=%p d=%p i=%p",
+ TP_printk("o=%08x d=%p i=%p",
__entry->obj, __entry->de, __entry->inode)
);

@@ -126,18 +126,18 @@ TRACE_EVENT(cachefiles_mkdir,
TP_ARGS(obj, de, ret),

TP_STRUCT__entry(
- __field(struct cachefiles_object *, obj )
+ __field(unsigned int, obj )
__field(struct dentry *, de )
__field(int, ret )
),

TP_fast_assign(
- __entry->obj = obj;
+ __entry->obj = obj->fscache.debug_id;
__entry->de = de;
__entry->ret = ret;
),

- TP_printk("o=%p d=%p r=%u",
+ TP_printk("o=%08x d=%p r=%u",
__entry->obj, __entry->de, __entry->ret)
);

@@ -148,18 +148,18 @@ TRACE_EVENT(cachefiles_create,
TP_ARGS(obj, de, ret),

TP_STRUCT__entry(
- __field(struct cachefiles_object *, obj )
+ __field(unsigned int, obj )
__field(struct dentry *, de )
__field(int, ret )
),

TP_fast_assign(
- __entry->obj = obj;
+ __entry->obj = obj->fscache.debug_id;
__entry->de = de;
__entry->ret = ret;
),

- TP_printk("o=%p d=%p r=%u",
+ TP_printk("o=%08x d=%p r=%u",
__entry->obj, __entry->de, __entry->ret)
);

@@ -172,18 +172,18 @@ TRACE_EVENT(cachefiles_unlink,

/* Note that obj may be NULL */
TP_STRUCT__entry(
- __field(struct cachefiles_object *, obj )
+ __field(unsigned int, obj )
__field(struct dentry *, de )
__field(enum fscache_why_object_killed, why )
),

TP_fast_assign(
- __entry->obj = obj;
+ __entry->obj = obj->fscache.debug_id;
__entry->de = de;
__entry->why = why;
),

- TP_printk("o=%p d=%p w=%s",
+ TP_printk("o=%08x d=%p w=%s",
__entry->obj, __entry->de,
__print_symbolic(__entry->why, cachefiles_obj_kill_traces))
);
@@ -198,20 +198,20 @@ TRACE_EVENT(cachefiles_rename,

/* Note that obj may be NULL */
TP_STRUCT__entry(
- __field(struct cachefiles_object *, obj )
+ __field(unsigned int, obj )
__field(struct dentry *, de )
__field(struct dentry *, to )
__field(enum fscache_why_object_killed, why )
),

TP_fast_assign(
- __entry->obj = obj;
+ __entry->obj = obj->fscache.debug_id;
__entry->de = de;
__entry->to = to;
__entry->why = why;
),

- TP_printk("o=%p d=%p t=%p w=%s",
+ TP_printk("o=%08x d=%p t=%p w=%s",
__entry->obj, __entry->de, __entry->to,
__print_symbolic(__entry->why, cachefiles_obj_kill_traces))
);
@@ -224,16 +224,16 @@ TRACE_EVENT(cachefiles_mark_active,

/* Note that obj may be NULL */
TP_STRUCT__entry(
- __field(struct cachefiles_object *, obj )
+ __field(unsigned int, obj )
__field(struct dentry *, de )
),

TP_fast_assign(
- __entry->obj = obj;
+ __entry->obj = obj->fscache.debug_id;
__entry->de = de;
),

- TP_printk("o=%p d=%p",
+ TP_printk("o=%08x d=%p",
__entry->obj, __entry->de)
);

@@ -246,22 +246,22 @@ TRACE_EVENT(cachefiles_wait_active,

/* Note that obj may be NULL */
TP_STRUCT__entry(
- __field(struct cachefiles_object *, obj )
+ __field(unsigned int, obj )
+ __field(unsigned int, xobj )
__field(struct dentry *, de )
- __field(struct cachefiles_object *, xobj )
__field(u16, flags )
__field(u16, fsc_flags )
),

TP_fast_assign(
- __entry->obj = obj;
+ __entry->obj = obj->fscache.debug_id;
__entry->de = de;
- __entry->xobj = xobj;
+ __entry->xobj = xobj->fscache.debug_id;
__entry->flags = xobj->flags;
__entry->fsc_flags = xobj->fscache.flags;
),

- TP_printk("o=%p d=%p wo=%p wf=%x wff=%x",
+ TP_printk("o=%08x d=%p wo=%08x wf=%x wff=%x",
__entry->obj, __entry->de, __entry->xobj,
__entry->flags, __entry->fsc_flags)
);
@@ -275,18 +275,18 @@ TRACE_EVENT(cachefiles_mark_inactive,

/* Note that obj may be NULL */
TP_STRUCT__entry(
- __field(struct cachefiles_object *, obj )
+ __field(unsigned int, obj )
__field(struct dentry *, de )
__field(struct inode *, inode )
),

TP_fast_assign(
- __entry->obj = obj;
+ __entry->obj = obj->fscache.debug_id;
__entry->de = de;
__entry->inode = inode;
),

- TP_printk("o=%p d=%p i=%p",
+ TP_printk("o=%08x d=%p i=%p",
__entry->obj, __entry->de, __entry->inode)
);

@@ -299,18 +299,18 @@ TRACE_EVENT(cachefiles_mark_buried,

/* Note that obj may be NULL */
TP_STRUCT__entry(
- __field(struct cachefiles_object *, obj )
+ __field(unsigned int, obj )
__field(struct dentry *, de )
__field(enum fscache_why_object_killed, why )
),

TP_fast_assign(
- __entry->obj = obj;
+ __entry->obj = obj->fscache.debug_id;
__entry->de = de;
__entry->why = why;
),

- TP_printk("o=%p d=%p w=%s",
+ TP_printk("o=%08x d=%p w=%s",
__entry->obj, __entry->de,
__print_symbolic(__entry->why, cachefiles_obj_kill_traces))
);
diff --git a/include/trace/events/fscache.h b/include/trace/events/fscache.h
index d16fe6ed78a2..0b9e058aba4d 100644
--- a/include/trace/events/fscache.h
+++ b/include/trace/events/fscache.h
@@ -167,8 +167,8 @@ TRACE_EVENT(fscache_cookie,
TP_ARGS(cookie, where, usage),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
- __field(struct fscache_cookie *, parent )
+ __field(unsigned int, cookie )
+ __field(unsigned int, parent )
__field(enum fscache_cookie_trace, where )
__field(int, usage )
__field(int, n_children )
@@ -177,8 +177,8 @@ TRACE_EVENT(fscache_cookie,
),

TP_fast_assign(
- __entry->cookie = cookie;
- __entry->parent = cookie->parent;
+ __entry->cookie = cookie->debug_id;
+ __entry->parent = cookie->parent ? cookie->parent->debug_id : 0;
__entry->where = where;
__entry->usage = usage;
__entry->n_children = atomic_read(&cookie->n_children);
@@ -186,7 +186,7 @@ TRACE_EVENT(fscache_cookie,
__entry->flags = cookie->flags;
),

- TP_printk("%s c=%p u=%d p=%p Nc=%d Na=%d f=%02x",
+ TP_printk("%s c=%08x u=%d p=%08x Nc=%d Na=%d f=%02x",
__print_symbolic(__entry->where, fscache_cookie_traces),
__entry->cookie, __entry->usage,
__entry->parent, __entry->n_children, __entry->n_active,
@@ -199,17 +199,17 @@ TRACE_EVENT(fscache_netfs,
TP_ARGS(netfs),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
+ __field(unsigned int, cookie )
__array(char, name, 8 )
),

TP_fast_assign(
- __entry->cookie = netfs->primary_index;
+ __entry->cookie = netfs->primary_index->debug_id;
strncpy(__entry->name, netfs->name, 8);
__entry->name[7] = 0;
),

- TP_printk("c=%p n=%s",
+ TP_printk("c=%08x n=%s",
__entry->cookie, __entry->name)
);

@@ -219,8 +219,8 @@ TRACE_EVENT(fscache_acquire,
TP_ARGS(cookie),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
- __field(struct fscache_cookie *, parent )
+ __field(unsigned int, cookie )
+ __field(unsigned int, parent )
__array(char, name, 8 )
__field(int, p_usage )
__field(int, p_n_children )
@@ -228,8 +228,8 @@ TRACE_EVENT(fscache_acquire,
),

TP_fast_assign(
- __entry->cookie = cookie;
- __entry->parent = cookie->parent;
+ __entry->cookie = cookie->debug_id;
+ __entry->parent = cookie->parent->debug_id;
__entry->p_usage = atomic_read(&cookie->parent->usage);
__entry->p_n_children = atomic_read(&cookie->parent->n_children);
__entry->p_flags = cookie->parent->flags;
@@ -237,7 +237,7 @@ TRACE_EVENT(fscache_acquire,
__entry->name[7] = 0;
),

- TP_printk("c=%p p=%p pu=%d pc=%d pf=%02x n=%s",
+ TP_printk("c=%08x p=%08x pu=%d pc=%d pf=%02x n=%s",
__entry->cookie, __entry->parent, __entry->p_usage,
__entry->p_n_children, __entry->p_flags, __entry->name)
);
@@ -248,8 +248,8 @@ TRACE_EVENT(fscache_relinquish,
TP_ARGS(cookie, retire),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
- __field(struct fscache_cookie *, parent )
+ __field(unsigned int, cookie )
+ __field(unsigned int, parent )
__field(int, usage )
__field(int, n_children )
__field(int, n_active )
@@ -258,8 +258,8 @@ TRACE_EVENT(fscache_relinquish,
),

TP_fast_assign(
- __entry->cookie = cookie;
- __entry->parent = cookie->parent;
+ __entry->cookie = cookie->debug_id;
+ __entry->parent = cookie->parent->debug_id;
__entry->usage = atomic_read(&cookie->usage);
__entry->n_children = atomic_read(&cookie->n_children);
__entry->n_active = atomic_read(&cookie->n_active);
@@ -267,7 +267,7 @@ TRACE_EVENT(fscache_relinquish,
__entry->retire = retire;
),

- TP_printk("c=%p u=%d p=%p Nc=%d Na=%d f=%02x r=%u",
+ TP_printk("c=%08x u=%d p=%08x Nc=%d Na=%d f=%02x r=%u",
__entry->cookie, __entry->usage,
__entry->parent, __entry->n_children, __entry->n_active,
__entry->flags, __entry->retire)
@@ -279,7 +279,7 @@ TRACE_EVENT(fscache_enable,
TP_ARGS(cookie),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
+ __field(unsigned int, cookie )
__field(int, usage )
__field(int, n_children )
__field(int, n_active )
@@ -287,14 +287,14 @@ TRACE_EVENT(fscache_enable,
),

TP_fast_assign(
- __entry->cookie = cookie;
+ __entry->cookie = cookie->debug_id;
__entry->usage = atomic_read(&cookie->usage);
__entry->n_children = atomic_read(&cookie->n_children);
__entry->n_active = atomic_read(&cookie->n_active);
__entry->flags = cookie->flags;
),

- TP_printk("c=%p u=%d Nc=%d Na=%d f=%02x",
+ TP_printk("c=%08x u=%d Nc=%d Na=%d f=%02x",
__entry->cookie, __entry->usage,
__entry->n_children, __entry->n_active, __entry->flags)
);
@@ -305,7 +305,7 @@ TRACE_EVENT(fscache_disable,
TP_ARGS(cookie),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
+ __field(unsigned int, cookie )
__field(int, usage )
__field(int, n_children )
__field(int, n_active )
@@ -313,14 +313,14 @@ TRACE_EVENT(fscache_disable,
),

TP_fast_assign(
- __entry->cookie = cookie;
+ __entry->cookie = cookie->debug_id;
__entry->usage = atomic_read(&cookie->usage);
__entry->n_children = atomic_read(&cookie->n_children);
__entry->n_active = atomic_read(&cookie->n_active);
__entry->flags = cookie->flags;
),

- TP_printk("c=%p u=%d Nc=%d Na=%d f=%02x",
+ TP_printk("c=%08x u=%d Nc=%d Na=%d f=%02x",
__entry->cookie, __entry->usage,
__entry->n_children, __entry->n_active, __entry->flags)
);
@@ -333,8 +333,8 @@ TRACE_EVENT(fscache_osm,
TP_ARGS(object, state, wait, oob, event_num),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
- __field(struct fscache_object *, object )
+ __field(unsigned int, cookie )
+ __field(unsigned int, object )
__array(char, state, 8 )
__field(bool, wait )
__field(bool, oob )
@@ -342,15 +342,15 @@ TRACE_EVENT(fscache_osm,
),

TP_fast_assign(
- __entry->cookie = object->cookie;
- __entry->object = object;
+ __entry->cookie = object->cookie->debug_id;
+ __entry->object = object->debug_id;
__entry->wait = wait;
__entry->oob = oob;
__entry->event_num = event_num;
memcpy(__entry->state, state->short_name, 8);
),

- TP_printk("c=%p o=%p %s %s%sev=%d",
+ TP_printk("c=%08x o=%08d %s %s%sev=%d",
__entry->cookie,
__entry->object,
__entry->state,
@@ -370,18 +370,18 @@ TRACE_EVENT(fscache_page,
TP_ARGS(cookie, page, why),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
+ __field(unsigned int, cookie )
__field(pgoff_t, page )
__field(enum fscache_page_trace, why )
),

TP_fast_assign(
- __entry->cookie = cookie;
+ __entry->cookie = cookie->debug_id;
__entry->page = page->index;
__entry->why = why;
),

- TP_printk("c=%p %s pg=%lx",
+ TP_printk("c=%08x %s pg=%lx",
__entry->cookie,
__print_symbolic(__entry->why, fscache_page_traces),
__entry->page)
@@ -394,20 +394,20 @@ TRACE_EVENT(fscache_check_page,
TP_ARGS(cookie, page, val, n),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
+ __field(unsigned int, cookie )
__field(void *, page )
__field(void *, val )
__field(int, n )
),

TP_fast_assign(
- __entry->cookie = cookie;
+ __entry->cookie = cookie->debug_id;
__entry->page = page;
__entry->val = val;
__entry->n = n;
),

- TP_printk("c=%p pg=%p val=%p n=%d",
+ TP_printk("c=%08x pg=%p val=%p n=%d",
__entry->cookie, __entry->page, __entry->val, __entry->n)
);

@@ -417,14 +417,14 @@ TRACE_EVENT(fscache_wake_cookie,
TP_ARGS(cookie),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
+ __field(unsigned int, cookie )
),

TP_fast_assign(
- __entry->cookie = cookie;
+ __entry->cookie = cookie->debug_id;
),

- TP_printk("c=%p", __entry->cookie)
+ TP_printk("c=%08x", __entry->cookie)
);

TRACE_EVENT(fscache_op,
@@ -434,18 +434,18 @@ TRACE_EVENT(fscache_op,
TP_ARGS(cookie, op, why),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
- __field(struct fscache_operation *, op )
+ __field(unsigned int, cookie )
+ __field(unsigned int, op )
__field(enum fscache_op_trace, why )
),

TP_fast_assign(
- __entry->cookie = cookie;
- __entry->op = op;
+ __entry->cookie = cookie->debug_id;
+ __entry->op = op->debug_id;
__entry->why = why;
),

- TP_printk("c=%p op=%p %s",
+ TP_printk("c=%08x op=%08x %s",
__entry->cookie, __entry->op,
__print_symbolic(__entry->why, fscache_op_traces))
);
@@ -457,20 +457,20 @@ TRACE_EVENT(fscache_page_op,
TP_ARGS(cookie, page, op, what),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
+ __field(unsigned int, cookie )
+ __field(unsigned int, op )
__field(pgoff_t, page )
- __field(struct fscache_operation *, op )
__field(enum fscache_page_op_trace, what )
),

TP_fast_assign(
- __entry->cookie = cookie;
+ __entry->cookie = cookie->debug_id;
__entry->page = page ? page->index : 0;
- __entry->op = op;
+ __entry->op = op->debug_id;
__entry->what = what;
),

- TP_printk("c=%p %s pg=%lx op=%p",
+ TP_printk("c=%08x %s pg=%lx op=%08x",
__entry->cookie,
__print_symbolic(__entry->what, fscache_page_op_traces),
__entry->page, __entry->op)
@@ -483,20 +483,20 @@ TRACE_EVENT(fscache_wrote_page,
TP_ARGS(cookie, page, op, ret),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
+ __field(unsigned int, cookie )
+ __field(unsigned int, op )
__field(pgoff_t, page )
- __field(struct fscache_operation *, op )
__field(int, ret )
),

TP_fast_assign(
- __entry->cookie = cookie;
+ __entry->cookie = cookie->debug_id;
__entry->page = page->index;
- __entry->op = op;
+ __entry->op = op->debug_id;
__entry->ret = ret;
),

- TP_printk("c=%p pg=%lx op=%p ret=%d",
+ TP_printk("c=%08x pg=%lx op=%08x ret=%d",
__entry->cookie, __entry->page, __entry->op, __entry->ret)
);

@@ -507,22 +507,22 @@ TRACE_EVENT(fscache_gang_lookup,
TP_ARGS(cookie, op, results, n, store_limit),

TP_STRUCT__entry(
- __field(struct fscache_cookie *, cookie )
- __field(struct fscache_operation *, op )
+ __field(unsigned int, cookie )
+ __field(unsigned int, op )
__field(pgoff_t, results0 )
__field(int, n )
__field(pgoff_t, store_limit )
),

TP_fast_assign(
- __entry->cookie = cookie;
- __entry->op = op;
+ __entry->cookie = cookie->debug_id;
+ __entry->op = op->debug_id;
__entry->results0 = results[0] ? ((struct page *)results[0])->index : (pgoff_t)-1;
__entry->n = n;
__entry->store_limit = store_limit;
),

- TP_printk("c=%p op=%p r0=%lx n=%d sl=%lx",
+ TP_printk("c=%08x op=%08x r0=%lx n=%d sl=%lx",
__entry->cookie, __entry->op, __entry->results0, __entry->n,
__entry->store_limit)
);


2021-06-21 21:46:49

by David Howells

[permalink] [raw]
Subject: [PATCH 05/12] fscache: Procfile to display cookies

Add /proc/fs/fscache/cookies to display active cookies.

Signed-off-by: David Howells <[email protected]>
---

fs/fscache/cookie.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++
fs/fscache/internal.h | 1
fs/fscache/proc.c | 7 +++
include/linux/fscache.h | 1
4 files changed, 112 insertions(+)

diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
index f2be98d2c64d..c7047544972b 100644
--- a/fs/fscache/cookie.c
+++ b/fs/fscache/cookie.c
@@ -19,6 +19,8 @@ static atomic_t fscache_object_debug_id = ATOMIC_INIT(0);

#define fscache_cookie_hash_shift 15
static struct hlist_bl_head fscache_cookie_hash[1 << fscache_cookie_hash_shift];
+static LIST_HEAD(fscache_cookies);
+static DEFINE_RWLOCK(fscache_cookies_lock);

static int fscache_acquire_non_index_cookie(struct fscache_cookie *cookie,
loff_t object_size);
@@ -65,6 +67,9 @@ void fscache_free_cookie(struct fscache_cookie *cookie)
{
if (cookie) {
BUG_ON(!hlist_empty(&cookie->backing_objects));
+ write_lock(&fscache_cookies_lock);
+ list_del(&cookie->proc_link);
+ write_unlock(&fscache_cookies_lock);
if (cookie->aux_len > sizeof(cookie->inline_aux))
kfree(cookie->aux);
if (cookie->key_len > sizeof(cookie->inline_key))
@@ -192,6 +197,10 @@ struct fscache_cookie *fscache_alloc_cookie(
/* radix tree insertion won't use the preallocation pool unless it's
* told it may not wait */
INIT_RADIX_TREE(&cookie->stores, GFP_NOFS & ~__GFP_DIRECT_RECLAIM);
+
+ write_lock(&fscache_cookies_lock);
+ list_add_tail(&cookie->proc_link, &fscache_cookies);
+ write_unlock(&fscache_cookies_lock);
return cookie;

nomem:
@@ -969,3 +978,97 @@ int __fscache_check_consistency(struct fscache_cookie *cookie,
return -ESTALE;
}
EXPORT_SYMBOL(__fscache_check_consistency);
+
+/*
+ * Generate a list of extant cookies in /proc/fs/fscache/cookies
+ */
+static int fscache_cookies_seq_show(struct seq_file *m, void *v)
+{
+ struct fscache_cookie *cookie;
+ unsigned int keylen = 0, auxlen = 0;
+ char _type[3], *type;
+ u8 *p;
+
+ if (v == &fscache_cookies) {
+ seq_puts(m,
+ "COOKIE PARENT USAGE CHILD ACT TY FL DEF NETFS_DATA\n"
+ "======== ======== ===== ===== === == === ================ ==========\n"
+ );
+ return 0;
+ }
+
+ cookie = list_entry(v, struct fscache_cookie, proc_link);
+
+ switch (cookie->type) {
+ case 0:
+ type = "IX";
+ break;
+ case 1:
+ type = "DT";
+ break;
+ default:
+ snprintf(_type, sizeof(_type), "%02u",
+ cookie->type);
+ type = _type;
+ break;
+ }
+
+ seq_printf(m,
+ "%08x %08x %5u %5u %3u %s %03lx %-16s %px",
+ cookie->debug_id,
+ cookie->parent ? cookie->parent->debug_id : 0,
+ atomic_read(&cookie->usage),
+ atomic_read(&cookie->n_children),
+ atomic_read(&cookie->n_active),
+ type,
+ cookie->flags,
+ cookie->def->name,
+ cookie->netfs_data);
+
+ keylen = cookie->key_len;
+ auxlen = cookie->aux_len;
+
+ if (keylen > 0 || auxlen > 0) {
+ seq_puts(m, " ");
+ p = keylen <= sizeof(cookie->inline_key) ?
+ cookie->inline_key : cookie->key;
+ for (; keylen > 0; keylen--)
+ seq_printf(m, "%02x", *p++);
+ if (auxlen > 0) {
+ seq_puts(m, ", ");
+ p = auxlen <= sizeof(cookie->inline_aux) ?
+ cookie->inline_aux : cookie->aux;
+ for (; auxlen > 0; auxlen--)
+ seq_printf(m, "%02x", *p++);
+ }
+ }
+
+ seq_puts(m, "\n");
+ return 0;
+}
+
+static void *fscache_cookies_seq_start(struct seq_file *m, loff_t *_pos)
+ __acquires(fscache_cookies_lock)
+{
+ read_lock(&fscache_cookies_lock);
+ return seq_list_start_head(&fscache_cookies, *_pos);
+}
+
+static void *fscache_cookies_seq_next(struct seq_file *m, void *v, loff_t *_pos)
+{
+ return seq_list_next(v, &fscache_cookies, _pos);
+}
+
+static void fscache_cookies_seq_stop(struct seq_file *m, void *v)
+ __releases(rcu)
+{
+ read_unlock(&fscache_cookies_lock);
+}
+
+
+const struct seq_operations fscache_cookies_seq_ops = {
+ .start = fscache_cookies_seq_start,
+ .next = fscache_cookies_seq_next,
+ .stop = fscache_cookies_seq_stop,
+ .show = fscache_cookies_seq_show,
+};
diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
index c483863b740a..207a6bc81ca9 100644
--- a/fs/fscache/internal.h
+++ b/fs/fscache/internal.h
@@ -45,6 +45,7 @@ extern struct fscache_cache *fscache_select_cache_for_object(
* cookie.c
*/
extern struct kmem_cache *fscache_cookie_jar;
+extern const struct seq_operations fscache_cookies_seq_ops;

extern void fscache_free_cookie(struct fscache_cookie *);
extern struct fscache_cookie *fscache_alloc_cookie(struct fscache_cookie *,
diff --git a/fs/fscache/proc.c b/fs/fscache/proc.c
index 90a7bc22f7e1..da51fdfc8641 100644
--- a/fs/fscache/proc.c
+++ b/fs/fscache/proc.c
@@ -21,6 +21,10 @@ int __init fscache_proc_init(void)
if (!proc_mkdir("fs/fscache", NULL))
goto error_dir;

+ if (!proc_create_seq("fs/fscache/cookies", S_IFREG | 0444, NULL,
+ &fscache_cookies_seq_ops))
+ goto error_cookies;
+
#ifdef CONFIG_FSCACHE_STATS
if (!proc_create_single("fs/fscache/stats", S_IFREG | 0444, NULL,
fscache_stats_show))
@@ -53,6 +57,8 @@ int __init fscache_proc_init(void)
remove_proc_entry("fs/fscache/stats", NULL);
error_stats:
#endif
+ remove_proc_entry("fs/fscache/cookies", NULL);
+error_cookies:
remove_proc_entry("fs/fscache", NULL);
error_dir:
_leave(" = -ENOMEM");
@@ -73,5 +79,6 @@ void fscache_proc_cleanup(void)
#ifdef CONFIG_FSCACHE_STATS
remove_proc_entry("fs/fscache/stats", NULL);
#endif
+ remove_proc_entry("fs/fscache/cookies", NULL);
remove_proc_entry("fs/fscache", NULL);
}
diff --git a/include/linux/fscache.h b/include/linux/fscache.h
index ba58c427cf9a..ea61e54a6bc5 100644
--- a/include/linux/fscache.h
+++ b/include/linux/fscache.h
@@ -133,6 +133,7 @@ struct fscache_cookie {
const struct fscache_cookie_def *def; /* definition */
struct fscache_cookie *parent; /* parent of this entry */
struct hlist_bl_node hash_link; /* Link in hash table */
+ struct list_head proc_link; /* Link in proc list */
void *netfs_data; /* back pointer to netfs */
struct radix_tree_root stores; /* pages to be stored on this cookie */
#define FSCACHE_COOKIE_PENDING_TAG 0 /* pages tag: pending write to cache */


2021-06-21 21:47:11

by David Howells

[permalink] [raw]
Subject: [PATCH 07/12] fscache: Remove the object list procfile

Remove the object list procfile from fscache as objects will become
entirely internal to the cache.

Signed-off-by: David Howells <[email protected]>
---

fs/fscache/Kconfig | 7 -
fs/fscache/Makefile | 1
fs/fscache/cache.c | 1
fs/fscache/cookie.c | 2
fs/fscache/internal.h | 13 -
fs/fscache/object-list.c | 414 -----------------------------------------
fs/fscache/object.c | 2
include/linux/fscache-cache.h | 3
8 files changed, 443 deletions(-)
delete mode 100644 fs/fscache/object-list.c

diff --git a/fs/fscache/Kconfig b/fs/fscache/Kconfig
index 5e3a5b3f950d..b313a978ae0a 100644
--- a/fs/fscache/Kconfig
+++ b/fs/fscache/Kconfig
@@ -38,10 +38,3 @@ config FSCACHE_DEBUG
enabled by setting bits in /sys/modules/fscache/parameter/debug.

See Documentation/filesystems/caching/fscache.rst for more information.
-
-config FSCACHE_OBJECT_LIST
- bool "Maintain global object list for debugging purposes"
- depends on FSCACHE && PROC_FS
- help
- Maintain a global list of active fscache objects that can be
- retrieved through /proc/fs/fscache/objects for debugging purposes
diff --git a/fs/fscache/Makefile b/fs/fscache/Makefile
index 45d5235a449b..03a871d689bb 100644
--- a/fs/fscache/Makefile
+++ b/fs/fscache/Makefile
@@ -16,6 +16,5 @@ fscache-y := \

fscache-$(CONFIG_PROC_FS) += proc.o
fscache-$(CONFIG_FSCACHE_STATS) += stats.o
-fscache-$(CONFIG_FSCACHE_OBJECT_LIST) += object-list.o

obj-$(CONFIG_FSCACHE) := fscache.o
diff --git a/fs/fscache/cache.c b/fs/fscache/cache.c
index fcc136361415..8a6ffcac867f 100644
--- a/fs/fscache/cache.c
+++ b/fs/fscache/cache.c
@@ -261,7 +261,6 @@ int fscache_add_cache(struct fscache_cache *cache,
spin_lock(&cache->object_list_lock);
list_add_tail(&ifsdef->cache_link, &cache->object_list);
spin_unlock(&cache->object_list_lock);
- fscache_objlist_add(ifsdef);

/* add the cache's netfs definition index object to the top level index
* cookie as a known backing object */
diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
index c7047544972b..2f4d5271ad2e 100644
--- a/fs/fscache/cookie.c
+++ b/fs/fscache/cookie.c
@@ -620,8 +620,6 @@ static int fscache_attach_object(struct fscache_cookie *cookie,

/* Attach to the cookie. The object already has a ref on it. */
hlist_add_head(&object->cookie_link, &cookie->backing_objects);
-
- fscache_objlist_add(object);
ret = 0;

cant_attach_object:
diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
index 796678b2b32a..200082cafdda 100644
--- a/fs/fscache/internal.h
+++ b/fs/fscache/internal.h
@@ -84,19 +84,6 @@ static inline bool fscache_object_congested(void)
*/
extern void fscache_enqueue_object(struct fscache_object *);

-/*
- * object-list.c
- */
-#ifdef CONFIG_FSCACHE_OBJECT_LIST
-extern const struct proc_ops fscache_objlist_proc_ops;
-
-extern void fscache_objlist_add(struct fscache_object *);
-extern void fscache_objlist_remove(struct fscache_object *);
-#else
-#define fscache_objlist_add(object) do {} while(0)
-#define fscache_objlist_remove(object) do {} while(0)
-#endif
-
/*
* operation.c
*/
diff --git a/fs/fscache/object-list.c b/fs/fscache/object-list.c
deleted file mode 100644
index 1a0dc32c0a33..000000000000
--- a/fs/fscache/object-list.c
+++ /dev/null
@@ -1,414 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/* Global fscache object list maintainer and viewer
- *
- * Copyright (C) 2009 Red Hat, Inc. All Rights Reserved.
- * Written by David Howells ([email protected])
- */
-
-#define FSCACHE_DEBUG_LEVEL COOKIE
-#include <linux/module.h>
-#include <linux/proc_fs.h>
-#include <linux/seq_file.h>
-#include <linux/slab.h>
-#include <linux/key.h>
-#include <keys/user-type.h>
-#include "internal.h"
-
-static struct rb_root fscache_object_list;
-static DEFINE_RWLOCK(fscache_object_list_lock);
-
-struct fscache_objlist_data {
- unsigned long config; /* display configuration */
-#define FSCACHE_OBJLIST_CONFIG_KEY 0x00000001 /* show object keys */
-#define FSCACHE_OBJLIST_CONFIG_AUX 0x00000002 /* show object auxdata */
-#define FSCACHE_OBJLIST_CONFIG_COOKIE 0x00000004 /* show objects with cookies */
-#define FSCACHE_OBJLIST_CONFIG_NOCOOKIE 0x00000008 /* show objects without cookies */
-#define FSCACHE_OBJLIST_CONFIG_BUSY 0x00000010 /* show busy objects */
-#define FSCACHE_OBJLIST_CONFIG_IDLE 0x00000020 /* show idle objects */
-#define FSCACHE_OBJLIST_CONFIG_PENDWR 0x00000040 /* show objects with pending writes */
-#define FSCACHE_OBJLIST_CONFIG_NOPENDWR 0x00000080 /* show objects without pending writes */
-#define FSCACHE_OBJLIST_CONFIG_READS 0x00000100 /* show objects with active reads */
-#define FSCACHE_OBJLIST_CONFIG_NOREADS 0x00000200 /* show objects without active reads */
-#define FSCACHE_OBJLIST_CONFIG_EVENTS 0x00000400 /* show objects with events */
-#define FSCACHE_OBJLIST_CONFIG_NOEVENTS 0x00000800 /* show objects without no events */
-#define FSCACHE_OBJLIST_CONFIG_WORK 0x00001000 /* show objects with work */
-#define FSCACHE_OBJLIST_CONFIG_NOWORK 0x00002000 /* show objects without work */
-};
-
-/*
- * Add an object to the object list
- * - we use the address of the fscache_object structure as the key into the
- * tree
- */
-void fscache_objlist_add(struct fscache_object *obj)
-{
- struct fscache_object *xobj;
- struct rb_node **p = &fscache_object_list.rb_node, *parent = NULL;
-
- ASSERT(RB_EMPTY_NODE(&obj->objlist_link));
-
- write_lock(&fscache_object_list_lock);
-
- while (*p) {
- parent = *p;
- xobj = rb_entry(parent, struct fscache_object, objlist_link);
-
- if (obj < xobj)
- p = &(*p)->rb_left;
- else if (obj > xobj)
- p = &(*p)->rb_right;
- else
- BUG();
- }
-
- rb_link_node(&obj->objlist_link, parent, p);
- rb_insert_color(&obj->objlist_link, &fscache_object_list);
-
- write_unlock(&fscache_object_list_lock);
-}
-
-/*
- * Remove an object from the object list.
- */
-void fscache_objlist_remove(struct fscache_object *obj)
-{
- if (RB_EMPTY_NODE(&obj->objlist_link))
- return;
-
- write_lock(&fscache_object_list_lock);
-
- BUG_ON(RB_EMPTY_ROOT(&fscache_object_list));
- rb_erase(&obj->objlist_link, &fscache_object_list);
-
- write_unlock(&fscache_object_list_lock);
-}
-
-/*
- * find the object in the tree on or after the specified index
- */
-static struct fscache_object *fscache_objlist_lookup(loff_t *_pos)
-{
- struct fscache_object *pobj, *obj = NULL, *minobj = NULL;
- struct rb_node *p;
- unsigned long pos;
-
- if (*_pos >= (unsigned long) ERR_PTR(-ENOENT))
- return NULL;
- pos = *_pos;
-
- /* banners (can't represent line 0 by pos 0 as that would involve
- * returning a NULL pointer) */
- if (pos == 0)
- return (struct fscache_object *)(long)++(*_pos);
- if (pos < 3)
- return (struct fscache_object *)pos;
-
- pobj = (struct fscache_object *)pos;
- p = fscache_object_list.rb_node;
- while (p) {
- obj = rb_entry(p, struct fscache_object, objlist_link);
- if (pobj < obj) {
- if (!minobj || minobj > obj)
- minobj = obj;
- p = p->rb_left;
- } else if (pobj > obj) {
- p = p->rb_right;
- } else {
- minobj = obj;
- break;
- }
- obj = NULL;
- }
-
- if (!minobj)
- *_pos = (unsigned long) ERR_PTR(-ENOENT);
- else if (minobj != obj)
- *_pos = (unsigned long) minobj;
- return minobj;
-}
-
-/*
- * set up the iterator to start reading from the first line
- */
-static void *fscache_objlist_start(struct seq_file *m, loff_t *_pos)
- __acquires(&fscache_object_list_lock)
-{
- read_lock(&fscache_object_list_lock);
- return fscache_objlist_lookup(_pos);
-}
-
-/*
- * move to the next line
- */
-static void *fscache_objlist_next(struct seq_file *m, void *v, loff_t *_pos)
-{
- (*_pos)++;
- return fscache_objlist_lookup(_pos);
-}
-
-/*
- * clean up after reading
- */
-static void fscache_objlist_stop(struct seq_file *m, void *v)
- __releases(&fscache_object_list_lock)
-{
- read_unlock(&fscache_object_list_lock);
-}
-
-/*
- * display an object
- */
-static int fscache_objlist_show(struct seq_file *m, void *v)
-{
- struct fscache_objlist_data *data = m->private;
- struct fscache_object *obj = v;
- struct fscache_cookie *cookie;
- unsigned long config = data->config;
- char _type[3], *type;
- u8 *p;
-
- if ((unsigned long) v == 1) {
- seq_puts(m, "OBJECT PARENT STAT CHLDN OPS OOP IPR EX READS"
- " EM EV FL S"
- " | COOKIE NETFS_COOKIE_DEF TY FL NETFS_DATA");
- if (config & (FSCACHE_OBJLIST_CONFIG_KEY |
- FSCACHE_OBJLIST_CONFIG_AUX))
- seq_puts(m, " ");
- if (config & FSCACHE_OBJLIST_CONFIG_KEY)
- seq_puts(m, "OBJECT_KEY");
- if ((config & (FSCACHE_OBJLIST_CONFIG_KEY |
- FSCACHE_OBJLIST_CONFIG_AUX)) ==
- (FSCACHE_OBJLIST_CONFIG_KEY | FSCACHE_OBJLIST_CONFIG_AUX))
- seq_puts(m, ", ");
- if (config & FSCACHE_OBJLIST_CONFIG_AUX)
- seq_puts(m, "AUX_DATA");
- seq_puts(m, "\n");
- return 0;
- }
-
- if ((unsigned long) v == 2) {
- seq_puts(m, "======== ======== ==== ===== === === === == ====="
- " == == == ="
- " | ======== ================ == === ================");
- if (config & (FSCACHE_OBJLIST_CONFIG_KEY |
- FSCACHE_OBJLIST_CONFIG_AUX))
- seq_puts(m, " ================");
- seq_puts(m, "\n");
- return 0;
- }
-
- /* filter out any unwanted objects */
-#define FILTER(criterion, _yes, _no) \
- do { \
- unsigned long yes = FSCACHE_OBJLIST_CONFIG_##_yes; \
- unsigned long no = FSCACHE_OBJLIST_CONFIG_##_no; \
- if (criterion) { \
- if (!(config & yes)) \
- return 0; \
- } else { \
- if (!(config & no)) \
- return 0; \
- } \
- } while(0)
-
- cookie = obj->cookie;
- if (~config) {
- FILTER(cookie->def,
- COOKIE, NOCOOKIE);
- FILTER(fscache_object_is_active(obj) ||
- obj->n_ops != 0 ||
- obj->n_obj_ops != 0 ||
- obj->flags ||
- !list_empty(&obj->dependents),
- BUSY, IDLE);
- FILTER(test_bit(FSCACHE_OBJECT_PENDING_WRITE, &obj->flags),
- PENDWR, NOPENDWR);
- FILTER(atomic_read(&obj->n_reads),
- READS, NOREADS);
- FILTER(obj->events & obj->event_mask,
- EVENTS, NOEVENTS);
- FILTER(work_busy(&obj->work), WORK, NOWORK);
- }
-
- seq_printf(m,
- "%08x %08x %s %5u %3u %3u %3u %2u %5u %2lx %2lx %2lx %1x | ",
- obj->debug_id,
- obj->parent ? obj->parent->debug_id : UINT_MAX,
- obj->state->short_name,
- obj->n_children,
- obj->n_ops,
- obj->n_obj_ops,
- obj->n_in_progress,
- obj->n_exclusive,
- atomic_read(&obj->n_reads),
- obj->event_mask,
- obj->events,
- obj->flags,
- work_busy(&obj->work));
-
- if (obj->cookie) {
- uint16_t keylen = 0, auxlen = 0;
-
- switch (cookie->type) {
- case 0:
- type = "IX";
- break;
- case 1:
- type = "DT";
- break;
- default:
- snprintf(_type, sizeof(_type), "%02u",
- cookie->type);
- type = _type;
- break;
- }
-
- seq_printf(m, "%08x %-16s %s %3lx %16p",
- cookie->debug_id,
- cookie->def->name,
- type,
- cookie->flags,
- cookie->netfs_data);
-
- if (config & FSCACHE_OBJLIST_CONFIG_KEY)
- keylen = cookie->key_len;
-
- if (config & FSCACHE_OBJLIST_CONFIG_AUX)
- auxlen = cookie->aux_len;
-
- if (keylen > 0 || auxlen > 0) {
- seq_puts(m, " ");
- p = keylen <= sizeof(cookie->inline_key) ?
- cookie->inline_key : cookie->key;
- for (; keylen > 0; keylen--)
- seq_printf(m, "%02x", *p++);
- if (auxlen > 0) {
- if (config & FSCACHE_OBJLIST_CONFIG_KEY)
- seq_puts(m, ", ");
- p = auxlen <= sizeof(cookie->inline_aux) ?
- cookie->inline_aux : cookie->aux;
- for (; auxlen > 0; auxlen--)
- seq_printf(m, "%02x", *p++);
- }
- }
-
- seq_puts(m, "\n");
- } else {
- seq_puts(m, "<no_netfs>\n");
- }
- return 0;
-}
-
-static const struct seq_operations fscache_objlist_ops = {
- .start = fscache_objlist_start,
- .stop = fscache_objlist_stop,
- .next = fscache_objlist_next,
- .show = fscache_objlist_show,
-};
-
-/*
- * get the configuration for filtering the list
- */
-static void fscache_objlist_config(struct fscache_objlist_data *data)
-{
-#ifdef CONFIG_KEYS
- const struct user_key_payload *confkey;
- unsigned long config;
- struct key *key;
- const char *buf;
- int len;
-
- key = request_key(&key_type_user, "fscache:objlist", NULL);
- if (IS_ERR(key))
- goto no_config;
-
- config = 0;
- rcu_read_lock();
-
- confkey = user_key_payload_rcu(key);
- if (!confkey) {
- /* key was revoked */
- rcu_read_unlock();
- key_put(key);
- goto no_config;
- }
-
- buf = confkey->data;
-
- for (len = confkey->datalen - 1; len >= 0; len--) {
- switch (buf[len]) {
- case 'K': config |= FSCACHE_OBJLIST_CONFIG_KEY; break;
- case 'A': config |= FSCACHE_OBJLIST_CONFIG_AUX; break;
- case 'C': config |= FSCACHE_OBJLIST_CONFIG_COOKIE; break;
- case 'c': config |= FSCACHE_OBJLIST_CONFIG_NOCOOKIE; break;
- case 'B': config |= FSCACHE_OBJLIST_CONFIG_BUSY; break;
- case 'b': config |= FSCACHE_OBJLIST_CONFIG_IDLE; break;
- case 'W': config |= FSCACHE_OBJLIST_CONFIG_PENDWR; break;
- case 'w': config |= FSCACHE_OBJLIST_CONFIG_NOPENDWR; break;
- case 'R': config |= FSCACHE_OBJLIST_CONFIG_READS; break;
- case 'r': config |= FSCACHE_OBJLIST_CONFIG_NOREADS; break;
- case 'S': config |= FSCACHE_OBJLIST_CONFIG_WORK; break;
- case 's': config |= FSCACHE_OBJLIST_CONFIG_NOWORK; break;
- }
- }
-
- rcu_read_unlock();
- key_put(key);
-
- if (!(config & (FSCACHE_OBJLIST_CONFIG_COOKIE | FSCACHE_OBJLIST_CONFIG_NOCOOKIE)))
- config |= FSCACHE_OBJLIST_CONFIG_COOKIE | FSCACHE_OBJLIST_CONFIG_NOCOOKIE;
- if (!(config & (FSCACHE_OBJLIST_CONFIG_BUSY | FSCACHE_OBJLIST_CONFIG_IDLE)))
- config |= FSCACHE_OBJLIST_CONFIG_BUSY | FSCACHE_OBJLIST_CONFIG_IDLE;
- if (!(config & (FSCACHE_OBJLIST_CONFIG_PENDWR | FSCACHE_OBJLIST_CONFIG_NOPENDWR)))
- config |= FSCACHE_OBJLIST_CONFIG_PENDWR | FSCACHE_OBJLIST_CONFIG_NOPENDWR;
- if (!(config & (FSCACHE_OBJLIST_CONFIG_READS | FSCACHE_OBJLIST_CONFIG_NOREADS)))
- config |= FSCACHE_OBJLIST_CONFIG_READS | FSCACHE_OBJLIST_CONFIG_NOREADS;
- if (!(config & (FSCACHE_OBJLIST_CONFIG_EVENTS | FSCACHE_OBJLIST_CONFIG_NOEVENTS)))
- config |= FSCACHE_OBJLIST_CONFIG_EVENTS | FSCACHE_OBJLIST_CONFIG_NOEVENTS;
- if (!(config & (FSCACHE_OBJLIST_CONFIG_WORK | FSCACHE_OBJLIST_CONFIG_NOWORK)))
- config |= FSCACHE_OBJLIST_CONFIG_WORK | FSCACHE_OBJLIST_CONFIG_NOWORK;
-
- data->config = config;
- return;
-
-no_config:
-#endif
- data->config = ULONG_MAX;
-}
-
-/*
- * open "/proc/fs/fscache/objects" to provide a list of active objects
- * - can be configured by a user-defined key added to the caller's keyrings
- */
-static int fscache_objlist_open(struct inode *inode, struct file *file)
-{
- struct fscache_objlist_data *data;
-
- data = __seq_open_private(file, &fscache_objlist_ops, sizeof(*data));
- if (!data)
- return -ENOMEM;
-
- /* get the configuration key */
- fscache_objlist_config(data);
-
- return 0;
-}
-
-/*
- * clean up on close
- */
-static int fscache_objlist_release(struct inode *inode, struct file *file)
-{
- struct seq_file *m = file->private_data;
-
- kfree(m->private);
- m->private = NULL;
- return seq_release(inode, file);
-}
-
-const struct proc_ops fscache_objlist_proc_ops = {
- .proc_open = fscache_objlist_open,
- .proc_read = seq_read,
- .proc_lseek = seq_lseek,
- .proc_release = fscache_objlist_release,
-};
diff --git a/fs/fscache/object.c b/fs/fscache/object.c
index 5dbaab2e1262..b3853274733f 100644
--- a/fs/fscache/object.c
+++ b/fs/fscache/object.c
@@ -794,8 +794,6 @@ static void fscache_put_object(struct fscache_object *object,
*/
void fscache_object_destroy(struct fscache_object *object)
{
- fscache_objlist_remove(object);
-
/* We can get rid of the cookie now */
fscache_cookie_put(object->cookie, fscache_cookie_put_object);
object->cookie = NULL;
diff --git a/include/linux/fscache-cache.h b/include/linux/fscache-cache.h
index fbff0b7e3ef1..8d39491c5f9f 100644
--- a/include/linux/fscache-cache.h
+++ b/include/linux/fscache-cache.h
@@ -384,9 +384,6 @@ struct fscache_object {
struct list_head dependents; /* FIFO of dependent objects */
struct list_head dep_link; /* link in parent's dependents list */
struct list_head pending_ops; /* unstarted operations on this object */
-#ifdef CONFIG_FSCACHE_OBJECT_LIST
- struct rb_node objlist_link; /* link in global object list */
-#endif
pgoff_t store_limit; /* current storage limit */
loff_t store_limit_l; /* current storage limit */
};


2021-06-21 21:47:18

by David Howells

[permalink] [raw]
Subject: [PATCH 09/12] cachefiles: Change %p in format strings to something else

Change plain %p in format strings in cachefiles code to something more
useful, since %p is now hashed before printing and thus no longer matches
the contents of an oops register dump.

Signed-off-by: David Howells <[email protected]>
---

fs/cachefiles/bind.c | 2 --
fs/cachefiles/interface.c | 6 +++---
fs/cachefiles/key.c | 2 +-
fs/cachefiles/namei.c | 48 +++++++++++++++++++++------------------------
fs/cachefiles/xattr.c | 4 ++--
5 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/fs/cachefiles/bind.c b/fs/cachefiles/bind.c
index 38bb7764b454..d463d89f5db8 100644
--- a/fs/cachefiles/bind.c
+++ b/fs/cachefiles/bind.c
@@ -108,8 +108,6 @@ static int cachefiles_daemon_add_cache(struct cachefiles_cache *cache)
atomic_set(&fsdef->usage, 1);
fsdef->type = FSCACHE_COOKIE_TYPE_INDEX;

- _debug("- fsdef %p", fsdef);
-
/* look up the directory at the root of the cache */
ret = kern_path(cache->rootdirname, LOOKUP_DIRECTORY, &path);
if (ret < 0)
diff --git a/fs/cachefiles/interface.c b/fs/cachefiles/interface.c
index da3948fdb615..da28ac1fa225 100644
--- a/fs/cachefiles/interface.c
+++ b/fs/cachefiles/interface.c
@@ -33,7 +33,7 @@ static struct fscache_object *cachefiles_alloc_object(

cache = container_of(_cache, struct cachefiles_cache, cache);

- _enter("{%s},%p,", cache->cache.identifier, cookie);
+ _enter("{%s},%x,", cache->cache.identifier, cookie->debug_id);

lookup_data = kmalloc(sizeof(*lookup_data), cachefiles_gfp);
if (!lookup_data)
@@ -96,7 +96,7 @@ static struct fscache_object *cachefiles_alloc_object(
lookup_data->key = key;
object->lookup_data = lookup_data;

- _leave(" = %p [%p]", &object->fscache, lookup_data);
+ _leave(" = %x [%p]", object->fscache.debug_id, lookup_data);
return &object->fscache;

nomem_key:
@@ -379,7 +379,7 @@ static void cachefiles_sync_cache(struct fscache_cache *_cache)
const struct cred *saved_cred;
int ret;

- _enter("%p", _cache);
+ _enter("%s", _cache->tag->name);

cache = container_of(_cache, struct cachefiles_cache, cache);

diff --git a/fs/cachefiles/key.c b/fs/cachefiles/key.c
index be96f5fc5cac..7f94efc97e23 100644
--- a/fs/cachefiles/key.c
+++ b/fs/cachefiles/key.c
@@ -150,6 +150,6 @@ char *cachefiles_cook_key(const u8 *raw, int keylen, uint8_t type)
key[len++] = 0;
key[len] = 0;

- _leave(" = %p %d", key, len);
+ _leave(" = %s %d", key, len);
return key;
}
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 92aa550dae7e..a9aca5ab5970 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -39,18 +39,18 @@ void __cachefiles_printk_object(struct cachefiles_object *object,
pr_err("%sops=%u inp=%u exc=%u\n",
prefix, object->fscache.n_ops, object->fscache.n_in_progress,
object->fscache.n_exclusive);
- pr_err("%sparent=%p\n",
- prefix, object->fscache.parent);
+ pr_err("%sparent=%x\n",
+ prefix, object->fscache.parent ? object->fscache.parent->debug_id : 0);

spin_lock(&object->fscache.lock);
cookie = object->fscache.cookie;
if (cookie) {
- pr_err("%scookie=%p [pr=%p nd=%p fl=%lx]\n",
+ pr_err("%scookie=%x [pr=%x nd=%p fl=%lx]\n",
prefix,
- object->fscache.cookie,
- object->fscache.cookie->parent,
- object->fscache.cookie->netfs_data,
- object->fscache.cookie->flags);
+ cookie->debug_id,
+ cookie->parent ? cookie->parent->debug_id : 0,
+ cookie->netfs_data,
+ cookie->flags);
pr_err("%skey=[%u] '", prefix, cookie->key_len);
k = (cookie->key_len <= sizeof(cookie->inline_key)) ?
cookie->inline_key : cookie->key;
@@ -110,7 +110,7 @@ static void cachefiles_mark_object_buried(struct cachefiles_cache *cache,

/* found the dentry for */
found_dentry:
- kdebug("preemptive burial: OBJ%x [%s] %p",
+ kdebug("preemptive burial: OBJ%x [%s] %pd",
object->fscache.debug_id,
object->fscache.state->name,
dentry);
@@ -140,7 +140,7 @@ static int cachefiles_mark_object_active(struct cachefiles_cache *cache,
struct rb_node **_p, *_parent = NULL;
struct dentry *dentry;

- _enter(",%p", object);
+ _enter(",%x", object->fscache.debug_id);

try_again:
write_lock(&cache->active_lock);
@@ -298,8 +298,6 @@ static int cachefiles_bury_object(struct cachefiles_cache *cache,

_enter(",'%pd','%pd'", dir, rep);

- _debug("remove %p from %p", rep, dir);
-
/* non-directories can just be unlinked */
if (!d_is_dir(rep)) {
_debug("unlink stale object");
@@ -446,7 +444,7 @@ int cachefiles_delete_object(struct cachefiles_cache *cache,
struct dentry *dir;
int ret;

- _enter(",OBJ%x{%p}", object->fscache.debug_id, object->dentry);
+ _enter(",OBJ%x{%pd}", object->fscache.debug_id, object->dentry);

ASSERT(object->dentry);
ASSERT(d_backing_inode(object->dentry));
@@ -499,7 +497,7 @@ int cachefiles_walk_to_object(struct cachefiles_object *parent,
const char *name;
int ret, nlen;

- _enter("OBJ%x{%p},OBJ%x,%s,",
+ _enter("OBJ%x{%pd},OBJ%x,%s,",
parent->fscache.debug_id, parent->dentry,
object->fscache.debug_id, key);

@@ -542,7 +540,7 @@ int cachefiles_walk_to_object(struct cachefiles_object *parent,

inode = d_backing_inode(next);
trace_cachefiles_lookup(object, next, inode);
- _debug("next -> %p %s", next, inode ? "positive" : "negative");
+ _debug("next -> %pd %s", next, inode ? "positive" : "negative");

if (!key)
object->new = !inode;
@@ -578,8 +576,8 @@ int cachefiles_walk_to_object(struct cachefiles_object *parent,
}
ASSERT(d_backing_inode(next));

- _debug("mkdir -> %p{%p{ino=%lu}}",
- next, d_backing_inode(next), d_backing_inode(next)->i_ino);
+ _debug("mkdir -> %pd{ino=%lu}",
+ next, d_backing_inode(next)->i_ino);

} else if (!d_can_lookup(next)) {
pr_err("inode %lu is not a directory\n",
@@ -607,8 +605,8 @@ int cachefiles_walk_to_object(struct cachefiles_object *parent,

ASSERT(d_backing_inode(next));

- _debug("create -> %p{%p{ino=%lu}}",
- next, d_backing_inode(next), d_backing_inode(next)->i_ino);
+ _debug("create -> %pd{ino=%lu}",
+ next, d_backing_inode(next)->i_ino);

} else if (!d_can_lookup(next) &&
!d_is_reg(next)
@@ -774,7 +772,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
goto lookup_error;
}

- _debug("subdir -> %p %s",
+ _debug("subdir -> %pd %s",
subdir, d_backing_inode(subdir) ? "positive" : "negative");

/* we need to create the subdir if it doesn't exist yet */
@@ -800,10 +798,8 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
}
ASSERT(d_backing_inode(subdir));

- _debug("mkdir -> %p{%p{ino=%lu}}",
- subdir,
- d_backing_inode(subdir),
- d_backing_inode(subdir)->i_ino);
+ _debug("mkdir -> %pd{ino=%lu}",
+ subdir, d_backing_inode(subdir)->i_ino);
}

inode_unlock(d_inode(dir));
@@ -878,7 +874,7 @@ static struct dentry *cachefiles_check_active(struct cachefiles_cache *cache,
if (IS_ERR(victim))
goto lookup_error;

- //_debug("victim -> %p %s",
+ //_debug("victim -> %pd %s",
// victim, d_backing_inode(victim) ? "positive" : "negative");

/* if the object is no longer there then we probably retired the object
@@ -909,7 +905,7 @@ static struct dentry *cachefiles_check_active(struct cachefiles_cache *cache,

read_unlock(&cache->active_lock);

- //_leave(" = %p", victim);
+ //_leave(" = %pd", victim);
return victim;

object_in_use:
@@ -955,7 +951,7 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir,
if (IS_ERR(victim))
return PTR_ERR(victim);

- _debug("victim -> %p %s",
+ _debug("victim -> %pd %s",
victim, d_backing_inode(victim) ? "positive" : "negative");

/* okay... the victim is not being used so we can cull it
diff --git a/fs/cachefiles/xattr.c b/fs/cachefiles/xattr.c
index a591b5e09637..9e82de668595 100644
--- a/fs/cachefiles/xattr.c
+++ b/fs/cachefiles/xattr.c
@@ -36,7 +36,7 @@ int cachefiles_check_object_type(struct cachefiles_object *object)
else
snprintf(type, 3, "%02x", object->fscache.cookie->def->type);

- _enter("%p{%s}", object, type);
+ _enter("%x{%s}", object->fscache.debug_id, type);

/* attempt to install a type label directly */
ret = vfs_setxattr(&init_user_ns, dentry, cachefiles_xattr_cache, type,
@@ -134,7 +134,7 @@ int cachefiles_update_object_xattr(struct cachefiles_object *object,
if (!dentry)
return -ESTALE;

- _enter("%p,#%d", object, auxdata->len);
+ _enter("%x,#%d", object->fscache.debug_id, auxdata->len);

/* attempt to install the cache metadata directly */
_debug("SET #%u", auxdata->len);


2021-06-21 21:47:22

by David Howells

[permalink] [raw]
Subject: [PATCH 08/12] fscache: Change %p in format strings to something else

Change plain %p in format strings in fscache code to something more useful,
since %p is now hashed before printing and thus no longer matches the
contents of an oops register dump.

Signed-off-by: David Howells <[email protected]>
---

fs/fscache/cache.c | 8 ++++----
fs/fscache/cookie.c | 16 +++++++---------
fs/fscache/object.c | 1 -
3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/fs/fscache/cache.c b/fs/fscache/cache.c
index 8a6ffcac867f..e7a5d7ab4085 100644
--- a/fs/fscache/cache.c
+++ b/fs/fscache/cache.c
@@ -116,7 +116,7 @@ struct fscache_cache *fscache_select_cache_for_object(
cache = NULL;

spin_unlock(&cookie->lock);
- _leave(" = %p [parent]", cache);
+ _leave(" = %s [parent]", cache ? cache->tag->name : "NULL");
return cache;
}

@@ -152,14 +152,14 @@ struct fscache_cache *fscache_select_cache_for_object(
if (test_bit(FSCACHE_IOERROR, &tag->cache->flags))
return NULL;

- _leave(" = %p [specific]", tag->cache);
+ _leave(" = %s [specific]", tag->name);
return tag->cache;

no_preference:
/* netfs has no preference - just select first cache */
cache = list_entry(fscache_cache_list.next,
struct fscache_cache, link);
- _leave(" = %p [first]", cache);
+ _leave(" = %s [first]", cache->tag->name);
return cache;
}

@@ -334,7 +334,7 @@ static void fscache_withdraw_all_objects(struct fscache_cache *cache,
struct fscache_object, cache_link);
list_move_tail(&object->cache_link, dying_objects);

- _debug("withdraw %p", object->cookie);
+ _debug("withdraw %x", object->cookie->debug_id);

/* This must be done under object_list_lock to prevent
* a race with fscache_drop_object().
diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
index 2f4d5271ad2e..ec9bce33085f 100644
--- a/fs/fscache/cookie.c
+++ b/fs/fscache/cookie.c
@@ -375,7 +375,7 @@ void __fscache_enable_cookie(struct fscache_cookie *cookie,
bool (*can_enable)(void *data),
void *data)
{
- _enter("%p", cookie);
+ _enter("%x", cookie->debug_id);

trace_fscache_enable(cookie);

@@ -472,10 +472,8 @@ static int fscache_acquire_non_index_cookie(struct fscache_cookie *cookie,

/* we may be required to wait for lookup to complete at this point */
if (!fscache_defer_lookup) {
- _debug("non-deferred lookup %p", &cookie->flags);
wait_on_bit(&cookie->flags, FSCACHE_COOKIE_LOOKING_UP,
TASK_UNINTERRUPTIBLE);
- _debug("complete");
if (test_bit(FSCACHE_COOKIE_UNAVAILABLE, &cookie->flags))
goto unavailable;
}
@@ -500,7 +498,7 @@ static int fscache_alloc_object(struct fscache_cache *cache,
struct fscache_object *object;
int ret;

- _enter("%p,%p{%s}", cache, cookie, cookie->def->name);
+ _enter("%s,%x{%s}", cache->tag->name, cookie->debug_id, cookie->def->name);

spin_lock(&cookie->lock);
hlist_for_each_entry(object, &cookie->backing_objects,
@@ -676,7 +674,7 @@ EXPORT_SYMBOL(__fscache_invalidate);
*/
void __fscache_wait_on_invalidate(struct fscache_cookie *cookie)
{
- _enter("%p", cookie);
+ _enter("%x", cookie->debug_id);

wait_on_bit(&cookie->flags, FSCACHE_COOKIE_INVALIDATING,
TASK_UNINTERRUPTIBLE);
@@ -731,7 +729,7 @@ void __fscache_disable_cookie(struct fscache_cookie *cookie,
struct fscache_object *object;
bool awaken = false;

- _enter("%p,%u", cookie, invalidate);
+ _enter("%x,%u", cookie->debug_id, invalidate);

trace_fscache_disable(cookie);

@@ -821,8 +819,8 @@ void __fscache_relinquish_cookie(struct fscache_cookie *cookie,
return;
}

- _enter("%p{%s,%p,%d},%d",
- cookie, cookie->def->name, cookie->netfs_data,
+ _enter("%x{%s,%d},%d",
+ cookie->debug_id, cookie->def->name,
atomic_read(&cookie->n_active), retire);

trace_fscache_relinquish(cookie, retire);
@@ -877,7 +875,7 @@ void fscache_cookie_put(struct fscache_cookie *cookie,
struct fscache_cookie *parent;
int usage;

- _enter("%p", cookie);
+ _enter("%x", cookie->debug_id);

do {
usage = atomic_dec_return(&cookie->usage);
diff --git a/fs/fscache/object.c b/fs/fscache/object.c
index b3853274733f..f346a78f4bd6 100644
--- a/fs/fscache/object.c
+++ b/fs/fscache/object.c
@@ -518,7 +518,6 @@ void fscache_object_lookup_negative(struct fscache_object *object)
set_bit(FSCACHE_COOKIE_NO_DATA_YET, &cookie->flags);
clear_bit(FSCACHE_COOKIE_UNAVAILABLE, &cookie->flags);

- _debug("wake up lookup %p", &cookie->flags);
clear_bit_unlock(FSCACHE_COOKIE_LOOKING_UP, &cookie->flags);
wake_up_bit(&cookie->flags, FSCACHE_COOKIE_LOOKING_UP);
}


2021-06-21 21:47:23

by David Howells

[permalink] [raw]
Subject: [PATCH 06/12] fscache, cachefiles: Remove the histogram stuff

Remove the histogram stuff as it's mostly going to be outdated.

Signed-off-by: David Howells <[email protected]>
---

fs/cachefiles/Kconfig | 19 -------
fs/cachefiles/Makefile | 2 -
fs/cachefiles/internal.h | 25 ---------
fs/cachefiles/main.c | 7 ---
fs/cachefiles/namei.c | 13 -----
fs/cachefiles/proc.c | 114 -----------------------------------------
fs/fscache/Kconfig | 17 ------
fs/fscache/Makefile | 1
fs/fscache/histogram.c | 87 -------------------------------
fs/fscache/internal.h | 24 ---------
fs/fscache/object.c | 5 --
fs/fscache/operation.c | 3 -
fs/fscache/page.c | 6 --
fs/fscache/proc.c | 13 -----
include/linux/fscache-cache.h | 1
15 files changed, 337 deletions(-)
delete mode 100644 fs/cachefiles/proc.c
delete mode 100644 fs/fscache/histogram.c

diff --git a/fs/cachefiles/Kconfig b/fs/cachefiles/Kconfig
index ff9ca55a9ae9..6827b40f7ddc 100644
--- a/fs/cachefiles/Kconfig
+++ b/fs/cachefiles/Kconfig
@@ -19,22 +19,3 @@ config CACHEFILES_DEBUG
caching on files module. If this is set, the debugging output may be
enabled by setting bits in /sys/modules/cachefiles/parameter/debug or
by including a debugging specifier in /etc/cachefilesd.conf.
-
-config CACHEFILES_HISTOGRAM
- bool "Gather latency information on CacheFiles"
- depends on CACHEFILES && PROC_FS
- help
-
- This option causes latency information to be gathered on CacheFiles
- operation and exported through file:
-
- /proc/fs/cachefiles/histogram
-
- The generation of this histogram adds a certain amount of overhead to
- execution as there are a number of points at which data is gathered,
- and on a multi-CPU system these may be on cachelines that keep
- bouncing between CPUs. On the other hand, the histogram may be
- useful for debugging purposes. Saying 'N' here is recommended.
-
- See Documentation/filesystems/caching/cachefiles.rst for more
- information.
diff --git a/fs/cachefiles/Makefile b/fs/cachefiles/Makefile
index 2227dc2d5498..02fd17731769 100644
--- a/fs/cachefiles/Makefile
+++ b/fs/cachefiles/Makefile
@@ -15,6 +15,4 @@ cachefiles-y := \
security.o \
xattr.o

-cachefiles-$(CONFIG_CACHEFILES_HISTOGRAM) += proc.o
-
obj-$(CONFIG_CACHEFILES) := cachefiles.o
diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h
index 4ed83aa5253b..0a511c36dab8 100644
--- a/fs/cachefiles/internal.h
+++ b/fs/cachefiles/internal.h
@@ -180,31 +180,6 @@ extern int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir,
extern int cachefiles_check_in_use(struct cachefiles_cache *cache,
struct dentry *dir, char *filename);

-/*
- * proc.c
- */
-#ifdef CONFIG_CACHEFILES_HISTOGRAM
-extern atomic_t cachefiles_lookup_histogram[HZ];
-extern atomic_t cachefiles_mkdir_histogram[HZ];
-extern atomic_t cachefiles_create_histogram[HZ];
-
-extern int __init cachefiles_proc_init(void);
-extern void cachefiles_proc_cleanup(void);
-static inline
-void cachefiles_hist(atomic_t histogram[], unsigned long start_jif)
-{
- unsigned long jif = jiffies - start_jif;
- if (jif >= HZ)
- jif = HZ - 1;
- atomic_inc(&histogram[jif]);
-}
-
-#else
-#define cachefiles_proc_init() (0)
-#define cachefiles_proc_cleanup() do {} while (0)
-#define cachefiles_hist(hist, start_jif) do {} while (0)
-#endif
-
/*
* rdwr.c
*/
diff --git a/fs/cachefiles/main.c b/fs/cachefiles/main.c
index ddf0cd58d60c..9c8d34c49b12 100644
--- a/fs/cachefiles/main.c
+++ b/fs/cachefiles/main.c
@@ -69,15 +69,9 @@ static int __init cachefiles_init(void)
goto error_object_jar;
}

- ret = cachefiles_proc_init();
- if (ret < 0)
- goto error_proc;
-
pr_info("Loaded\n");
return 0;

-error_proc:
- kmem_cache_destroy(cachefiles_object_jar);
error_object_jar:
misc_deregister(&cachefiles_dev);
error_dev:
@@ -94,7 +88,6 @@ static void __exit cachefiles_exit(void)
{
pr_info("Unloading\n");

- cachefiles_proc_cleanup();
kmem_cache_destroy(cachefiles_object_jar);
misc_deregister(&cachefiles_dev);
}
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 7bf0732ae25c..92aa550dae7e 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -496,7 +496,6 @@ int cachefiles_walk_to_object(struct cachefiles_object *parent,
struct dentry *dir, *next = NULL;
struct inode *inode;
struct path path;
- unsigned long start;
const char *name;
int ret, nlen;

@@ -535,9 +534,7 @@ int cachefiles_walk_to_object(struct cachefiles_object *parent,

inode_lock_nested(d_inode(dir), I_MUTEX_PARENT);

- start = jiffies;
next = lookup_one_len(name, dir, nlen);
- cachefiles_hist(cachefiles_lookup_histogram, start);
if (IS_ERR(next)) {
trace_cachefiles_lookup(object, next, NULL);
goto lookup_error;
@@ -568,9 +565,7 @@ int cachefiles_walk_to_object(struct cachefiles_object *parent,
ret = security_path_mkdir(&path, next, 0);
if (ret < 0)
goto create_error;
- start = jiffies;
ret = vfs_mkdir(&init_user_ns, d_inode(dir), next, 0);
- cachefiles_hist(cachefiles_mkdir_histogram, start);
if (!key)
trace_cachefiles_mkdir(object, next, ret);
if (ret < 0)
@@ -604,10 +599,8 @@ int cachefiles_walk_to_object(struct cachefiles_object *parent,
ret = security_path_mknod(&path, next, S_IFREG, 0);
if (ret < 0)
goto create_error;
- start = jiffies;
ret = vfs_create(&init_user_ns, d_inode(dir), next,
S_IFREG, true);
- cachefiles_hist(cachefiles_create_histogram, start);
trace_cachefiles_create(object, next, ret);
if (ret < 0)
goto create_error;
@@ -765,7 +758,6 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
const char *dirname)
{
struct dentry *subdir;
- unsigned long start;
struct path path;
int ret;

@@ -775,9 +767,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
inode_lock(d_inode(dir));

retry:
- start = jiffies;
subdir = lookup_one_len(dirname, dir, strlen(dirname));
- cachefiles_hist(cachefiles_lookup_histogram, start);
if (IS_ERR(subdir)) {
if (PTR_ERR(subdir) == -ENOMEM)
goto nomem_d_alloc;
@@ -876,7 +866,6 @@ static struct dentry *cachefiles_check_active(struct cachefiles_cache *cache,
struct cachefiles_object *object;
struct rb_node *_n;
struct dentry *victim;
- unsigned long start;
int ret;

//_enter(",%pd/,%s",
@@ -885,9 +874,7 @@ static struct dentry *cachefiles_check_active(struct cachefiles_cache *cache,
/* look up the victim */
inode_lock_nested(d_inode(dir), I_MUTEX_PARENT);

- start = jiffies;
victim = lookup_one_len(filename, dir, strlen(filename));
- cachefiles_hist(cachefiles_lookup_histogram, start);
if (IS_ERR(victim))
goto lookup_error;

diff --git a/fs/cachefiles/proc.c b/fs/cachefiles/proc.c
deleted file mode 100644
index 6e67aea0f24e..000000000000
--- a/fs/cachefiles/proc.c
+++ /dev/null
@@ -1,114 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/* CacheFiles statistics
- *
- * Copyright (C) 2007 Red Hat, Inc. All Rights Reserved.
- * Written by David Howells ([email protected])
- */
-
-#include <linux/module.h>
-#include <linux/proc_fs.h>
-#include <linux/seq_file.h>
-#include "internal.h"
-
-atomic_t cachefiles_lookup_histogram[HZ];
-atomic_t cachefiles_mkdir_histogram[HZ];
-atomic_t cachefiles_create_histogram[HZ];
-
-/*
- * display the latency histogram
- */
-static int cachefiles_histogram_show(struct seq_file *m, void *v)
-{
- unsigned long index;
- unsigned x, y, z, t;
-
- switch ((unsigned long) v) {
- case 1:
- seq_puts(m, "JIFS SECS LOOKUPS MKDIRS CREATES\n");
- return 0;
- case 2:
- seq_puts(m, "===== ===== ========= ========= =========\n");
- return 0;
- default:
- index = (unsigned long) v - 3;
- x = atomic_read(&cachefiles_lookup_histogram[index]);
- y = atomic_read(&cachefiles_mkdir_histogram[index]);
- z = atomic_read(&cachefiles_create_histogram[index]);
- if (x == 0 && y == 0 && z == 0)
- return 0;
-
- t = (index * 1000) / HZ;
-
- seq_printf(m, "%4lu 0.%03u %9u %9u %9u\n", index, t, x, y, z);
- return 0;
- }
-}
-
-/*
- * set up the iterator to start reading from the first line
- */
-static void *cachefiles_histogram_start(struct seq_file *m, loff_t *_pos)
-{
- if ((unsigned long long)*_pos >= HZ + 2)
- return NULL;
- if (*_pos == 0)
- *_pos = 1;
- return (void *)(unsigned long) *_pos;
-}
-
-/*
- * move to the next line
- */
-static void *cachefiles_histogram_next(struct seq_file *m, void *v, loff_t *pos)
-{
- (*pos)++;
- return (unsigned long long)*pos > HZ + 2 ?
- NULL : (void *)(unsigned long) *pos;
-}
-
-/*
- * clean up after reading
- */
-static void cachefiles_histogram_stop(struct seq_file *m, void *v)
-{
-}
-
-static const struct seq_operations cachefiles_histogram_ops = {
- .start = cachefiles_histogram_start,
- .stop = cachefiles_histogram_stop,
- .next = cachefiles_histogram_next,
- .show = cachefiles_histogram_show,
-};
-
-/*
- * initialise the /proc/fs/cachefiles/ directory
- */
-int __init cachefiles_proc_init(void)
-{
- _enter("");
-
- if (!proc_mkdir("fs/cachefiles", NULL))
- goto error_dir;
-
- if (!proc_create_seq("fs/cachefiles/histogram", S_IFREG | 0444, NULL,
- &cachefiles_histogram_ops))
- goto error_histogram;
-
- _leave(" = 0");
- return 0;
-
-error_histogram:
- remove_proc_entry("fs/cachefiles", NULL);
-error_dir:
- _leave(" = -ENOMEM");
- return -ENOMEM;
-}
-
-/*
- * clean up the /proc/fs/cachefiles/ directory
- */
-void cachefiles_proc_cleanup(void)
-{
- remove_proc_entry("fs/cachefiles/histogram", NULL);
- remove_proc_entry("fs/cachefiles", NULL);
-}
diff --git a/fs/fscache/Kconfig b/fs/fscache/Kconfig
index 92c87d8e0913..5e3a5b3f950d 100644
--- a/fs/fscache/Kconfig
+++ b/fs/fscache/Kconfig
@@ -29,23 +29,6 @@ config FSCACHE_STATS

See Documentation/filesystems/caching/fscache.rst for more information.

-config FSCACHE_HISTOGRAM
- bool "Gather latency information on local caching"
- depends on FSCACHE && PROC_FS
- help
- This option causes latency information to be gathered on local
- caching and exported through file:
-
- /proc/fs/fscache/histogram
-
- The generation of this histogram adds a certain amount of overhead to
- execution as there are a number of points at which data is gathered,
- and on a multi-CPU system these may be on cachelines that keep
- bouncing between CPUs. On the other hand, the histogram may be
- useful for debugging purposes. Saying 'N' here is recommended.
-
- See Documentation/filesystems/caching/fscache.rst for more information.
-
config FSCACHE_DEBUG
bool "Debug FS-Cache"
depends on FSCACHE
diff --git a/fs/fscache/Makefile b/fs/fscache/Makefile
index 3b2ffa93ac18..45d5235a449b 100644
--- a/fs/fscache/Makefile
+++ b/fs/fscache/Makefile
@@ -16,7 +16,6 @@ fscache-y := \

fscache-$(CONFIG_PROC_FS) += proc.o
fscache-$(CONFIG_FSCACHE_STATS) += stats.o
-fscache-$(CONFIG_FSCACHE_HISTOGRAM) += histogram.o
fscache-$(CONFIG_FSCACHE_OBJECT_LIST) += object-list.o

obj-$(CONFIG_FSCACHE) := fscache.o
diff --git a/fs/fscache/histogram.c b/fs/fscache/histogram.c
deleted file mode 100644
index 4e5beeaaf454..000000000000
--- a/fs/fscache/histogram.c
+++ /dev/null
@@ -1,87 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/* FS-Cache latency histogram
- *
- * Copyright (C) 2008 Red Hat, Inc. All Rights Reserved.
- * Written by David Howells ([email protected])
- */
-
-#define FSCACHE_DEBUG_LEVEL THREAD
-#include <linux/module.h>
-#include <linux/proc_fs.h>
-#include <linux/seq_file.h>
-#include "internal.h"
-
-atomic_t fscache_obj_instantiate_histogram[HZ];
-atomic_t fscache_objs_histogram[HZ];
-atomic_t fscache_ops_histogram[HZ];
-atomic_t fscache_retrieval_delay_histogram[HZ];
-atomic_t fscache_retrieval_histogram[HZ];
-
-/*
- * display the time-taken histogram
- */
-static int fscache_histogram_show(struct seq_file *m, void *v)
-{
- unsigned long index;
- unsigned n[5], t;
-
- switch ((unsigned long) v) {
- case 1:
- seq_puts(m, "JIFS SECS OBJ INST OP RUNS OBJ RUNS RETRV DLY RETRIEVLS\n");
- return 0;
- case 2:
- seq_puts(m, "===== ===== ========= ========= ========= ========= =========\n");
- return 0;
- default:
- index = (unsigned long) v - 3;
- n[0] = atomic_read(&fscache_obj_instantiate_histogram[index]);
- n[1] = atomic_read(&fscache_ops_histogram[index]);
- n[2] = atomic_read(&fscache_objs_histogram[index]);
- n[3] = atomic_read(&fscache_retrieval_delay_histogram[index]);
- n[4] = atomic_read(&fscache_retrieval_histogram[index]);
- if (!(n[0] | n[1] | n[2] | n[3] | n[4]))
- return 0;
-
- t = (index * 1000) / HZ;
-
- seq_printf(m, "%4lu 0.%03u %9u %9u %9u %9u %9u\n",
- index, t, n[0], n[1], n[2], n[3], n[4]);
- return 0;
- }
-}
-
-/*
- * set up the iterator to start reading from the first line
- */
-static void *fscache_histogram_start(struct seq_file *m, loff_t *_pos)
-{
- if ((unsigned long long)*_pos >= HZ + 2)
- return NULL;
- if (*_pos == 0)
- *_pos = 1;
- return (void *)(unsigned long) *_pos;
-}
-
-/*
- * move to the next line
- */
-static void *fscache_histogram_next(struct seq_file *m, void *v, loff_t *pos)
-{
- (*pos)++;
- return (unsigned long long)*pos > HZ + 2 ?
- NULL : (void *)(unsigned long) *pos;
-}
-
-/*
- * clean up after reading
- */
-static void fscache_histogram_stop(struct seq_file *m, void *v)
-{
-}
-
-const struct seq_operations fscache_histogram_ops = {
- .start = fscache_histogram_start,
- .stop = fscache_histogram_stop,
- .next = fscache_histogram_next,
- .show = fscache_histogram_show,
-};
diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
index 207a6bc81ca9..796678b2b32a 100644
--- a/fs/fscache/internal.h
+++ b/fs/fscache/internal.h
@@ -63,30 +63,6 @@ extern void fscache_cookie_put(struct fscache_cookie *,
extern struct fscache_cookie fscache_fsdef_index;
extern struct fscache_cookie_def fscache_fsdef_netfs_def;

-/*
- * histogram.c
- */
-#ifdef CONFIG_FSCACHE_HISTOGRAM
-extern atomic_t fscache_obj_instantiate_histogram[HZ];
-extern atomic_t fscache_objs_histogram[HZ];
-extern atomic_t fscache_ops_histogram[HZ];
-extern atomic_t fscache_retrieval_delay_histogram[HZ];
-extern atomic_t fscache_retrieval_histogram[HZ];
-
-static inline void fscache_hist(atomic_t histogram[], unsigned long start_jif)
-{
- unsigned long jif = jiffies - start_jif;
- if (jif >= HZ)
- jif = HZ - 1;
- atomic_inc(&histogram[jif]);
-}
-
-extern const struct seq_operations fscache_histogram_ops;
-
-#else
-#define fscache_hist(hist, start_jif) do {} while (0)
-#endif
-
/*
* main.c
*/
diff --git a/fs/fscache/object.c b/fs/fscache/object.c
index cb2146e02cd5..5dbaab2e1262 100644
--- a/fs/fscache/object.c
+++ b/fs/fscache/object.c
@@ -277,13 +277,10 @@ static void fscache_object_work_func(struct work_struct *work)
{
struct fscache_object *object =
container_of(work, struct fscache_object, work);
- unsigned long start;

_enter("{OBJ%x}", object->debug_id);

- start = jiffies;
fscache_object_sm_dispatcher(object);
- fscache_hist(fscache_objs_histogram, start);
fscache_put_object(object, fscache_obj_put_work);
}

@@ -436,7 +433,6 @@ static const struct fscache_state *fscache_parent_ready(struct fscache_object *o
spin_lock(&parent->lock);
parent->n_ops++;
parent->n_obj_ops++;
- object->lookup_jif = jiffies;
spin_unlock(&parent->lock);

_leave("");
@@ -596,7 +592,6 @@ static const struct fscache_state *fscache_object_available(struct fscache_objec
object->cache->ops->lookup_complete(object);
fscache_stat_d(&fscache_n_cop_lookup_complete);

- fscache_hist(fscache_obj_instantiate_histogram, object->lookup_jif);
fscache_stat(&fscache_n_object_avail);

_leave("");
diff --git a/fs/fscache/operation.c b/fs/fscache/operation.c
index 4a5651d4904e..433877107700 100644
--- a/fs/fscache/operation.c
+++ b/fs/fscache/operation.c
@@ -616,7 +616,6 @@ void fscache_op_work_func(struct work_struct *work)
{
struct fscache_operation *op =
container_of(work, struct fscache_operation, work);
- unsigned long start;

_enter("{OBJ%x OP%x,%d}",
op->object->debug_id, op->debug_id, atomic_read(&op->usage));
@@ -624,9 +623,7 @@ void fscache_op_work_func(struct work_struct *work)
trace_fscache_op(op->object->cookie, op, fscache_op_work);

ASSERT(op->processor != NULL);
- start = jiffies;
op->processor(op);
- fscache_hist(fscache_ops_histogram, start);
fscache_put_operation(op);

_leave("");
diff --git a/fs/fscache/page.c b/fs/fscache/page.c
index 991b0a871744..27df94ef0e0b 100644
--- a/fs/fscache/page.c
+++ b/fs/fscache/page.c
@@ -289,7 +289,6 @@ static void fscache_release_retrieval_op(struct fscache_operation *_op)
ASSERTIFCMP(op->op.state != FSCACHE_OP_ST_INITIALISED,
atomic_read(&op->n_pages), ==, 0);

- fscache_hist(fscache_retrieval_histogram, op->start_time);
if (op->context)
fscache_put_context(op->cookie, op->context);

@@ -324,7 +323,6 @@ struct fscache_retrieval *fscache_alloc_retrieval(
op->mapping = mapping;
op->end_io_func = end_io_func;
op->context = context;
- op->start_time = jiffies;
INIT_LIST_HEAD(&op->to_do);

/* Pin the netfs read context in case we need to do the actual netfs
@@ -340,8 +338,6 @@ struct fscache_retrieval *fscache_alloc_retrieval(
*/
int fscache_wait_for_deferred_lookup(struct fscache_cookie *cookie)
{
- unsigned long jif;
-
_enter("");

if (!test_bit(FSCACHE_COOKIE_LOOKING_UP, &cookie->flags)) {
@@ -351,7 +347,6 @@ int fscache_wait_for_deferred_lookup(struct fscache_cookie *cookie)

fscache_stat(&fscache_n_retrievals_wait);

- jif = jiffies;
if (wait_on_bit(&cookie->flags, FSCACHE_COOKIE_LOOKING_UP,
TASK_INTERRUPTIBLE) != 0) {
fscache_stat(&fscache_n_retrievals_intr);
@@ -362,7 +357,6 @@ int fscache_wait_for_deferred_lookup(struct fscache_cookie *cookie)
ASSERT(!test_bit(FSCACHE_COOKIE_LOOKING_UP, &cookie->flags));

smp_rmb();
- fscache_hist(fscache_retrieval_delay_histogram, jif);
_leave(" = 0 [dly]");
return 0;
}
diff --git a/fs/fscache/proc.c b/fs/fscache/proc.c
index da51fdfc8641..061df8f61ffc 100644
--- a/fs/fscache/proc.c
+++ b/fs/fscache/proc.c
@@ -31,12 +31,6 @@ int __init fscache_proc_init(void)
goto error_stats;
#endif

-#ifdef CONFIG_FSCACHE_HISTOGRAM
- if (!proc_create_seq("fs/fscache/histogram", S_IFREG | 0444, NULL,
- &fscache_histogram_ops))
- goto error_histogram;
-#endif
-
#ifdef CONFIG_FSCACHE_OBJECT_LIST
if (!proc_create("fs/fscache/objects", S_IFREG | 0444, NULL,
&fscache_objlist_proc_ops))
@@ -49,10 +43,6 @@ int __init fscache_proc_init(void)
#ifdef CONFIG_FSCACHE_OBJECT_LIST
error_objects:
#endif
-#ifdef CONFIG_FSCACHE_HISTOGRAM
- remove_proc_entry("fs/fscache/histogram", NULL);
-error_histogram:
-#endif
#ifdef CONFIG_FSCACHE_STATS
remove_proc_entry("fs/fscache/stats", NULL);
error_stats:
@@ -73,9 +63,6 @@ void fscache_proc_cleanup(void)
#ifdef CONFIG_FSCACHE_OBJECT_LIST
remove_proc_entry("fs/fscache/objects", NULL);
#endif
-#ifdef CONFIG_FSCACHE_HISTOGRAM
- remove_proc_entry("fs/fscache/histogram", NULL);
-#endif
#ifdef CONFIG_FSCACHE_STATS
remove_proc_entry("fs/fscache/stats", NULL);
#endif
diff --git a/include/linux/fscache-cache.h b/include/linux/fscache-cache.h
index 3235ddbdcc09..fbff0b7e3ef1 100644
--- a/include/linux/fscache-cache.h
+++ b/include/linux/fscache-cache.h
@@ -147,7 +147,6 @@ struct fscache_retrieval {
fscache_rw_complete_t end_io_func; /* function to call on I/O completion */
void *context; /* netfs read context (pinned) */
struct list_head to_do; /* list of things to be done by the backend */
- unsigned long start_time; /* time at which retrieval started */
atomic_t n_pages; /* number of pages to be retrieved */
};



2021-06-21 21:47:55

by David Howells

[permalink] [raw]
Subject: [PATCH 11/12] fscache: Fix fscache_cookie_put() to not deref after dec

fscache_cookie_put() accesses the cookie it has just put inside the
tracepoint that monitors the change - but this is something it's not
allowed to do if we didn't reduce the count to zero.

Fix this by dropping most of those values from the tracepoint and grabbing
the cookie debug ID before doing the dec.

Also take the opportunity to switch over the usage and where arguments on
the tracepoint to put the reason last.

Signed-off-by: David Howells <[email protected]>
---

fs/fscache/cookie.c | 10 ++++++----
fs/fscache/internal.h | 2 +-
fs/fscache/netfs.c | 2 +-
include/trace/events/fscache.h | 24 +++++++-----------------
4 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
index 2558814193e9..6df3732cf1b4 100644
--- a/fs/fscache/cookie.c
+++ b/fs/fscache/cookie.c
@@ -225,8 +225,8 @@ struct fscache_cookie *fscache_hash_cookie(struct fscache_cookie *candidate)

collision:
if (test_and_set_bit(FSCACHE_COOKIE_ACQUIRED, &cursor->flags)) {
- trace_fscache_cookie(cursor, fscache_cookie_collision,
- atomic_read(&cursor->usage));
+ trace_fscache_cookie(cursor->debug_id, atomic_read(&cursor->usage),
+ fscache_cookie_collision);
pr_err("Duplicate cookie detected\n");
fscache_print_cookie(cursor, 'O');
fscache_print_cookie(candidate, 'N');
@@ -305,7 +305,8 @@ struct fscache_cookie *__fscache_acquire_cookie(

cookie = fscache_hash_cookie(candidate);
if (!cookie) {
- trace_fscache_cookie(candidate, fscache_cookie_discard, 1);
+ trace_fscache_cookie(candidate->debug_id, 1,
+ fscache_cookie_discard);
goto out;
}

@@ -866,8 +867,9 @@ void fscache_cookie_put(struct fscache_cookie *cookie,
_enter("%x", cookie->debug_id);

do {
+ unsigned int cookie_debug_id = cookie->debug_id;
usage = atomic_dec_return(&cookie->usage);
- trace_fscache_cookie(cookie, where, usage);
+ trace_fscache_cookie(cookie_debug_id, usage, where);

if (usage > 0)
return;
diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
index a49136c63e4b..345105dbbfd1 100644
--- a/fs/fscache/internal.h
+++ b/fs/fscache/internal.h
@@ -291,7 +291,7 @@ static inline void fscache_cookie_get(struct fscache_cookie *cookie,
{
int usage = atomic_inc_return(&cookie->usage);

- trace_fscache_cookie(cookie, where, usage);
+ trace_fscache_cookie(cookie->debug_id, usage, where);
}

/*
diff --git a/fs/fscache/netfs.c b/fs/fscache/netfs.c
index cce92216fa28..d6bdb7b5e723 100644
--- a/fs/fscache/netfs.c
+++ b/fs/fscache/netfs.c
@@ -37,7 +37,7 @@ int __fscache_register_netfs(struct fscache_netfs *netfs)
if (!cookie)
goto already_registered;
if (cookie != candidate) {
- trace_fscache_cookie(candidate, fscache_cookie_discard, 1);
+ trace_fscache_cookie(candidate->debug_id, 1, fscache_cookie_discard);
fscache_free_cookie(candidate);
}

diff --git a/include/trace/events/fscache.h b/include/trace/events/fscache.h
index 0b9e058aba4d..55b8802740fa 100644
--- a/include/trace/events/fscache.h
+++ b/include/trace/events/fscache.h
@@ -160,37 +160,27 @@ fscache_cookie_traces;


TRACE_EVENT(fscache_cookie,
- TP_PROTO(struct fscache_cookie *cookie,
- enum fscache_cookie_trace where,
- int usage),
+ TP_PROTO(unsigned int cookie_debug_id,
+ int usage,
+ enum fscache_cookie_trace where),

- TP_ARGS(cookie, where, usage),
+ TP_ARGS(cookie_debug_id, usage, where),

TP_STRUCT__entry(
__field(unsigned int, cookie )
- __field(unsigned int, parent )
__field(enum fscache_cookie_trace, where )
__field(int, usage )
- __field(int, n_children )
- __field(int, n_active )
- __field(u8, flags )
),

TP_fast_assign(
- __entry->cookie = cookie->debug_id;
- __entry->parent = cookie->parent ? cookie->parent->debug_id : 0;
+ __entry->cookie = cookie_debug_id;
__entry->where = where;
__entry->usage = usage;
- __entry->n_children = atomic_read(&cookie->n_children);
- __entry->n_active = atomic_read(&cookie->n_active);
- __entry->flags = cookie->flags;
),

- TP_printk("%s c=%08x u=%d p=%08x Nc=%d Na=%d f=%02x",
+ TP_printk("%s c=%08x u=%d",
__print_symbolic(__entry->where, fscache_cookie_traces),
- __entry->cookie, __entry->usage,
- __entry->parent, __entry->n_children, __entry->n_active,
- __entry->flags)
+ __entry->cookie, __entry->usage)
);

TRACE_EVENT(fscache_netfs,


2021-06-21 21:48:36

by David Howells

[permalink] [raw]
Subject: [PATCH 10/12] fscache: Fix cookie key hashing

The current hash algorithm used for hashing cookie keys is really bad,
producing almost no dispersion (after a test kernel build, ~30000 files
were split over just 18 out of the 32768 hash buckets).

Borrow the full_name_hash() hash function into fscache to do the hashing
for cookie keys and, in the future, volume keys.

I don't want to use full_name_hash() as-is because I want the hash value to
be consistent across arches and over time as the hash value produced may
get used on disk.

I can also optimise parts of it away as the key will always be a padded
array of aligned 32-bit words.

Signed-off-by: David Howells <[email protected]>
---

fs/fscache/cookie.c | 14 +-------------
fs/fscache/internal.h | 2 ++
fs/fscache/main.c | 39 +++++++++++++++++++++++++++++++++++++++
3 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
index ec9bce33085f..2558814193e9 100644
--- a/fs/fscache/cookie.c
+++ b/fs/fscache/cookie.c
@@ -87,10 +87,8 @@ void fscache_free_cookie(struct fscache_cookie *cookie)
static int fscache_set_key(struct fscache_cookie *cookie,
const void *index_key, size_t index_key_len)
{
- unsigned long long h;
u32 *buf;
int bufs;
- int i;

bufs = DIV_ROUND_UP(index_key_len, sizeof(*buf));

@@ -104,17 +102,7 @@ static int fscache_set_key(struct fscache_cookie *cookie,
}

memcpy(buf, index_key, index_key_len);
-
- /* Calculate a hash and combine this with the length in the first word
- * or first half word
- */
- h = (unsigned long)cookie->parent;
- h += index_key_len + cookie->type;
-
- for (i = 0; i < bufs; i++)
- h += buf[i];
-
- cookie->key_hash = h ^ (h >> 32);
+ cookie->key_hash = fscache_hash(0, buf, bufs);
return 0;
}

diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
index 200082cafdda..a49136c63e4b 100644
--- a/fs/fscache/internal.h
+++ b/fs/fscache/internal.h
@@ -74,6 +74,8 @@ extern struct workqueue_struct *fscache_object_wq;
extern struct workqueue_struct *fscache_op_wq;
DECLARE_PER_CPU(wait_queue_head_t, fscache_object_cong_wait);

+extern unsigned int fscache_hash(unsigned int salt, unsigned int *data, unsigned int n);
+
static inline bool fscache_object_congested(void)
{
return workqueue_congested(WORK_CPU_UNBOUND, fscache_object_wq);
diff --git a/fs/fscache/main.c b/fs/fscache/main.c
index c1e6cc9091aa..4207f98e405f 100644
--- a/fs/fscache/main.c
+++ b/fs/fscache/main.c
@@ -93,6 +93,45 @@ static struct ctl_table fscache_sysctls_root[] = {
};
#endif

+/*
+ * Mixing scores (in bits) for (7,20):
+ * Input delta: 1-bit 2-bit
+ * 1 round: 330.3 9201.6
+ * 2 rounds: 1246.4 25475.4
+ * 3 rounds: 1907.1 31295.1
+ * 4 rounds: 2042.3 31718.6
+ * Perfect: 2048 31744
+ * (32*64) (32*31/2 * 64)
+ */
+#define HASH_MIX(x, y, a) \
+ ( x ^= (a), \
+ y ^= x, x = rol32(x, 7),\
+ x += y, y = rol32(y,20),\
+ y *= 9 )
+
+static inline unsigned int fold_hash(unsigned long x, unsigned long y)
+{
+ /* Use arch-optimized multiply if one exists */
+ return __hash_32(y ^ __hash_32(x));
+}
+
+/*
+ * Generate a hash. This is derived from full_name_hash(), but we want to be
+ * sure it is arch independent and that it doesn't change as bits of the
+ * computed hash value might appear on disk. The caller also guarantees that
+ * the hashed data will be a series of aligned 32-bit words.
+ */
+unsigned int fscache_hash(unsigned int salt, unsigned int *data, unsigned int n)
+{
+ unsigned int a, x = 0, y = salt;
+
+ for (; n; n--) {
+ a = *data++;
+ HASH_MIX(x, y, a);
+ }
+ return fold_hash(x, y);
+}
+
/*
* initialise the fs caching module
*/


2021-08-24 14:25:34

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH 11/12] fscache: Fix fscache_cookie_put() to not deref after dec

On Mon, 2021-06-21 at 22:47 +0100, David Howells wrote:
> fscache_cookie_put() accesses the cookie it has just put inside the
> tracepoint that monitors the change - but this is something it's not
> allowed to do if we didn't reduce the count to zero.

Do you mean "if the count went to zero." ?

>
> Fix this by dropping most of those values from the tracepoint and grabbing
> the cookie debug ID before doing the dec.
>
> Also take the opportunity to switch over the usage and where arguments on
> the tracepoint to put the reason last.
>
> Signed-off-by: David Howells <[email protected]>
> ---
>
> fs/fscache/cookie.c | 10 ++++++----
> fs/fscache/internal.h | 2 +-
> fs/fscache/netfs.c | 2 +-
> include/trace/events/fscache.h | 24 +++++++-----------------
> 4 files changed, 15 insertions(+), 23 deletions(-)
>
> diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
> index 2558814193e9..6df3732cf1b4 100644
> --- a/fs/fscache/cookie.c
> +++ b/fs/fscache/cookie.c
> @@ -225,8 +225,8 @@ struct fscache_cookie *fscache_hash_cookie(struct fscache_cookie *candidate)
>
> collision:
> if (test_and_set_bit(FSCACHE_COOKIE_ACQUIRED, &cursor->flags)) {
> - trace_fscache_cookie(cursor, fscache_cookie_collision,
> - atomic_read(&cursor->usage));
> + trace_fscache_cookie(cursor->debug_id, atomic_read(&cursor->usage),
> + fscache_cookie_collision);
> pr_err("Duplicate cookie detected\n");
> fscache_print_cookie(cursor, 'O');
> fscache_print_cookie(candidate, 'N');
> @@ -305,7 +305,8 @@ struct fscache_cookie *__fscache_acquire_cookie(
>
> cookie = fscache_hash_cookie(candidate);
> if (!cookie) {
> - trace_fscache_cookie(candidate, fscache_cookie_discard, 1);
> + trace_fscache_cookie(candidate->debug_id, 1,
> + fscache_cookie_discard);
> goto out;
> }
>
> @@ -866,8 +867,9 @@ void fscache_cookie_put(struct fscache_cookie *cookie,
> _enter("%x", cookie->debug_id);
>
> do {
> + unsigned int cookie_debug_id = cookie->debug_id;
> usage = atomic_dec_return(&cookie->usage);
> - trace_fscache_cookie(cookie, where, usage);
> + trace_fscache_cookie(cookie_debug_id, usage, where);
>
> if (usage > 0)
> return;
> diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
> index a49136c63e4b..345105dbbfd1 100644
> --- a/fs/fscache/internal.h
> +++ b/fs/fscache/internal.h
> @@ -291,7 +291,7 @@ static inline void fscache_cookie_get(struct fscache_cookie *cookie,
> {
> int usage = atomic_inc_return(&cookie->usage);
>
> - trace_fscache_cookie(cookie, where, usage);
> + trace_fscache_cookie(cookie->debug_id, usage, where);
> }
>
> /*
> diff --git a/fs/fscache/netfs.c b/fs/fscache/netfs.c
> index cce92216fa28..d6bdb7b5e723 100644
> --- a/fs/fscache/netfs.c
> +++ b/fs/fscache/netfs.c
> @@ -37,7 +37,7 @@ int __fscache_register_netfs(struct fscache_netfs *netfs)
> if (!cookie)
> goto already_registered;
> if (cookie != candidate) {
> - trace_fscache_cookie(candidate, fscache_cookie_discard, 1);
> + trace_fscache_cookie(candidate->debug_id, 1, fscache_cookie_discard);
> fscache_free_cookie(candidate);
> }
>
> diff --git a/include/trace/events/fscache.h b/include/trace/events/fscache.h
> index 0b9e058aba4d..55b8802740fa 100644
> --- a/include/trace/events/fscache.h
> +++ b/include/trace/events/fscache.h
> @@ -160,37 +160,27 @@ fscache_cookie_traces;
>
>
> TRACE_EVENT(fscache_cookie,
> - TP_PROTO(struct fscache_cookie *cookie,
> - enum fscache_cookie_trace where,
> - int usage),
> + TP_PROTO(unsigned int cookie_debug_id,
> + int usage,
> + enum fscache_cookie_trace where),
>
> - TP_ARGS(cookie, where, usage),
> + TP_ARGS(cookie_debug_id, usage, where),
>
> TP_STRUCT__entry(
> __field(unsigned int, cookie )
> - __field(unsigned int, parent )
> __field(enum fscache_cookie_trace, where )
> __field(int, usage )
> - __field(int, n_children )
> - __field(int, n_active )
> - __field(u8, flags )
> ),
>
> TP_fast_assign(
> - __entry->cookie = cookie->debug_id;
> - __entry->parent = cookie->parent ? cookie->parent->debug_id : 0;
> + __entry->cookie = cookie_debug_id;
> __entry->where = where;
> __entry->usage = usage;
> - __entry->n_children = atomic_read(&cookie->n_children);
> - __entry->n_active = atomic_read(&cookie->n_active);
> - __entry->flags = cookie->flags;
> ),
>
> - TP_printk("%s c=%08x u=%d p=%08x Nc=%d Na=%d f=%02x",
> + TP_printk("%s c=%08x u=%d",
> __print_symbolic(__entry->where, fscache_cookie_traces),
> - __entry->cookie, __entry->usage,
> - __entry->parent, __entry->n_children, __entry->n_active,
> - __entry->flags)
> + __entry->cookie, __entry->usage)
> );
>
> TRACE_EVENT(fscache_netfs,
>
>

Patch itself looks fine though.
--
Jeff Layton <[email protected]>

2021-08-24 14:26:06

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH 00/12] fscache: Some prep work for fscache rewrite

On Mon, 2021-06-21 at 22:44 +0100, David Howells wrote:
> Here are some patches that perform some preparatory work for the fscache
> rewrite that's being worked on. These include:
>
> (1) Always select netfs stats when enabling fscache stats since they're
> displayed through the same procfile.
>
> (2) Add a cookie debug ID that can be used in tracepoints instead of a
> pointer and cache it in the netfs_cache_resources struct rather than
> in the netfs_read_request struct to make it more available.
>
> (3) Use file_inode() in cachefiles rather than dereferencing file->f_inode
> directly.
>
> (4) Provide a procfile to display fscache cookies.
>
> (5) Remove the fscache and cachefiles histogram procfiles.
>
> (6) Remove the fscache object list procfile.
>
> (7) Avoid using %p in fscache and cachefiles as the value is hashed and
> not comparable to the register dump in an oops trace.
>
> (8) Fix the cookie hash function to actually achieve useful dispersion.
>
> (9) Fix fscache_cookie_put() so that it doesn't dereference the cookie
> pointer in the tracepoint after the refcount has been decremented
> (we're only allowed to do that if we decremented it to zero).
>
> (10) Use refcount_t rather than atomic_t for the fscache_cookie refcount.
>
> The patches can be found on this branch:
>
> http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=fscache-next
>
> David
> ---
> David Howells (12):
> fscache: Select netfs stats if fscache stats are enabled
> netfs: Move cookie debug ID to struct netfs_cache_resources
> cachefiles: Use file_inode() rather than accessing ->f_inode
> fscache: Add a cookie debug ID and use that in traces
> fscache: Procfile to display cookies
> fscache, cachefiles: Remove the histogram stuff
> fscache: Remove the object list procfile
> fscache: Change %p in format strings to something else
> cachefiles: Change %p in format strings to something else
> fscache: Fix cookie key hashing
> fscache: Fix fscache_cookie_put() to not deref after dec
> fscache: Use refcount_t for the cookie refcount instead of atomic_t
>
>
> fs/cachefiles/Kconfig | 19 --
> fs/cachefiles/Makefile | 2 -
> fs/cachefiles/bind.c | 2 -
> fs/cachefiles/interface.c | 6 +-
> fs/cachefiles/internal.h | 25 --
> fs/cachefiles/io.c | 6 +-
> fs/cachefiles/key.c | 2 +-
> fs/cachefiles/main.c | 7 -
> fs/cachefiles/namei.c | 61 ++---
> fs/cachefiles/proc.c | 114 --------
> fs/cachefiles/xattr.c | 4 +-
> fs/fscache/Kconfig | 24 --
> fs/fscache/Makefile | 2 -
> fs/fscache/cache.c | 11 +-
> fs/fscache/cookie.c | 201 +++++++++++----
> fs/fscache/fsdef.c | 3 +-
> fs/fscache/histogram.c | 87 -------
> fs/fscache/internal.h | 57 +---
> fs/fscache/main.c | 39 +++
> fs/fscache/netfs.c | 2 +-
> fs/fscache/object-list.c | 414 ------------------------------
> fs/fscache/object.c | 8 -
> fs/fscache/operation.c | 3 -
> fs/fscache/page.c | 6 -
> fs/fscache/proc.c | 20 +-
> include/linux/fscache-cache.h | 4 -
> include/linux/fscache.h | 4 +-
> include/linux/netfs.h | 2 +-
> include/trace/events/cachefiles.h | 68 ++---
> include/trace/events/fscache.h | 160 ++++++------
> include/trace/events/netfs.h | 2 +-
> 31 files changed, 367 insertions(+), 998 deletions(-)
> delete mode 100644 fs/cachefiles/proc.c
> delete mode 100644 fs/fscache/histogram.c
> delete mode 100644 fs/fscache/object-list.c
>
>

This all looks good (modulo a nitpicky changelog comment). You can add:

Reviewed-by: Jeff Layton <[email protected]>

2021-08-24 16:12:10

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH 10/12] fscache: Fix cookie key hashing

On Mon, 2021-06-21 at 22:46 +0100, David Howells wrote:
> The current hash algorithm used for hashing cookie keys is really bad,
> producing almost no dispersion (after a test kernel build, ~30000 files
> were split over just 18 out of the 32768 hash buckets).
>
> Borrow the full_name_hash() hash function into fscache to do the hashing
> for cookie keys and, in the future, volume keys.
>
> I don't want to use full_name_hash() as-is because I want the hash value to
> be consistent across arches and over time as the hash value produced may
> get used on disk.
>
> I can also optimise parts of it away as the key will always be a padded
> array of aligned 32-bit words.
>
> Signed-off-by: David Howells <[email protected]>
> ---
>

What happens when this patch encounters a cache that was built before
it? Do you need to couple this with some sort of global cache
invalidation or rehashing event?

> fs/fscache/cookie.c | 14 +-------------
> fs/fscache/internal.h | 2 ++
> fs/fscache/main.c | 39 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 42 insertions(+), 13 deletions(-)
>
> diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
> index ec9bce33085f..2558814193e9 100644
> --- a/fs/fscache/cookie.c
> +++ b/fs/fscache/cookie.c
> @@ -87,10 +87,8 @@ void fscache_free_cookie(struct fscache_cookie *cookie)
> static int fscache_set_key(struct fscache_cookie *cookie,
> const void *index_key, size_t index_key_len)
> {
> - unsigned long long h;
> u32 *buf;
> int bufs;
> - int i;
>
> bufs = DIV_ROUND_UP(index_key_len, sizeof(*buf));
>
> @@ -104,17 +102,7 @@ static int fscache_set_key(struct fscache_cookie *cookie,
> }
>
> memcpy(buf, index_key, index_key_len);
> -
> - /* Calculate a hash and combine this with the length in the first word
> - * or first half word
> - */
> - h = (unsigned long)cookie->parent;
> - h += index_key_len + cookie->type;
> -
> - for (i = 0; i < bufs; i++)
> - h += buf[i];
> -
> - cookie->key_hash = h ^ (h >> 32);
> + cookie->key_hash = fscache_hash(0, buf, bufs);
> return 0;
> }
>
> diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
> index 200082cafdda..a49136c63e4b 100644
> --- a/fs/fscache/internal.h
> +++ b/fs/fscache/internal.h
> @@ -74,6 +74,8 @@ extern struct workqueue_struct *fscache_object_wq;
> extern struct workqueue_struct *fscache_op_wq;
> DECLARE_PER_CPU(wait_queue_head_t, fscache_object_cong_wait);
>
> +extern unsigned int fscache_hash(unsigned int salt, unsigned int *data, unsigned int n);
> +
> static inline bool fscache_object_congested(void)
> {
> return workqueue_congested(WORK_CPU_UNBOUND, fscache_object_wq);
> diff --git a/fs/fscache/main.c b/fs/fscache/main.c
> index c1e6cc9091aa..4207f98e405f 100644
> --- a/fs/fscache/main.c
> +++ b/fs/fscache/main.c
> @@ -93,6 +93,45 @@ static struct ctl_table fscache_sysctls_root[] = {
> };
> #endif
>
> +/*
> + * Mixing scores (in bits) for (7,20):
> + * Input delta: 1-bit 2-bit
> + * 1 round: 330.3 9201.6
> + * 2 rounds: 1246.4 25475.4
> + * 3 rounds: 1907.1 31295.1
> + * 4 rounds: 2042.3 31718.6
> + * Perfect: 2048 31744
> + * (32*64) (32*31/2 * 64)
> + */
> +#define HASH_MIX(x, y, a) \
> + ( x ^= (a), \
> + y ^= x, x = rol32(x, 7),\
> + x += y, y = rol32(y,20),\
> + y *= 9 )
> +
> +static inline unsigned int fold_hash(unsigned long x, unsigned long y)
> +{
> + /* Use arch-optimized multiply if one exists */
> + return __hash_32(y ^ __hash_32(x));
> +}
> +
> +/*
> + * Generate a hash. This is derived from full_name_hash(), but we want to be
> + * sure it is arch independent and that it doesn't change as bits of the
> + * computed hash value might appear on disk. The caller also guarantees that
> + * the hashed data will be a series of aligned 32-bit words.
> + */
> +unsigned int fscache_hash(unsigned int salt, unsigned int *data, unsigned int n)
> +{
> + unsigned int a, x = 0, y = salt;
> +
> + for (; n; n--) {
> + a = *data++;
> + HASH_MIX(x, y, a);
> + }
> + return fold_hash(x, y);
> +}
> +
> /*
> * initialise the fs caching module
> */
>
>

--
Jeff Layton <[email protected]>

2021-08-25 14:06:28

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 10/12] fscache: Fix cookie key hashing

Jeff Layton <[email protected]> wrote:

> What happens when this patch encounters a cache that was built before
> it? Do you need to couple this with some sort of global cache
> invalidation or rehashing event?

At the moment, nothing. cachefiles generates a second hash value, but doing
so is a duplication of effort.

David

2021-08-25 14:06:38

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 11/12] fscache: Fix fscache_cookie_put() to not deref after dec

Jeff Layton <[email protected]> wrote:

> > fscache_cookie_put() accesses the cookie it has just put inside the
> > tracepoint that monitors the change - but this is something it's not
> > allowed to do if we didn't reduce the count to zero.
>
> Do you mean "if the count went to zero." ?

No. If *we* reduced the count to zero, it falls to us to destroy the object,
so we're allowed to look into it again.

If we didn't reduce the count to zero, then someone else might destroy it
before we look into it again.

David