* Re: [v8, bpf-next, 4/9] net/wireless/iwlwifi: fix iwlwifi_dev_ucode_error tracepoint
From: Steven Rostedt @ 2018-05-24 23:39 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Johannes Berg, Alexei Starovoitov, davem, daniel, torvalds,
peterz, mathieu.desnoyers, netdev, kernel-team, linux-api,
linux-wireless
In-Reply-To: <20180524232837.24jvdsdiohkpj7fs@ast-mbp>
On Thu, 24 May 2018 16:28:39 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> Ohh. I didn't realize that networking wireless doesn't fall under netdev.
> I thought wireless folks are silent because they are embarrassed
> by a function with 17 arguments.
Please lets refrain from the demeaning comments.
I agree with your argument, but not the tone.
-- Steve
^ permalink raw reply
* Re: [v8, bpf-next, 4/9] net/wireless/iwlwifi: fix iwlwifi_dev_ucode_error tracepoint
From: Alexei Starovoitov @ 2018-05-24 23:28 UTC (permalink / raw)
To: Johannes Berg
Cc: Alexei Starovoitov, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
daniel-FeC+5ew28dpmcu3hnIyYJQ,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
peterz-wEGCiKHe2LqWVfeAwA7xHQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w,
netdev-u79uwXL29TY76Z2rM5mHXA, kernel-team-b10kYP2dOMg,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1527073388.3759.21.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
On Wed, May 23, 2018 at 01:03:08PM +0200, Johannes Berg wrote:
> On Wed, 2018-03-28 at 12:05 -0700, Alexei Starovoitov wrote:
> > fix iwlwifi_dev_ucode_error tracepoint to pass pointer to a table
> > instead of all 17 arguments by value.
> > dvm/main.c and mvm/utils.c have 'struct iwl_error_event_table'
> > defined with very similar yet subtly different fields and offsets.
> > tracepoint is still common and using definition of 'struct iwl_error_event_table'
> > from dvm/commands.h while copying fields.
> > Long term this tracepoint probably should be split into two.
>
> It would've been nice to CC the wireless list for wireless related
> patches ...
Ohh. I didn't realize that networking wireless doesn't fall under netdev.
I thought wireless folks are silent because they are embarrassed
by a function with 17 arguments.
> > --- a/drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c
> > @@ -30,6 +30,7 @@
> > #ifndef __CHECKER__
> > #include "iwl-trans.h"
> >
> > +#include "dvm/commands.h"
>
> In particular, this breaks the whole driver abstraction.
>
> > +++ b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
> > @@ -549,12 +549,7 @@ static void iwl_mvm_dump_lmac_error_log(struct iwl_mvm *mvm, u32 base)
> >
> > IWL_ERR(mvm, "Loaded firmware version: %s\n", mvm->fw->fw_version);
> >
> > - trace_iwlwifi_dev_ucode_error(trans->dev, table.error_id, table.tsf_low,
> > - table.data1, table.data2, table.data3,
> > - table.blink2, table.ilink1,
> > - table.ilink2, table.bcon_time, table.gp1,
> > - table.gp2, table.fw_rev_type, table.major,
> > - table.minor, table.hw_ver, table.brd_ver);
> > + trace_iwlwifi_dev_ucode_error(trans->dev, &table, table.hw_ver, table.brd_ver);
>
> This is also utterly wrong because mvm has - for better or worse - a
> different type "struct iwl_error_event_table" in this file ...
As I was trying to explain in the commit log the single struct
is used in both places, but differences in two
"struct iwl_error_event_table" are carefully matched
field and by field. For two extra fields it was not
possible and they are passed separately as you can see above.
I still believe that tracepoint output is still exactly
the same before and after the patch.
I guess you see the breakage because new fields got
added into one "struct iwl_error_event_table",
but were not added to its evil twin "struct iwl_error_event_table"
with the same name after the patch landed ?
imo wireless folks need to avoid such naming conflicts.
I suggest to isolate common fields into separate base struct and
give two children structs different names.
^ permalink raw reply
* Re: [PATCH -next 0/6] rhashtable: guarantee first allocation
From: Linus Torvalds @ 2018-05-24 21:41 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: Andrew Morton, Thomas Graf, Herbert Xu, Manfred Spraul,
guillaume.knispel, Linux API, Linux Kernel Mailing List
In-Reply-To: <20180524211135.27760-1-dave@stgolabs.net>
On Thu, May 24, 2018 at 2:28 PM Davidlohr Bueso <dave@stgolabs.net> wrote:
> 10 files changed, 79 insertions(+), 86 deletions(-)
I certainly can't complain about this small code removal, but I think if we
did the kvmalloc_node() cleanup, we'd be able to get rid of even more.
For example, bucket_table_alloc() does that
if (gfp != GFP_KERNEL)
tbl = kzalloc(size, gfp | __GFP_NOWARN | __GFP_NORETRY);
else
tbl = kvzalloc(size, gfp);
purely due to the kvalloc_node() oddity. Wouldn't it be nice to just write
it as
tbl = kvzalloc(size, gfp);
knowing that the whole point of all the kv*alloc*() functions is to "just
do the right thing given size, gpf mask, and ease of allocation".
Linus
^ permalink raw reply
* Re: [PATCH 3/6] lib/bucket_locks: use kvmalloc_array()
From: Linus Torvalds @ 2018-05-24 21:37 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: Andrew Morton, Thomas Graf, Herbert Xu, Manfred Spraul,
guillaume.knispel, Linux API, Linux Kernel Mailing List,
Davidlohr Bueso
In-Reply-To: <20180524211135.27760-4-dave@stgolabs.net>
On Thu, May 24, 2018 at 2:28 PM Davidlohr Bueso <dave@stgolabs.net> wrote:
> if (gfpflags_allow_blocking(gfp))
> - tlocks = kvmalloc(size * sizeof(spinlock_t), gfp);
> + tlocks = kvmalloc_array(size, sizeof(spinlock_t),
gfp);
> else
> tlocks = kmalloc_array(size, sizeof(spinlock_t),
gfp);
Side note: how about we just move that "gfpflags_allow_blocking()" into
kvmalloc() instead, and make kvmalloc() generally usable?
Now we have that really odd situation where kvmalloc() takes gfp flags, but
to quote the comment:
* Any use of gfp flags outside of GFP_KERNEL should be consulted with mm
people.
and the code:
/*
* vmalloc uses GFP_KERNEL for some internal allocations (e.g page
tables)
* so the given set of flags has to be compatible.
*/
WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL);
which isn't really all that helpful. Do mm people really want to be
consulted about random uses?
Maybe we could just make the rule for kvmalloc() be to only fall back on
vmalloc for allocations that are
- larger than page size
- blocking and allow GFP_KERNEL (so basically that WARN_ON_ONCE() logic in
kvmalloc_node).
Hmm? Isn't that what everybody really *wants* kvmalloc() and friends to do?
Linus
^ permalink raw reply
* [PATCH 6/6] lib/test_rhashtable: rhashtable_init() can no longer fail
From: Davidlohr Bueso @ 2018-05-24 21:11 UTC (permalink / raw)
To: akpm, torvalds
Cc: tgraf, herbert, manfred, guillaume.knispel, linux-api,
linux-kernel, Davidlohr Bueso, Davidlohr Bueso
In-Reply-To: <20180524211135.27760-1-dave@stgolabs.net>
Update the test module as such.
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
lib/test_rhashtable.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
index f4000c137dbe..a894eb0407f0 100644
--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -182,11 +182,7 @@ static void test_bucket_stats(struct rhashtable *ht, unsigned int entries)
struct rhashtable_iter hti;
struct rhash_head *pos;
- err = rhashtable_walk_init(ht, &hti, GFP_KERNEL);
- if (err) {
- pr_warn("Test failed: allocation error");
- return;
- }
+ rhashtable_walk_init(ht, &hti, GFP_KERNEL);
rhashtable_walk_start(&hti);
--
2.13.6
^ permalink raw reply related
* [PATCH 5/6] ipc: simplify ipc initialization
From: Davidlohr Bueso @ 2018-05-24 21:11 UTC (permalink / raw)
To: akpm, torvalds
Cc: tgraf, herbert, manfred, guillaume.knispel, linux-api,
linux-kernel, Davidlohr Bueso, Davidlohr Bueso
In-Reply-To: <20180524211135.27760-1-dave@stgolabs.net>
Now that we know that rhashtable_init() will not fail, we
can get rid of a lot of the unnecessary cleanup paths when
the call errored out.
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
ipc/msg.c | 9 ++++-----
ipc/namespace.c | 20 ++++----------------
ipc/sem.c | 10 ++++------
ipc/shm.c | 9 ++++-----
ipc/util.c | 18 +++++-------------
ipc/util.h | 18 +++++++++---------
6 files changed, 30 insertions(+), 54 deletions(-)
diff --git a/ipc/msg.c b/ipc/msg.c
index 3b6545302598..62545ce19173 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -1228,7 +1228,7 @@ COMPAT_SYSCALL_DEFINE5(msgrcv, int, msqid, compat_uptr_t, msgp,
}
#endif
-int msg_init_ns(struct ipc_namespace *ns)
+void msg_init_ns(struct ipc_namespace *ns)
{
ns->msg_ctlmax = MSGMAX;
ns->msg_ctlmnb = MSGMNB;
@@ -1236,7 +1236,7 @@ int msg_init_ns(struct ipc_namespace *ns)
atomic_set(&ns->msg_bytes, 0);
atomic_set(&ns->msg_hdrs, 0);
- return ipc_init_ids(&ns->ids[IPC_MSG_IDS]);
+ ipc_init_ids(&ns->ids[IPC_MSG_IDS]);
}
#ifdef CONFIG_IPC_NS
@@ -1277,12 +1277,11 @@ static int sysvipc_msg_proc_show(struct seq_file *s, void *it)
}
#endif
-int __init msg_init(void)
+void __init msg_init(void)
{
- const int err = msg_init_ns(&init_ipc_ns);
+ msg_init_ns(&init_ipc_ns);
ipc_init_proc_interface("sysvipc/msg",
" key msqid perms cbytes qnum lspid lrpid uid gid cuid cgid stime rtime ctime\n",
IPC_MSG_IDS, sysvipc_msg_proc_show);
- return err;
}
diff --git a/ipc/namespace.c b/ipc/namespace.c
index f59a89966f92..21607791d62c 100644
--- a/ipc/namespace.c
+++ b/ipc/namespace.c
@@ -55,28 +55,16 @@ static struct ipc_namespace *create_ipc_ns(struct user_namespace *user_ns,
ns->user_ns = get_user_ns(user_ns);
ns->ucounts = ucounts;
- err = sem_init_ns(ns);
+ err = mq_init_ns(ns);
if (err)
goto fail_put;
- err = msg_init_ns(ns);
- if (err)
- goto fail_destroy_sem;
- err = shm_init_ns(ns);
- if (err)
- goto fail_destroy_msg;
- err = mq_init_ns(ns);
- if (err)
- goto fail_destroy_shm;
+ sem_init_ns(ns);
+ msg_init_ns(ns);
+ shm_init_ns(ns);
return ns;
-fail_destroy_shm:
- shm_exit_ns(ns);
-fail_destroy_msg:
- msg_exit_ns(ns);
-fail_destroy_sem:
- sem_exit_ns(ns);
fail_put:
put_user_ns(ns->user_ns);
ns_free_inum(&ns->ns);
diff --git a/ipc/sem.c b/ipc/sem.c
index 20f649eed8c6..78ea913ee0d8 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -220,14 +220,14 @@ static int sysvipc_sem_proc_show(struct seq_file *s, void *it);
#define sc_semopm sem_ctls[2]
#define sc_semmni sem_ctls[3]
-int sem_init_ns(struct ipc_namespace *ns)
+void sem_init_ns(struct ipc_namespace *ns)
{
ns->sc_semmsl = SEMMSL;
ns->sc_semmns = SEMMNS;
ns->sc_semopm = SEMOPM;
ns->sc_semmni = SEMMNI;
ns->used_sems = 0;
- return ipc_init_ids(&ns->ids[IPC_SEM_IDS]);
+ ipc_init_ids(&ns->ids[IPC_SEM_IDS]);
}
#ifdef CONFIG_IPC_NS
@@ -239,14 +239,12 @@ void sem_exit_ns(struct ipc_namespace *ns)
}
#endif
-int __init sem_init(void)
+void __init sem_init(void)
{
- const int err = sem_init_ns(&init_ipc_ns);
-
+ sem_init_ns(&init_ipc_ns);
ipc_init_proc_interface("sysvipc/sem",
" key semid perms nsems uid gid cuid cgid otime ctime\n",
IPC_SEM_IDS, sysvipc_sem_proc_show);
- return err;
}
/**
diff --git a/ipc/shm.c b/ipc/shm.c
index 29978ee76c2e..97468bd06b6e 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -95,14 +95,14 @@ static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp);
static int sysvipc_shm_proc_show(struct seq_file *s, void *it);
#endif
-int shm_init_ns(struct ipc_namespace *ns)
+void shm_init_ns(struct ipc_namespace *ns)
{
ns->shm_ctlmax = SHMMAX;
ns->shm_ctlall = SHMALL;
ns->shm_ctlmni = SHMMNI;
ns->shm_rmid_forced = 0;
ns->shm_tot = 0;
- return ipc_init_ids(&shm_ids(ns));
+ ipc_init_ids(&shm_ids(ns));
}
/*
@@ -135,9 +135,8 @@ void shm_exit_ns(struct ipc_namespace *ns)
static int __init ipc_ns_init(void)
{
- const int err = shm_init_ns(&init_ipc_ns);
- WARN(err, "ipc: sysv shm_init_ns failed: %d\n", err);
- return err;
+ shm_init_ns(&init_ipc_ns);
+ return 0;
}
pure_initcall(ipc_ns_init);
diff --git a/ipc/util.c b/ipc/util.c
index 823e09e72c58..06d7c575847c 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -87,16 +87,12 @@ struct ipc_proc_iface {
*/
static int __init ipc_init(void)
{
- int err_sem, err_msg;
-
proc_mkdir("sysvipc", NULL);
- err_sem = sem_init();
- WARN(err_sem, "ipc: sysv sem_init failed: %d\n", err_sem);
- err_msg = msg_init();
- WARN(err_msg, "ipc: sysv msg_init failed: %d\n", err_msg);
+ sem_init();
+ msg_init();
shm_init();
- return err_msg ? err_msg : err_sem;
+ return 0;
}
device_initcall(ipc_init);
@@ -115,21 +111,17 @@ static const struct rhashtable_params ipc_kht_params = {
* Set up the sequence range to use for the ipc identifier range (limited
* below IPCMNI) then initialise the keys hashtable and ids idr.
*/
-int ipc_init_ids(struct ipc_ids *ids)
+void ipc_init_ids(struct ipc_ids *ids)
{
- int err;
ids->in_use = 0;
ids->seq = 0;
init_rwsem(&ids->rwsem);
- err = rhashtable_init(&ids->key_ht, &ipc_kht_params);
- if (err)
- return err;
+ rhashtable_init(&ids->key_ht, &ipc_kht_params);
idr_init(&ids->ipcs_idr);
ids->max_id = -1;
#ifdef CONFIG_CHECKPOINT_RESTORE
ids->next_id = -1;
#endif
- return 0;
}
#ifdef CONFIG_PROC_FS
diff --git a/ipc/util.h b/ipc/util.h
index 0aba3230d007..65fad8a94da8 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -18,8 +18,8 @@
#define IPCMNI 32768 /* <= MAX_INT limit for ipc arrays (including sysctl changes) */
#define SEQ_MULTIPLIER (IPCMNI)
-int sem_init(void);
-int msg_init(void);
+void sem_init(void);
+void msg_init(void);
void shm_init(void);
struct ipc_namespace;
@@ -34,17 +34,17 @@ static inline void mq_put_mnt(struct ipc_namespace *ns) { }
#endif
#ifdef CONFIG_SYSVIPC
-int sem_init_ns(struct ipc_namespace *ns);
-int msg_init_ns(struct ipc_namespace *ns);
-int shm_init_ns(struct ipc_namespace *ns);
+void sem_init_ns(struct ipc_namespace *ns);
+void msg_init_ns(struct ipc_namespace *ns);
+void shm_init_ns(struct ipc_namespace *ns);
void sem_exit_ns(struct ipc_namespace *ns);
void msg_exit_ns(struct ipc_namespace *ns);
void shm_exit_ns(struct ipc_namespace *ns);
#else
-static inline int sem_init_ns(struct ipc_namespace *ns) { return 0; }
-static inline int msg_init_ns(struct ipc_namespace *ns) { return 0; }
-static inline int shm_init_ns(struct ipc_namespace *ns) { return 0; }
+static inline void sem_init_ns(struct ipc_namespace *ns) { }
+static inline void msg_init_ns(struct ipc_namespace *ns) { }
+static inline void shm_init_ns(struct ipc_namespace *ns) { }
static inline void sem_exit_ns(struct ipc_namespace *ns) { }
static inline void msg_exit_ns(struct ipc_namespace *ns) { }
@@ -83,7 +83,7 @@ struct ipc_ops {
struct seq_file;
struct ipc_ids;
-int ipc_init_ids(struct ipc_ids *);
+void ipc_init_ids(struct ipc_ids *);
#ifdef CONFIG_PROC_FS
void __init ipc_init_proc_interface(const char *path, const char *header,
int ids, int (*show)(struct seq_file *, void *));
--
2.13.6
^ permalink raw reply related
* [PATCH 4/6] ipc: get rid of ids->tables_initialized hack
From: Davidlohr Bueso @ 2018-05-24 21:11 UTC (permalink / raw)
To: akpm, torvalds
Cc: tgraf, herbert, manfred, guillaume.knispel, linux-api,
linux-kernel, Davidlohr Bueso, Davidlohr Bueso
In-Reply-To: <20180524211135.27760-1-dave@stgolabs.net>
In sysvipc we have an ids->tables_initialized regarding the
rhashtable, introduced in:
0cfb6aee70b (ipc: optimize semget/shmget/msgget for lots of keys).
It's there, specifically, to prevent nil pointer dereferences,
from using an uninitialized api. Considering how rhashtable_init()
can fail (probably due to ENOMEM, if anything), this made the
overall ipc initialization capable of failure as well. That alone
is ugly, but fine, however I've spotted a few issues regarding the
semantics of tables_initialized (however unlikely they may be):
- There is inconsistency in what we return to userspace: ipc_addid()
returns ENOSPC which is certainly _wrong_, while ipc_obtain_object_idr()
returns EINVAL.
- After we started using rhashtables, ipc_findkey() can return nil upon
!tables_initialized, but the caller expects nil for when the ipc structure
isn't found, and can therefore call into ipcget() callbacks.
Now that rhashtable initialization cannot fail, we can properly
get rid of the hack altogether.
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
include/linux/ipc_namespace.h | 1 -
ipc/util.c | 23 ++++++++---------------
2 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index b5630c8eb2f3..37f3a4b7c637 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -16,7 +16,6 @@ struct user_namespace;
struct ipc_ids {
int in_use;
unsigned short seq;
- bool tables_initialized;
struct rw_semaphore rwsem;
struct idr ipcs_idr;
int max_id;
diff --git a/ipc/util.c b/ipc/util.c
index 4e81182fa0ac..823e09e72c58 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -125,7 +125,6 @@ int ipc_init_ids(struct ipc_ids *ids)
if (err)
return err;
idr_init(&ids->ipcs_idr);
- ids->tables_initialized = true;
ids->max_id = -1;
#ifdef CONFIG_CHECKPOINT_RESTORE
ids->next_id = -1;
@@ -178,19 +177,16 @@ void __init ipc_init_proc_interface(const char *path, const char *header,
*/
static struct kern_ipc_perm *ipc_findkey(struct ipc_ids *ids, key_t key)
{
- struct kern_ipc_perm *ipcp = NULL;
+ struct kern_ipc_perm *ipcp;
- if (likely(ids->tables_initialized))
- ipcp = rhashtable_lookup_fast(&ids->key_ht, &key,
+ ipcp = rhashtable_lookup_fast(&ids->key_ht, &key,
ipc_kht_params);
+ if (!ipcp)
+ return NULL;
- if (ipcp) {
- rcu_read_lock();
- ipc_lock_object(ipcp);
- return ipcp;
- }
-
- return NULL;
+ rcu_read_lock();
+ ipc_lock_object(ipcp);
+ return ipcp;
}
#ifdef CONFIG_CHECKPOINT_RESTORE
@@ -255,7 +251,7 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int limit)
if (limit > IPCMNI)
limit = IPCMNI;
- if (!ids->tables_initialized || ids->in_use >= limit)
+ if (ids->in_use >= limit)
return -ENOSPC;
idr_preload(GFP_KERNEL);
@@ -566,9 +562,6 @@ struct kern_ipc_perm *ipc_obtain_object_idr(struct ipc_ids *ids, int id)
struct kern_ipc_perm *out;
int lid = ipcid_to_idx(id);
- if (unlikely(!ids->tables_initialized))
- return ERR_PTR(-EINVAL);
-
out = idr_find(&ids->ipcs_idr, lid);
if (!out)
return ERR_PTR(-EINVAL);
--
2.13.6
^ permalink raw reply related
* [PATCH 3/6] lib/bucket_locks: use kvmalloc_array()
From: Davidlohr Bueso @ 2018-05-24 21:11 UTC (permalink / raw)
To: akpm, torvalds
Cc: tgraf, herbert, manfred, guillaume.knispel, linux-api,
linux-kernel, Davidlohr Bueso, Davidlohr Bueso
In-Reply-To: <20180524211135.27760-1-dave@stgolabs.net>
For some reason we don't use this call, but we rely just
fine on kmalloc_array(). Make both consistent.
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
lib/bucket_locks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/bucket_locks.c b/lib/bucket_locks.c
index 266a97c5708b..75fe7386756f 100644
--- a/lib/bucket_locks.c
+++ b/lib/bucket_locks.c
@@ -31,7 +31,7 @@ int alloc_bucket_spinlocks(spinlock_t **locks, unsigned int *locks_mask,
if (sizeof(spinlock_t) != 0) {
if (gfpflags_allow_blocking(gfp))
- tlocks = kvmalloc(size * sizeof(spinlock_t), gfp);
+ tlocks = kvmalloc_array(size, sizeof(spinlock_t), gfp);
else
tlocks = kmalloc_array(size, sizeof(spinlock_t), gfp);
if (!tlocks)
--
2.13.6
^ permalink raw reply related
* [PATCH 2/6] lib/rhashtable: guarantee initial hashtable allocation
From: Davidlohr Bueso @ 2018-05-24 21:11 UTC (permalink / raw)
To: akpm, torvalds
Cc: tgraf, herbert, manfred, guillaume.knispel, linux-api,
linux-kernel, Davidlohr Bueso, Davidlohr Bueso
In-Reply-To: <20180524211135.27760-1-dave@stgolabs.net>
rhashtable_init() may fail due to -ENOMEM, thus making the
entire api unusable. This patch removes this scenario,
however unlikely. In order to guarantee memory allocation,
this patch refactors bucket_table_alloc() to add a 'retry'
parameter which always ends up doing GFP_KERNEL|__GFP_NOFAIL
for both the tbl as well as alloc_bucket_spinlocks().
So upon the first table allocation failure, we shrink the
size to the smallest value that makes sense and retry the alloc
with the same semantics. If we fail again, then we force the
call with __GFP_NOFAIL. With the defaults, this means that from
64 buckets, we retry with only 4. Any later issues regarding
performance due to collisions or larger table resizing (when
more memory becomes available) is the last of our problems.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
lib/rhashtable.c | 40 +++++++++++++++++++++++++++++++++++-----
1 file changed, 35 insertions(+), 5 deletions(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 05a4b1b8b8ce..28f28602e2f5 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -166,15 +166,20 @@ static struct bucket_table *nested_bucket_table_alloc(struct rhashtable *ht,
return tbl;
}
-static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
- size_t nbuckets,
- gfp_t gfp)
+static struct bucket_table *__bucket_table_alloc(struct rhashtable *ht,
+ size_t nbuckets,
+ gfp_t gfp, bool retry)
{
struct bucket_table *tbl = NULL;
size_t size, max_locks;
int i;
size = sizeof(*tbl) + nbuckets * sizeof(tbl->buckets[0]);
+ if (retry) {
+ gfp |= __GFP_NOFAIL;
+ tbl = kzalloc(size, gfp);
+ } /* fall-through */
+
if (gfp != GFP_KERNEL)
tbl = kzalloc(size, gfp | __GFP_NOWARN | __GFP_NORETRY);
else
@@ -211,6 +216,20 @@ static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
return tbl;
}
+static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
+ size_t nbuckets,
+ gfp_t gfp)
+{
+ return __bucket_table_alloc(ht, nbuckets, gfp, false);
+}
+
+static struct bucket_table *bucket_table_alloc_retry(struct rhashtable *ht,
+ size_t nbuckets,
+ gfp_t gfp)
+{
+ return __bucket_table_alloc(ht, nbuckets, gfp, true);
+}
+
static struct bucket_table *rhashtable_last_table(struct rhashtable *ht,
struct bucket_table *tbl)
{
@@ -1067,9 +1086,20 @@ int rhashtable_init(struct rhashtable *ht,
}
}
+ /*
+ * This is api initialization and thus we need to guarantee the
+ * initial rhashtable allocation. Upon failure, retry with a
+ * smallest possible size, otherwise we exhaust our options with
+ * __GFP_NOFAIL.
+ */
tbl = bucket_table_alloc(ht, size, GFP_KERNEL);
- if (tbl == NULL)
- return -ENOMEM;
+ if (unlikely(tbl == NULL)) {
+ size = HASH_MIN_SIZE;
+
+ tbl = bucket_table_alloc(ht, size, GFP_KERNEL);
+ if (tbl == NULL)
+ tbl = bucket_table_alloc_retry(ht, size, GFP_KERNEL);
+ }
atomic_set(&ht->nelems, 0);
--
2.13.6
^ permalink raw reply related
* [PATCH 1/6] lib/rhashtable: convert param sanitations to WARN_ON
From: Davidlohr Bueso @ 2018-05-24 21:11 UTC (permalink / raw)
To: akpm, torvalds
Cc: tgraf, herbert, manfred, guillaume.knispel, linux-api,
linux-kernel, Davidlohr Bueso, Davidlohr Bueso
In-Reply-To: <20180524211135.27760-1-dave@stgolabs.net>
For the purpose of making rhashtable_init() unable to fail,
we can replace the returning -EINVAL with WARN_ONs whenever
the caller passes bogus parameters during initialization.
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
lib/rhashtable.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 9427b5766134..05a4b1b8b8ce 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -1024,12 +1024,11 @@ int rhashtable_init(struct rhashtable *ht,
size = HASH_DEFAULT_SIZE;
- if ((!params->key_len && !params->obj_hashfn) ||
- (params->obj_hashfn && !params->obj_cmpfn))
- return -EINVAL;
+ WARN_ON((!params->key_len && !params->obj_hashfn) ||
+ (params->obj_hashfn && !params->obj_cmpfn));
- if (params->nulls_base && params->nulls_base < (1U << RHT_BASE_SHIFT))
- return -EINVAL;
+ WARN_ON(params->nulls_base &&
+ params->nulls_base < (1U << RHT_BASE_SHIFT));
memset(ht, 0, sizeof(*ht));
mutex_init(&ht->mutex);
--
2.13.6
^ permalink raw reply related
* [PATCH -next 0/6] rhashtable: guarantee first allocation
From: Davidlohr Bueso @ 2018-05-24 21:11 UTC (permalink / raw)
To: akpm, torvalds
Cc: tgraf, herbert, manfred, guillaume.knispel, linux-api,
linux-kernel, Davidlohr Bueso
Hi,
This series is the result of the discussion with Linus around ipc
subsystem initialization and how it behaves with error return when
calling rhashtable_init()[1]. Instead of caring about the error
or calling the infamous BUG_ON, Linus suggested we guarantee the
rhashtable allocation.
First two patches modify rhashtable_init() to just return 0, future
patches I guess can update more callers.
patch 3 is a nit I found while reading the code and just makes use
kvmalloc_array().
patch 4+5 remove some ipc hacks we no longer need.
patch 6 updates the rhashtable test module. Trivial.
Please consider for v4.18.
Thanks!
[0] https://lkml.org/lkml/2018/5/23/758
Davidlohr Bueso (6):
lib/rhashtable: convert param sanitations to WARN_ON
lib/rhashtable: guarantee initial hashtable allocation
lib/bucket_locks: use kvmalloc_array()
ipc: get rid of ids->tables_initialized hack
ipc: simplify ipc initialization
lib/test_rhashtable: rhashtable_init() can no longer fail
include/linux/ipc_namespace.h | 1 -
ipc/msg.c | 9 ++++----
ipc/namespace.c | 20 ++++--------------
ipc/sem.c | 10 ++++-----
ipc/shm.c | 9 ++++----
ipc/util.c | 41 ++++++++++++------------------------
ipc/util.h | 18 ++++++++--------
lib/bucket_locks.c | 2 +-
lib/rhashtable.c | 49 ++++++++++++++++++++++++++++++++++---------
lib/test_rhashtable.c | 6 +-----
10 files changed, 79 insertions(+), 86 deletions(-)
--
2.13.6
^ permalink raw reply
* Re: semantics of rhashtable and sysvipc
From: Linus Torvalds @ 2018-05-24 19:17 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: Thomas Graf, Herbert Xu, Andrew Morton, Manfred Spraul,
guillaume.knispel, Linux API, Linux Kernel Mailing List
In-Reply-To: <20180524185155.3bx4ujgz5f5g3epi@linux-n805>
On Thu, May 24, 2018 at 12:08 PM Davidlohr Bueso <dave@stgolabs.net> wrote:
> However, after how about the resize being based on HASH_MIN_SIZE instead
of
> HASH_DEFAULT_SIZE?
I think that sounds reasonable. We wouldn't expect this to ever happen in
practice, and as you say, if it *does* happen, the size of the hash array
is the last of our problems.
> Considering that some users set p.min_size to be rather large-ish (up to
1024
> buckets afaict), we'd need the following:
> size = min(ht->p.min_size, HASH_MIN_SIZE);
Bah, let's just go for simplicity, and just make it HASH_MIN_SIZE
unconditionally, and just have a single fallback: if the first "normal"
allocation fails, do one single unconditional allocation with HASH_MIN_SIZE
and GFP_NOFAIL.
I think that should work fine.
Linus
^ permalink raw reply
* Re: semantics of rhashtable and sysvipc
From: Davidlohr Bueso @ 2018-05-24 18:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: Thomas Graf, Herbert Xu, Andrew Morton, Manfred Spraul,
guillaume.knispel, Linux API, Linux Kernel Mailing List
In-Reply-To: <CA+55aFwhkOnHngHpQ3wif5iy7PtOAVdjTcwce=kgBq0TCgmfJA@mail.gmail.com>
On Thu, 24 May 2018, Linus Torvalds wrote:
>This doesn't seem to be taking 'param->min_size' into account.
It was in that rounded_hashtable_size() does, however, after more
thought I think we can do better by taking it much more into account.
>
>I'm not sure that matters, but right now, if you have nelem_hint set and a
>min_size, the min_size is honored (if you have just min_size it's already
>ignored because the rhashtable always starts with HASH_DEFAULT_SIZE). So I
>could imagine that somebody uses it to guarantee something. The docs say
>that "min_size" is the minimum size for *shrinking* not for initializing,
>so I guess it's debatable.
>
>Also, wouldn't it make sense to make this all be a while loop? Or are you
>just depending on the knowledge that HASH_DEFAULT_SIZE / 2 is already
>guaranteed to be so small that there's no point? A comment to that effect
>would be good, perhaps.
Yes, this is why I didn't loop. With the default size of 64 buckets, we
allocate 640 + 128 = 768 bytes for the tbl and the lock array, respectively.
By halving this, upon retrying, I was relying on it being to "small to fail".
However, after how about the resize being based on HASH_MIN_SIZE instead of
HASH_DEFAULT_SIZE? That way the initial table would be a _lot_ smaller and
aid the allocator that much more; which is why we're here in the first place.
Any performance costs of collisions would be completely unimportant in this
scenario.
Considering that some users set p.min_size to be rather large-ish (up to 1024
buckets afaict), we'd need the following:
size = min(ht->p.min_size, HASH_MIN_SIZE);
Which takes into account the min_size = max(ht->p.min_size, HASH_MIN_SIZE)
which came before, thus p.min_size == 0 is already taken into account.
Thanks,
Davidlohr
^ permalink raw reply
* Re: [RFC PATCH 0/5] kmalloc-reclaimable caches
From: Matthew Wilcox @ 2018-05-24 18:48 UTC (permalink / raw)
To: Randy Dunlap
Cc: Vlastimil Babka, linux-mm, Roman Gushchin, Michal Hocko,
Johannes Weiner, linux-kernel, linux-api, Christoph Lameter,
Pekka Enberg, David Rientjes, Joonsoo Kim, Mel Gorman,
Vijayanand Jitta
In-Reply-To: <cfb7c8df-2a6a-bf84-8a30-df97c58c9c47@infradead.org>
On Thu, May 24, 2018 at 11:40:59AM -0700, Randy Dunlap wrote:
> >> while (size > 1024) {
>
> I would use (size >= 1024)
> so that 1M is printed instead of 1024K.
Yes; that's what I meant to type. Thanks!
^ permalink raw reply
* Re: [RFC PATCH 0/5] kmalloc-reclaimable caches
From: Randy Dunlap @ 2018-05-24 18:40 UTC (permalink / raw)
To: Matthew Wilcox, Vlastimil Babka
Cc: linux-mm, Roman Gushchin, Michal Hocko, Johannes Weiner,
linux-kernel, linux-api, Christoph Lameter, Pekka Enberg,
David Rientjes, Joonsoo Kim, Mel Gorman, Vijayanand Jitta
In-Reply-To: <0944e1ed-60fe-36ce-ea06-936b3f595d5f@infradead.org>
On 05/24/2018 09:18 AM, Randy Dunlap wrote:
> On 05/24/2018 04:43 AM, Matthew Wilcox wrote:
>> On Thu, May 24, 2018 at 01:00:06PM +0200, Vlastimil Babka wrote:
>>> Now for the issues a.k.a. why RFC:
>>>
>>> - I haven't find any other obvious users for reclaimable kmalloc (yet)
>>
>> Is that a problem? This sounds like it's enough to solve Facebook's
>> problem.
>>
>>> - the name of caches kmalloc-reclaimable-X is rather long
>>
>> Yes; Christoph and I were talking about restricting slab names to 16 bytes
>> just to make /proc/slabinfo easier to read. How about
>>
>> kmalloc-rec-128k
>> 1234567890123456
>>
>> Just makes it ;-)
>>
>> Of course, somebody needs to do the work to use k/M instead of 4194304.
>> We also need to bikeshed about when to switch; should it be:
>>
>> kmalloc-rec-512
>> kmalloc-rec-1024
>> kmalloc-rec-2048
>> kmalloc-rec-4096
>> kmalloc-rec-8192
>> kmalloc-rec-16k
>>
>> or should it be
>>
>> kmalloc-rec-512
>> kmalloc-rec-1k
>> kmalloc-rec-2k
>> kmalloc-rec-4k
>> kmalloc-rec-8k
>> kmalloc-rec-16k
>>
>> I slightly favour the latter as it'll be easier to implement. Something like
>
> Yes, agree, start using the suffix early.
>
>>
>> static const char suffixes[3] = ' kM';
>> int idx = 0;
>>
>> while (size > 1024) {
I would use (size >= 1024)
so that 1M is printed instead of 1024K.
>> size /= 1024;
>> idx++;
>> }
>>
>> sprintf("%d%c", size, suffices[idx]);
>
> suffixes
>>
>> --
>
>
--
~Randy
^ permalink raw reply
* Re: semantics of rhashtable and sysvipc
From: Linus Torvalds @ 2018-05-24 17:53 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: Thomas Graf, Herbert Xu, Andrew Morton, Manfred Spraul,
guillaume.knispel, Linux API, Linux Kernel Mailing List
In-Reply-To: <20180524170700.wblnybinjzx5rwky@linux-n805>
On Thu, May 24, 2018 at 10:23 AM Davidlohr Bueso <dave@stgolabs.net> wrote:
> tbl = bucket_table_alloc(ht, size, GFP_KERNEL);
> - if (tbl == NULL)
> - return -ENOMEM;
> + if (unlikely(tbl == NULL)) {
> + size = min(size, HASH_DEFAULT_SIZE) / 2;
> +
> + tbl = bucket_table_alloc(ht, size, GFP_KERNEL);
> + if (tbl == NULL)
> + tbl = bucket_table_alloc_retry(ht, size,
GFP_KERNEL);
> + }
This doesn't seem to be taking 'param->min_size' into account.
I'm not sure that matters, but right now, if you have nelem_hint set and a
min_size, the min_size is honored (if you have just min_size it's already
ignored because the rhashtable always starts with HASH_DEFAULT_SIZE). So I
could imagine that somebody uses it to guarantee something. The docs say
that "min_size" is the minimum size for *shrinking* not for initializing,
so I guess it's debatable.
Also, wouldn't it make sense to make this all be a while loop? Or are you
just depending on the knowledge that HASH_DEFAULT_SIZE / 2 is already
guaranteed to be so small that there's no point? A comment to that effect
would be good, perhaps.
Linus
^ permalink raw reply
* Re: [RFC PATCH 0/5] kmalloc-reclaimable caches
From: Laura Abbott @ 2018-05-24 17:35 UTC (permalink / raw)
To: Vlastimil Babka, Roman Gushchin
Cc: linux-mm, Michal Hocko, Johannes Weiner, linux-kernel, linux-api,
Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Mel Gorman, Vijayanand Jitta
In-Reply-To: <fea26519-2b5f-b404-872d-47afabcd3393@suse.cz>
On 05/24/2018 08:52 AM, Vlastimil Babka wrote:
> On 05/24/2018 02:13 PM, Roman Gushchin wrote:
>> On Thu, May 24, 2018 at 01:00:06PM +0200, Vlastimil Babka wrote:
>>> Hi,
>>>
>>> - I haven't find any other obvious users for reclaimable kmalloc (yet)
>>
>> As I remember, ION memory allocator was discussed related to this theme:
>> https://lkml.org/lkml/2018/4/24/1288
>
> +CC Laura
>
> Yeah ION added the NR_INDIRECTLY_RECLAIMABLE_BYTES handling, which is
> adjusted to page granularity in patch 4. I'm not sure if it should use
> kmalloc as it seems to be allocating order-X pages, where kmalloc/slab
> just means extra overhead. But maybe if it doesn't allocate/free too
> frequently, it could work?
>
The page pool allocation is supposed to be a slow path but it's
one I'd rather not have too much overhead. It also just looks really odd
to be allocating higher order pages via kmalloc imho.
>>> I did a superset as IIRC somebody suggested that in the older threads or at LSF.
>>
>> This looks nice to me!
>>
>> Thanks!
>>
>
^ permalink raw reply
* Re: [PATCH v5 20/28] fpga: dfl: add fpga bridge platform driver for FME
From: Alan Tull @ 2018-05-24 17:26 UTC (permalink / raw)
To: Wu Hao
Cc: Moritz Fischer, linux-fpga, linux-kernel, linux-api, Kang, Luwei,
Zhang, Yi Z, Tim Whisonant, Enno Luebbers, Shiva Rao,
Christopher Rauer
In-Reply-To: <20180523234213.GA24455@hao-dev>
On Wed, May 23, 2018 at 6:42 PM, Wu Hao <hao.wu@intel.com> wrote:
> On Wed, May 23, 2018 at 04:06:17PM -0500, Alan Tull wrote:
>> On Wed, May 23, 2018 at 10:28 AM, Wu Hao <hao.wu@intel.com> wrote:
>> > On Wed, May 23, 2018 at 10:15:00AM -0500, Alan Tull wrote:
>> >> On Tue, May 1, 2018 at 9:50 PM, Wu Hao <hao.wu@intel.com> wrote:
>> >>
>> >> Hi Hao,
>> >>
>> >> > This patch adds fpga bridge platform driver for FPGA Management Engine.
>> >> > It implements the enable_set callback for fpga bridge.
>> >> >
>> >> > Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
>> >> > Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
>> >> > Signed-off-by: Shiva Rao <shiva.rao@intel.com>
>> >> > Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
>> >> > Signed-off-by: Wu Hao <hao.wu@intel.com>
>> >> > Acked-by: Alan Tull <atull@kernel.org>
>> >> > Acked-by: Moritz Fischer <mdf@kernel.org>
>> >> > ---
>> >> > v3: rename driver to fpga-dfl-fme-br
>> >> > remove useless dev_dbg in probe function.
>> >> > rebased due to fpga api change.
>> >> > v4: rename to dfl-fme-br and fix SPDX license issue
>> >> > include dfl-fme-pr.h instead of dfl-fme.h
>> >> > add Acked-by from Alan and Moritz
>> >> > v5: rebase due to API changes.
>> >> > defer port and its ops finding when really need.
>> >> > ---
>> >> > drivers/fpga/Kconfig | 6 +++
>> >> > drivers/fpga/Makefile | 1 +
>> >> > drivers/fpga/dfl-fme-br.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++
>> >> > 3 files changed, 121 insertions(+)
>> >> > create mode 100644 drivers/fpga/dfl-fme-br.c
>> >> >
>> >> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
>> >> > index 89f76e8..a8f939a 100644
>> >> > --- a/drivers/fpga/Kconfig
>> >> > +++ b/drivers/fpga/Kconfig
>> >> > @@ -156,6 +156,12 @@ config FPGA_DFL_FME_MGR
>> >> > help
>> >> > Say Y to enable FPGA Manager driver for FPGA Management Engine.
>> >> >
>> >> > +config FPGA_DFL_FME_BRIDGE
>> >> > + tristate "FPGA DFL FME Bridge Driver"
>> >> > + depends on FPGA_DFL_FME
>> >> > + help
>> >> > + Say Y to enable FPGA Bridge driver for FPGA Management Engine.
>> >> > +
>> >> > config FPGA_DFL_PCI
>> >> > tristate "FPGA Device Feature List (DFL) PCIe Device Driver"
>> >> > depends on PCI && FPGA_DFL
>> >> > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
>> >> > index f82814a..75096e9 100644
>> >> > --- a/drivers/fpga/Makefile
>> >> > +++ b/drivers/fpga/Makefile
>> >> > @@ -32,6 +32,7 @@ obj-$(CONFIG_OF_FPGA_REGION) += of-fpga-region.o
>> >> > obj-$(CONFIG_FPGA_DFL) += dfl.o
>> >> > obj-$(CONFIG_FPGA_DFL_FME) += dfl-fme.o
>> >> > obj-$(CONFIG_FPGA_DFL_FME_MGR) += dfl-fme-mgr.o
>> >> > +obj-$(CONFIG_FPGA_DFL_FME_BRIDGE) += dfl-fme-br.o
>> >> >
>> >> > dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
>> >> >
>> >> > diff --git a/drivers/fpga/dfl-fme-br.c b/drivers/fpga/dfl-fme-br.c
>> >> > new file mode 100644
>> >> > index 0000000..5c51b08
>> >> > --- /dev/null
>> >> > +++ b/drivers/fpga/dfl-fme-br.c
>> >> > @@ -0,0 +1,114 @@
>> >> > +// SPDX-License-Identifier: GPL-2.0
>> >> > +/*
>> >> > + * FPGA Bridge Driver for FPGA Management Engine (FME)
>> >> > + *
>> >> > + * Copyright (C) 2017 Intel Corporation, Inc.
>> >> > + *
>> >> > + * Authors:
>> >> > + * Wu Hao <hao.wu@intel.com>
>> >> > + * Joseph Grecco <joe.grecco@intel.com>
>> >> > + * Enno Luebbers <enno.luebbers@intel.com>
>> >> > + * Tim Whisonant <tim.whisonant@intel.com>
>> >> > + * Ananda Ravuri <ananda.ravuri@intel.com>
>> >> > + * Henry Mitchel <henry.mitchel@intel.com>
>> >> > + */
>> >> > +
>> >> > +#include <linux/module.h>
>> >> > +#include <linux/fpga/fpga-bridge.h>
>> >> > +
>> >> > +#include "dfl.h"
>> >> > +#include "dfl-fme-pr.h"
>> >> > +
>> >> > +struct fme_br_priv {
>> >> > + struct dfl_fme_br_pdata *pdata;
>> >> > + struct dfl_fpga_port_ops *port_ops;
>> >> > + struct platform_device *port_pdev;
>> >> > +};
>> >> > +
>> >> > +static int fme_bridge_enable_set(struct fpga_bridge *bridge, bool enable)
>> >> > +{
>> >> > + struct fme_br_priv *priv = bridge->priv;
>> >> > + struct platform_device *port_pdev;
>> >> > + struct dfl_fpga_port_ops *ops;
>> >> > +
>> >> > + if (!priv->port_pdev) {
>> >> > + port_pdev = dfl_fpga_cdev_find_port(priv->pdata->cdev,
>> >> > + &priv->pdata->port_id,
>> >> > + dfl_fpga_check_port_id);
>> >> > + if (!port_pdev)
>> >> > + return -ENODEV;
>> >> > +
>> >> > + priv->port_pdev = port_pdev;
>> >> > + }
>> >> > +
>> >> > + if (priv->port_pdev && !priv->port_ops) {
>> >> > + ops = dfl_fpga_get_port_ops(priv->port_pdev);
>> >> > + if (!ops || !ops->enable_set)
>> >> > + return -ENOENT;
>> >> > +
>> >> > + priv->port_ops = ops;
>> >> > + }
>> >>
>> >> This is saving some pointers. Is it possible that the port_pdev or
>> >> port_ops could go away?
>> >
>> > Hi Alan
>> >
>> > Thanks for the comments.
>> >
>> > The find_port function will get the port device to prevent that.
>> > You can see put device in remove function. And it's similar for the
>> > port ops. In dfl_fpga_get_port_ops function, it will prevent unexpected
>> > port ops removing by try module get.
>>
>> OK, good, and I see the find_port function documents that put_device
>> is needed, so that's good too.
>>
>> When we previously discussed this [1] you described a procedure of
>> hot-unplugging the VF AFU from the VM and turning it back to PF before
>> doing the FPGA programming. Some comments on that would be helpful,
>> maybe located with the port_ops functions.
>>
>
> Hi Alan
>
> Actually I plan to add those descriptions about virtualization in the
> documenation/fpga/dfl.txt, together with the patch which enables the
> virtualization support for Intel FPGA device (e.g Intel PAC card).
Great
> I
> think dfl.txt is a better place to have that procedure documented.
> How do you think?
If it is only in the dff.txt, it may remain unclear (in the code) that
that is an issue that the code is working around.
>
>
>> [1] https://lkml.org/lkml/2018/4/6/180
>>
>> >
>> >>
>> >> Also, the port ops routines probably should be named
>> >> dfl_fpga_port_ops_get/find_port/etc
>> >>
>> >
>> > Hm.. as I see there are functions named as get_device(), put_device(), so
>> > I just name it as ..._get_port_ops and ..._put_port_ops in similar way. :)
>> > If you think that could be a better name, it's fine for me to change them.
>>
>> Yes, I've seen that too. I think keeping the prefix the same helps to
>> organize the namespace. Plus :) if I'm grepping through the code, I
>> can be lazy and not have to use a regex to find the port ops
>> functions.
>
> so we good with current naming?
Recently GregKH give us some guidance about using a common prefix [2].
>
> some places we find it's get_device() and put_device(), but other functions
> like device_add(), device_del(). actually it's not a big problem I think, :)
> currently port_ops has a unified style ..._add/del/get/put_port_ops. I am
> fine with both kind of naming, if you prefer ...port_ops_add/del/get/put
> style, please let me know, I can change them in the next version.
Since you have struct dfl_fpga_port_ops, you can just add _{action} to
the name of the struct and you have dfl_fpga_port_ops_add/del/get/pu
Thanks,
Alan
[2] https://lkml.org/lkml/2018/3/15/709
>
> Thanks
> Hao
>
>>
>> Alan
>>
>> >
>> > Thanks
>> > Hao
>> >
>> >> Alan
>> >>
>> >> > +
>> >> > + return priv->port_ops->enable_set(priv->port_pdev, enable);
>> >> > +}
>> >> > +
>> >> > +static const struct fpga_bridge_ops fme_bridge_ops = {
>> >> > + .enable_set = fme_bridge_enable_set,
>> >> > +};
>> >> > +
>> >> > +static int fme_br_probe(struct platform_device *pdev)
>> >> > +{
>> >> > + struct device *dev = &pdev->dev;
>> >> > + struct fme_br_priv *priv;
>> >> > + struct fpga_bridge *br;
>> >> > + int ret;
>> >> > +
>> >> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> >> > + if (!priv)
>> >> > + return -ENOMEM;
>> >> > +
>> >> > + priv->pdata = dev_get_platdata(dev);
>> >> > +
>> >> > + br = fpga_bridge_create(dev, "DFL FPGA FME Bridge",
>> >> > + &fme_bridge_ops, priv);
>> >> > + if (!br)
>> >> > + return -ENOMEM;
>> >> > +
>> >> > + platform_set_drvdata(pdev, br);
>> >> > +
>> >> > + ret = fpga_bridge_register(br);
>> >> > + if (ret)
>> >> > + fpga_bridge_free(br);
>> >> > +
>> >> > + return ret;
>> >> > +}
>> >> > +
>> >> > +static int fme_br_remove(struct platform_device *pdev)
>> >> > +{
>> >> > + struct fpga_bridge *br = platform_get_drvdata(pdev);
>> >> > + struct fme_br_priv *priv = br->priv;
>> >> > +
>> >> > + fpga_bridge_unregister(br);
>> >> > +
>> >> > + if (priv->port_pdev)
>> >> > + put_device(&priv->port_pdev->dev);
>> >> > + if (priv->port_ops)
>> >> > + dfl_fpga_put_port_ops(priv->port_ops);
>> >> > +
>> >> > + return 0;
>> >> > +}
>> >> > +
>> >> > +static struct platform_driver fme_br_driver = {
>> >> > + .driver = {
>> >> > + .name = DFL_FPGA_FME_BRIDGE,
>> >> > + },
>> >> > + .probe = fme_br_probe,
>> >> > + .remove = fme_br_remove,
>> >> > +};
>> >> > +
>> >> > +module_platform_driver(fme_br_driver);
>> >> > +
>> >> > +MODULE_DESCRIPTION("FPGA Bridge for DFL FPGA Management Engine");
>> >> > +MODULE_AUTHOR("Intel Corporation");
>> >> > +MODULE_LICENSE("GPL v2");
>> >> > +MODULE_ALIAS("platform:dfl-fme-bridge");
>> >> > --
>> >> > 1.8.3.1
>> >> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: semantics of rhashtable and sysvipc
From: Davidlohr Bueso @ 2018-05-24 17:07 UTC (permalink / raw)
To: Linus Torvalds
Cc: Thomas Graf, Herbert Xu, Andrew Morton, Manfred Spraul,
guillaume.knispel, Linux API, Linux Kernel Mailing List
In-Reply-To: <CA+55aFwAtfiMa22-OFGf1dNR8CNzSKjjLLj5UL-HqgapV7Tf1A@mail.gmail.com>
On Wed, 23 May 2018, Linus Torvalds wrote:
>One option is to make rhashtable_alloc() shrink the allocation and try
>again if it fails, and then you *can* do __GFP_NOFAIL eventually.
The below attempts to implements this, along with converting the EINVAL cases
to WARN_ON().
I've refactored bucket_table_alloc() to add a 'retry' param which always
ends up doing GFP_KERNEL|__GFP_NOFAIL for both the tbl as well as
alloc_bucket_spinlocks(). I've arbitrarily shrunk the size in half upon
initial allocation failure. So we default from 64 to 32 buckets, and if
the user is hinting at larger nelems, we simply disregard that and retry
with 32. Similarly, smaller values are also cut in half.
In addition, I think we can also get rid of explicitly passing the gfp flags
to bucket_table_alloc() (we only do GFP_KERNEL or GFP_ATOMIC), and leave it
up to __bucket_table_alloc() by adding a new bucket_table_alloc_noblock() or
something for GFP_ATOMIC.
>
>In fact, it can validly be argued that rhashtable_init() is just buggy
>as-is. The whole *point* olf that function is to size things appropriately,
>and returning -ENOMEM obviously means that it didn't do its job.
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 9427b5766134..e86a396aebcf 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -166,16 +166,21 @@ static struct bucket_table *nested_bucket_table_alloc(struct rhashtable *ht,
return tbl;
}
-static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
- size_t nbuckets,
- gfp_t gfp)
+static struct bucket_table *__bucket_table_alloc(struct rhashtable *ht,
+ size_t nbuckets,
+ gfp_t gfp, bool retry)
{
struct bucket_table *tbl = NULL;
size_t size, max_locks;
int i;
size = sizeof(*tbl) + nbuckets * sizeof(tbl->buckets[0]);
- if (gfp != GFP_KERNEL)
+ if (retry) {
+ gfp |= __GFP_NOFAIL;
+ tbl = kzalloc(size, gfp);
+ }
+
+ else if (gfp != GFP_KERNEL)
tbl = kzalloc(size, gfp | __GFP_NOWARN | __GFP_NORETRY);
else
tbl = kvzalloc(size, gfp);
@@ -211,6 +216,20 @@ static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
return tbl;
}
+static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
+ size_t nbuckets,
+ gfp_t gfp)
+{
+ return __bucket_table_alloc(ht, nbuckets, gfp, false);
+}
+
+static struct bucket_table *bucket_table_alloc_retry(struct rhashtable *ht,
+ size_t nbuckets,
+ gfp_t gfp)
+{
+ return __bucket_table_alloc(ht, nbuckets, gfp, true);
+}
+
static struct bucket_table *rhashtable_last_table(struct rhashtable *ht,
struct bucket_table *tbl)
{
@@ -1024,12 +1043,11 @@ int rhashtable_init(struct rhashtable *ht,
size = HASH_DEFAULT_SIZE;
- if ((!params->key_len && !params->obj_hashfn) ||
- (params->obj_hashfn && !params->obj_cmpfn))
- return -EINVAL;
+ WARN_ON((!params->key_len && !params->obj_hashfn) ||
+ (params->obj_hashfn && !params->obj_cmpfn));
- if (params->nulls_base && params->nulls_base < (1U << RHT_BASE_SHIFT))
- return -EINVAL;
+ WARN_ON(params->nulls_base &&
+ params->nulls_base < (1U << RHT_BASE_SHIFT));
memset(ht, 0, sizeof(*ht));
mutex_init(&ht->mutex);
@@ -1068,9 +1086,23 @@ int rhashtable_init(struct rhashtable *ht,
}
}
+ /*
+ * This is api initialization. We need to guarantee the initial
+ * rhashtable allocation. Upon failure, retry with a smaller size,
+ * otherwise we exhaust our options with __GFP_NOFAIL.
+ *
+ * The size of the table is shrunk to at least half the original
+ * value. Users that use large nelem_hint values are lowered to 32
+ * buckets.
+ */
tbl = bucket_table_alloc(ht, size, GFP_KERNEL);
- if (tbl == NULL)
- return -ENOMEM;
+ if (unlikely(tbl == NULL)) {
+ size = min(size, HASH_DEFAULT_SIZE) / 2;
+
+ tbl = bucket_table_alloc(ht, size, GFP_KERNEL);
+ if (tbl == NULL)
+ tbl = bucket_table_alloc_retry(ht, size, GFP_KERNEL);
+ }
atomic_set(&ht->nelems, 0);
^ permalink raw reply related
* Re: [RFC PATCH 0/5] kmalloc-reclaimable caches
From: Randy Dunlap @ 2018-05-24 16:18 UTC (permalink / raw)
To: Matthew Wilcox, Vlastimil Babka
Cc: linux-mm, Roman Gushchin, Michal Hocko, Johannes Weiner,
linux-kernel, linux-api, Christoph Lameter, Pekka Enberg,
David Rientjes, Joonsoo Kim, Mel Gorman, Vijayanand Jitta
In-Reply-To: <20180524114350.GA10323@bombadil.infradead.org>
On 05/24/2018 04:43 AM, Matthew Wilcox wrote:
> On Thu, May 24, 2018 at 01:00:06PM +0200, Vlastimil Babka wrote:
>> Now for the issues a.k.a. why RFC:
>>
>> - I haven't find any other obvious users for reclaimable kmalloc (yet)
>
> Is that a problem? This sounds like it's enough to solve Facebook's
> problem.
>
>> - the name of caches kmalloc-reclaimable-X is rather long
>
> Yes; Christoph and I were talking about restricting slab names to 16 bytes
> just to make /proc/slabinfo easier to read. How about
>
> kmalloc-rec-128k
> 1234567890123456
>
> Just makes it ;-)
>
> Of course, somebody needs to do the work to use k/M instead of 4194304.
> We also need to bikeshed about when to switch; should it be:
>
> kmalloc-rec-512
> kmalloc-rec-1024
> kmalloc-rec-2048
> kmalloc-rec-4096
> kmalloc-rec-8192
> kmalloc-rec-16k
>
> or should it be
>
> kmalloc-rec-512
> kmalloc-rec-1k
> kmalloc-rec-2k
> kmalloc-rec-4k
> kmalloc-rec-8k
> kmalloc-rec-16k
>
> I slightly favour the latter as it'll be easier to implement. Something like
Yes, agree, start using the suffix early.
>
> static const char suffixes[3] = ' kM';
> int idx = 0;
>
> while (size > 1024) {
> size /= 1024;
> idx++;
> }
>
> sprintf("%d%c", size, suffices[idx]);
suffixes
>
> --
--
~Randy
^ permalink raw reply
* Re: [RFC PATCH 0/5] kmalloc-reclaimable caches
From: Vlastimil Babka @ 2018-05-24 15:52 UTC (permalink / raw)
To: Roman Gushchin
Cc: linux-mm, Michal Hocko, Johannes Weiner, linux-kernel, linux-api,
Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Mel Gorman, Vijayanand Jitta, Laura Abbott
In-Reply-To: <20180524121347.GA10763@castle.DHCP.thefacebook.com>
On 05/24/2018 02:13 PM, Roman Gushchin wrote:
> On Thu, May 24, 2018 at 01:00:06PM +0200, Vlastimil Babka wrote:
>> Hi,
>>
>> - I haven't find any other obvious users for reclaimable kmalloc (yet)
>
> As I remember, ION memory allocator was discussed related to this theme:
> https://lkml.org/lkml/2018/4/24/1288
+CC Laura
Yeah ION added the NR_INDIRECTLY_RECLAIMABLE_BYTES handling, which is
adjusted to page granularity in patch 4. I'm not sure if it should use
kmalloc as it seems to be allocating order-X pages, where kmalloc/slab
just means extra overhead. But maybe if it doesn't allocate/free too
frequently, it could work?
>> I did a superset as IIRC somebody suggested that in the older threads or at LSF.
>
> This looks nice to me!
>
> Thanks!
>
^ permalink raw reply
* Re: [RFC PATCH 0/5] kmalloc-reclaimable caches
From: Johannes Weiner @ 2018-05-24 15:32 UTC (permalink / raw)
To: Vlastimil Babka
Cc: linux-mm, Roman Gushchin, Michal Hocko, linux-kernel, linux-api,
Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Mel Gorman, Vijayanand Jitta
In-Reply-To: <20180524110011.1940-1-vbabka@suse.cz>
On Thu, May 24, 2018 at 01:00:06PM +0200, Vlastimil Babka wrote:
> - the vmstat/meminfo counter name is rather general and might suggest it also
> includes reclaimable page caches, which it doesn't
>
> Suggestions welcome for all three points. For the last one, we might also keep
> the counter separate from nr_slab_reclaimable, not superset. I did a superset
> as IIRC somebody suggested that in the older threads or at LSF.
Yeah, the "reclaimable" name is too generic. How about KReclaimable?
The counter being a superset sounds good to me. We use this info for
both load balancing and manual debugging. For load balancing code it's
nice not having to worry about finding all the counters that hold
reclaimable memory depending on kernel version; it's always simply
user cache + user anon + kernel reclaimable. And for debugging, we can
always add more specific subset counters later on if we need them.
^ permalink raw reply
* Re: [PATCH 07/24] arm64: ilp32: add documentation on the ILP32 ABI for ARM64
From: Dr. Philipp Tomsich @ 2018-05-24 12:24 UTC (permalink / raw)
To: Yury Norov
Cc: Pavel Machek, Catalin Marinas, Arnd Bergmann, linux-arm-kernel,
LKML, linux-doc, linux-arch, linux-api, Adam Borowski,
Alexander Graf, Alexey Klimov, Andreas Schwab, Andrew Pinski,
Bamvor Zhangjian, Chris Metcalf, Christoph Muellner, Dave Martin,
David S . Miller, Florian Weimer, Geert Uytterhoeven
In-Reply-To: <20180524121524.GA3873@yury-thinkpad>
Yury & Pavel,
> On 24 May 2018, at 14:15, Yury Norov <ynorov@caviumnetworks.com> wrote:
>
> Hi Pavel,
>
> On Wed, May 23, 2018 at 04:06:20PM +0200, Pavel Machek wrote:
>> On Wed 2018-05-16 11:18:52, Yury Norov wrote:
>>> Based on Andrew Pinski's patch-series.
>>>
>>> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
>>
>> So Andrew's signoff should be here?
>
> Yes it should, but it lost since v4. I'll restore it.
>
>>> ---
>>> Documentation/arm64/ilp32.txt | 45 +++++++++++++++++++++++++++++++++++
>>> 1 file changed, 45 insertions(+)
>>> create mode 100644 Documentation/arm64/ilp32.txt
>>>
>>> diff --git a/Documentation/arm64/ilp32.txt b/Documentation/arm64/ilp32.txt
>>> new file mode 100644
>>> index 000000000000..d0fd5109c4b2
>>> --- /dev/null
>>> +++ b/Documentation/arm64/ilp32.txt
>>> @@ -0,0 +1,45 @@
>>> +ILP32 AARCH64 SYSCALL ABI
>>> +=========================
>>> +
>>> +This document describes the ILP32 syscall ABI and where it differs
>>> +from the generic compat linux syscall interface.
>>
>> I was hoping to learn what ILP32 is / what is it good for, but no,
>> this does not tell me... it would be good to do a short explanation
>> here, and maybe reference it from cover letter of the series...
>> Pavel
>
> ILP32 is ABI acronym that means "Integers, Longs and Pointers are 32-bit".
> And LP64 means "Longs and Pointers are 64-bit”.
Just a nitpick: ILP32 is in fact just the memory model, but calling from ILP32
code into the Linux kernel requires modifications to the syscall-ABI due to
datastructure layout changing (every time a pointer or a ‘long’ is present in
a structure). As such structures are passed between the userspace and the
kernel (and also due to the fact that time_t is an ‘unsigned long’ in the C
language standard), modifications to the syscall ABI in Linux are needed to
support ILP32 processes calling into the kernel.
Things get a bit more involved, as the final consensus was to pass 64bit
quantities in the lower half of 2 64bit registers instead of as a single register:
this makes the way (on AArch64) that an ILP32 process calls into the kernel
more dissimilar from a LP64 process calling the same syscall.
What this rambling boils down to is: “ILP32" is the memory model, whereas
this series deals with the “Linux/AArch64 syscall ABI for ILP32 processes”.
Thanks,
Phil.
>
> There's AN490 - "ILP32 for AArch64 Whitepaper" from ARM which covers
> the topic:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dai0490a/ar01s01.html
>
> And some talks:
> http://connect.linaro.org/resource/bkk16/bkk16-305b/
>
> Briefly, ILP32 is 32-bit ABI that works with AARCH64 instruction set. It looks
> better in some performance tests, and is useful for compatibility with 32-bit
> legacy code.
>
> If you're more familiar with x86 terminology, in ARM world LP64 corresponds
> to x86_64, AARCH32_EL0 corresponds to x86_32, and ILP32 corresponds to x32
> ABI.
>
> I'll add link to AN490 in next submission.
>
> Yury
>
>> --
>> (english) http://www.livejournal.com/~pavelmachek
>> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [PATCH 07/24] arm64: ilp32: add documentation on the ILP32 ABI for ARM64
From: Yury Norov @ 2018-05-24 12:15 UTC (permalink / raw)
To: Pavel Machek
Cc: Catalin Marinas, Arnd Bergmann, linux-arm-kernel, linux-kernel,
linux-doc, linux-arch, linux-api, Adam Borowski, Alexander Graf,
Alexey Klimov, Andreas Schwab, Andrew Pinski, Bamvor Zhangjian,
Chris Metcalf, Christoph Muellner, Dave Martin, David S . Miller,
Florian Weimer, Geert Uytterhoeven, Heiko Carstens,
James Hogan <jame>
In-Reply-To: <20180523140620.GA27215@amd>
Hi Pavel,
On Wed, May 23, 2018 at 04:06:20PM +0200, Pavel Machek wrote:
> On Wed 2018-05-16 11:18:52, Yury Norov wrote:
> > Based on Andrew Pinski's patch-series.
> >
> > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
>
> So Andrew's signoff should be here?
Yes it should, but it lost since v4. I'll restore it.
> > ---
> > Documentation/arm64/ilp32.txt | 45 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 45 insertions(+)
> > create mode 100644 Documentation/arm64/ilp32.txt
> >
> > diff --git a/Documentation/arm64/ilp32.txt b/Documentation/arm64/ilp32.txt
> > new file mode 100644
> > index 000000000000..d0fd5109c4b2
> > --- /dev/null
> > +++ b/Documentation/arm64/ilp32.txt
> > @@ -0,0 +1,45 @@
> > +ILP32 AARCH64 SYSCALL ABI
> > +=========================
> > +
> > +This document describes the ILP32 syscall ABI and where it differs
> > +from the generic compat linux syscall interface.
>
> I was hoping to learn what ILP32 is / what is it good for, but no,
> this does not tell me... it would be good to do a short explanation
> here, and maybe reference it from cover letter of the series...
> Pavel
ILP32 is ABI acronym that means "Integers, Longs and Pointers are 32-bit".
And LP64 means "Longs and Pointers are 64-bit".
There's AN490 - "ILP32 for AArch64 Whitepaper" from ARM which covers
the topic:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dai0490a/ar01s01.html
And some talks:
http://connect.linaro.org/resource/bkk16/bkk16-305b/
Briefly, ILP32 is 32-bit ABI that works with AARCH64 instruction set. It looks
better in some performance tests, and is useful for compatibility with 32-bit
legacy code.
If you're more familiar with x86 terminology, in ARM world LP64 corresponds
to x86_64, AARCH32_EL0 corresponds to x86_32, and ILP32 corresponds to x32
ABI.
I'll add link to AN490 in next submission.
Yury
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [RFC PATCH 0/5] kmalloc-reclaimable caches
From: Roman Gushchin @ 2018-05-24 12:13 UTC (permalink / raw)
To: Vlastimil Babka
Cc: linux-mm, Michal Hocko, Johannes Weiner, linux-kernel, linux-api,
Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Mel Gorman, Vijayanand Jitta
In-Reply-To: <20180524110011.1940-1-vbabka@suse.cz>
On Thu, May 24, 2018 at 01:00:06PM +0200, Vlastimil Babka wrote:
> Hi,
>
> as discussed at LSF/MM [1] here's a RFC patchset that introduces
> kmalloc-reclaimable caches (more details in the first patch) and uses them
> for SLAB freelists and dcache external names. The latter allows us to
> repurpose the NR_INDIRECTLY_RECLAIMABLE_BYTES counter later in the series.
>
> This is how /proc/slabinfo looks like after booting in virtme:
>
> ...
> kmalloc-reclaimable-4194304 0 0 4194304 1 1024 : tunables 1 1 0 : slabdata 0 0 0
> ...
> kmalloc-reclaimable-96 17 64 128 32 1 : tunables 120 60 8 : slabdata 2 2 0
> kmalloc-reclaimable-64 50 128 64 64 1 : tunables 120 60 8 : slabdata 2 2 6
> kmalloc-reclaimable-32 0 0 32 124 1 : tunables 120 60 8 : slabdata 0 0 0
> kmalloc-4194304 0 0 4194304 1 1024 : tunables 1 1 0 : slabdata 0 0 0
> ...
> kmalloc-64 2888 2944 64 64 1 : tunables 120 60 8 : slabdata 46 46 454
> kmalloc-32 4325 4712 32 124 1 : tunables 120 60 8 : slabdata 38 38 563
> kmalloc-128 1178 1216 128 32 1 : tunables 120 60 8 : slabdata 38 38 114
> ...
>
> /proc/vmstat with new/renamed nr_reclaimable counter (patch 4):
>
> ...
> nr_slab_reclaimable 2817
> nr_slab_unreclaimable 1781
> ...
> nr_reclaimable 2817
> ...
>
> /proc/meminfo with exposed nr_reclaimable counter (patch 5):
>
> ...
> AnonPages: 8624 kB
> Mapped: 3340 kB
> Shmem: 564 kB
> Reclaimable: 11272 kB
> Slab: 18368 kB
> SReclaimable: 11272 kB
> SUnreclaim: 7096 kB
> KernelStack: 1168 kB
> PageTables: 448 kB
> ...
>
> Now for the issues a.k.a. why RFC:
>
> - I haven't find any other obvious users for reclaimable kmalloc (yet)
As I remember, ION memory allocator was discussed related to this theme:
https://lkml.org/lkml/2018/4/24/1288
> I did a superset as IIRC somebody suggested that in the older threads or at LSF.
This looks nice to me!
Thanks!
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox