Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1740389 > unrolled thread
| Started by | Gargi Sharma <gs051095@gmail.com> |
|---|---|
| First post | 2017-09-27 07:10 +0200 |
| Last post | 2017-09-29 02:40 +0200 |
| Articles | 4 on this page of 24 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH v2 0/2] Replace PID bitmap allocation with IDR API Gargi Sharma <gs051095@gmail.com> - 2017-09-27 07:10 +0200
[PATCH v2 1/2] pid: Replace pid bitmap implementation with IDR API Gargi Sharma <gs051095@gmail.com> - 2017-09-27 07:10 +0200
Re: [PATCH v2 1/2] pid: Replace pid bitmap implementation with IDR API Rik van Riel <riel@surriel.com> - 2017-09-27 15:20 +0200
Re: [PATCH v2 1/2] pid: Replace pid bitmap implementation with IDR API Oleg Nesterov <oleg@redhat.com> - 2017-09-27 16:10 +0200
Re: [PATCH v2 1/2] pid: Replace pid bitmap implementation with IDR API Gargi Sharma <gs051095@gmail.com> - 2017-09-27 17:10 +0200
Re: [PATCH v2 1/2] pid: Replace pid bitmap implementation with IDR API Oleg Nesterov <oleg@redhat.com> - 2017-09-27 17:20 +0200
Re: [PATCH v2 1/2] pid: Replace pid bitmap implementation with IDR API Gargi Sharma <gs051095@gmail.com> - 2017-09-27 17:10 +0200
Re: [PATCH v2 1/2] pid: Replace pid bitmap implementation with IDR API Oleg Nesterov <oleg@redhat.com> - 2017-09-27 17:10 +0200
Re: [PATCH v2 1/2] pid: Replace pid bitmap implementation with IDR API Christoph Hellwig <hch@infradead.org> - 2017-10-01 11:20 +0200
Re: [PATCH v2 1/2] pid: Replace pid bitmap implementation with IDR API Gargi Sharma <gs051095@gmail.com> - 2017-10-01 12:40 +0200
Re: [PATCH v2 1/2] pid: Replace pid bitmap implementation with IDR API Rik van Riel <riel@surriel.com> - 2017-10-02 15:20 +0200
Re: [PATCH v2 1/2] pid: Replace pid bitmap implementation with IDR API Rik van Riel <riel@surriel.com> - 2017-10-02 15:10 +0200
[PATCH v2 2/2] pid: Remove pidhash Gargi Sharma <gs051095@gmail.com> - 2017-09-27 07:10 +0200
Re: [PATCH v2 2/2] pid: Remove pidhash Oleg Nesterov <oleg@redhat.com> - 2017-09-27 17:50 +0200
Re: [PATCH v2 2/2] pid: Remove pidhash Oleg Nesterov <oleg@redhat.com> - 2017-09-27 18:30 +0200
Re: [PATCH v2 2/2] pid: Remove pidhash Gargi Sharma <gs051095@gmail.com> - 2017-09-30 17:50 +0200
Re: [PATCH v2 2/2] pid: Remove pidhash Rik van Riel <riel@surriel.com> - 2017-10-02 22:50 +0200
Re: [PATCH v2 2/2] pid: Remove pidhash Gargi Sharma <gs051095@gmail.com> - 2017-10-02 22:50 +0200
Re: [PATCH v2 2/2] pid: Remove pidhash Oleg Nesterov <oleg@redhat.com> - 2017-10-02 22:50 +0200
Re: [PATCH v2 2/2] pid: Remove pidhash Rik van Riel <riel@surriel.com> - 2017-10-02 22:50 +0200
Re: [PATCH v2 0/2] Replace PID bitmap allocation with IDR API ebiederm@xmission.com (Eric W. Biederman) - 2017-09-27 19:20 +0200
Re: [PATCH v2 0/2] Replace PID bitmap allocation with IDR API Rik van Riel <riel@surriel.com> - 2017-09-28 21:50 +0200
Re: [PATCH v2 0/2] Replace PID bitmap allocation with IDR API Gargi Sharma <gs051095@gmail.com> - 2017-09-28 22:10 +0200
Re: [PATCH v2 0/2] Replace PID bitmap allocation with IDR API Rik van Riel <riel@surriel.com> - 2017-09-29 02:40 +0200
Page 2 of 2 — ← Prev page 1 [2]
| From | ebiederm@xmission.com (Eric W. Biederman) |
|---|---|
| Date | 2017-09-27 19:20 +0200 |
| Message-ID | <uuoa5-60L-5@gated-at.bofh.it> |
| In reply to | #1740389 |
Gargi Sharma <gs051095@gmail.com> writes: > This patch series replaces kernel bitmap implementation of PID allocation > with IDR API. These patches are written to simplify the kernel by replacing custom code with calls to generic code. > > The following are the stats for pid and pid_namespace object files > before and after the replacement. There is a noteworthy change between > the IDR and bitmap implementation. > > Before > text data bss dec hex filename > 8447 3894 64 12405 3075 kernel/pid.o > After > text data bss dec hex filename > 3301 304 0 3605 e15 kernel/pid.o > > Before > text data bss dec hex filename > 5692 1842 192 7726 1e2e kernel/pid_namespace.o > After > text data bss dec hex filename > 2870 216 16 3102 c1e kernel/pid_namespace.o > > There wasn't a considerable difference between the time required for > allocation of PIDs to the processes. How about the time to call readdir on /proc? How many processes were you testing with? Why just the bitmap? Why not update the hash table as well. An rbtree or an rhashtable might be better. Hmm. Oh look there is patch 2/2 and you do replace the hashtable with the idr as well. Now I am very interested in a comparison of data structures. How does the runtime memory footprint of your new pid hash implementation compare to the old memory footprint? There are a lot of options in this space and it does not sound like you have looked at the options very thoroughly. I am a little worried that in the quest to reuse code you may have made the total amount of code executed larger and more susceptible to more cache line misses. From what Oleg has pointed out and from your the holes in your description I am generally leery of this patchset as the attention to detail was appears lower than necessary for this to be more than a proof of concept. Eric > --- > Changes in v2: > - Removed redundant IDR function that was introduced > in the previous patchset. > - Renamed PIDNS_HASH_ADDING > - Used idr_for_each_entry_continue() > - Used idr_find() to lookup pids > > Gargi Sharma (2): > pid: Replace pid bitmap implementation with IDR API > pid: Remove pidhash > > arch/powerpc/platforms/cell/spufs/sched.c | 2 +- > fs/proc/loadavg.c | 2 +- > include/linux/init_task.h | 1 - > include/linux/pid.h | 2 - > include/linux/pid_namespace.h | 18 +-- > init/main.c | 3 +- > kernel/fork.c | 2 +- > kernel/pid.c | 239 +++++------------------------- > kernel/pid_namespace.c | 54 +++---- > 9 files changed, 68 insertions(+), 255 deletions(-)
[toc] | [prev] | [next] | [standalone]
| From | Rik van Riel <riel@surriel.com> |
|---|---|
| Date | 2017-09-28 21:50 +0200 |
| Message-ID | <uuMYN-4HS-5@gated-at.bofh.it> |
| In reply to | #1740895 |
[Multipart message — attachments visible in raw view] — view raw
On Fri, 2017-09-29 at 01:09 +0530, Gargi Sharma wrote: > 1000 processes that just sleep and sit around without doing > anything(100 second sleep and then exit). > > pstree with 10,000 processes > real 0m0.859s > user 0m0.536s > sys 0m0.172s > > ps with 10,000 processes > real 0m0.918s > user 0m0.100s > sys 0m0.172s > > Stats for calling readdir on /proc with 10,000 processes > real 0m0.092s > user 0m0.000s > sys 0m0.020s Is that with or without your patches? How does it compare to a kernel with(out) your patches? -- All Rights Reversed.
[toc] | [prev] | [next] | [standalone]
| From | Gargi Sharma <gs051095@gmail.com> |
|---|---|
| Date | 2017-09-28 22:10 +0200 |
| Message-ID | <uuNia-536-11@gated-at.bofh.it> |
| In reply to | #1741798 |
On Thu, Sep 28, 2017 at 3:46 PM, Rik van Riel <riel@surriel.com> wrote: > On Fri, 2017-09-29 at 01:09 +0530, Gargi Sharma wrote: > >> 1000 processes that just sleep and sit around without doing >> anything(100 second sleep and then exit). >> >> pstree with 10,000 processes >> real 0m0.859s >> user 0m0.536s >> sys 0m0.172s >> >> ps with 10,000 processes >> real 0m0.918s >> user 0m0.100s >> sys 0m0.172s >> >> Stats for calling readdir on /proc with 10,000 processes >> real 0m0.092s >> user 0m0.000s >> sys 0m0.020s > > Is that with or without your patches? > > How does it compare to a kernel with(out) your patches? Ah thanks for pointing this out. Those were without the patches. Here are the stats for easier comparison. With Patches Without patches pstree real 0m0.542s real 0m0.859s user 0m0.335s user 0m0.536s sys 0m0.150s sys 0m0.172s ps real 0m0.722s real 0m0.918s user 0m0.064s user 0m0.100s sys 0m0.162s sys 0m0.172s readdir real 0m0.080s real 0m0.092s user 0m0.000s user 0m0.000s sys 0m0.021s sys 0m0.020s Thanks! Gargi > > -- > All Rights Reversed. On Fri, Sep 29, 2017 at 1:16 AM, Rik van Riel <riel@surriel.com> wrote: > On Fri, 2017-09-29 at 01:09 +0530, Gargi Sharma wrote: > >> 1000 processes that just sleep and sit around without doing >> anything(100 second sleep and then exit). >> >> pstree with 10,000 processes >> real 0m0.859s >> user 0m0.536s >> sys 0m0.172s >> >> ps with 10,000 processes >> real 0m0.918s >> user 0m0.100s >> sys 0m0.172s >> >> Stats for calling readdir on /proc with 10,000 processes >> real 0m0.092s >> user 0m0.000s >> sys 0m0.020s > > Is that with or without your patches? > > How does it compare to a kernel with(out) your patches? > > -- > All Rights Reversed.
[toc] | [prev] | [next] | [standalone]
| From | Rik van Riel <riel@surriel.com> |
|---|---|
| Date | 2017-09-29 02:40 +0200 |
| Message-ID | <uuRvs-7tU-15@gated-at.bofh.it> |
| In reply to | #1741803 |
[Multipart message — attachments visible in raw view] — view raw
On Fri, 2017-09-29 at 01:35 +0530, Gargi Sharma wrote: > On Thu, Sep 28, 2017 at 3:46 PM, Rik van Riel <riel@surriel.com> > wrote: > > On Fri, 2017-09-29 at 01:09 +0530, Gargi Sharma wrote: > > > > > 1000 processes that just sleep and sit around without doing > > > anything(100 second sleep and then exit). > > > > Is that with or without your patches? > > > > How does it compare to a kernel with(out) your patches? > > Ah thanks for pointing this out. Those were without the patches. > Here are the stats for easier comparison. > > With Patches Without patches > pstree > real 0m0.542s real 0m0.859s > user 0m0.335s user 0m0.536s > sys 0m0.150s sys 0m0.172s > > ps > real 0m0.722s real 0m0.918s > user 0m0.064s user 0m0.100s > sys 0m0.162s sys 0m0.172s > > readdir > real 0m0.080s real 0m0.092s > user 0m0.000s user 0m0.000s > sys 0m0.021s sys 0m0.020s So your patches speed up the use of /proc? I suspect pstree and ps benefit from the simplification and speedup of find_pid_ns, which is called from find_task_by_pid_ns. That is great news. -- All Rights Reversed.
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web