public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Octavian Purdila <tavi@cs.pub.ro>,
	"David S . Miller" <davem@davemloft.net>
Subject: Re: [v2 000/115] faster tree-based sysctl implementation
Date: Sun, 08 May 2011 20:11:00 -0700	[thread overview]
Message-ID: <m1ei48zhyz.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <1304894407-32201-1-git-send-email-lucian.grijincu@gmail.com> (Lucian Adrian Grijincu's message of "Mon, 9 May 2011 00:38:12 +0200")

Lucian Adrian Grijincu <lucian.grijincu@gmail.com> writes:

> Eric also asked for:
> - replacing the per-header list of subdirs with a rbtree.
>   -- Again, lack of time, and this can always be added at a later time
>      to optimize lookup and duplicate checks. At the moment this patch
>      series does not add a complexity regression over the previous
>      implementation, au contraire.

Instead you add a dubious mechanisms, to avoid duplicates checks
because the duplicate checks takes time.

A good internal data structure should have logN duplicate checks
and that should not be cost prohibitive. For maintainability of
the users a good internal data structure removes the need for
special cases, and it actually makes use the sysctl fast.
Your implementation continues to have a O(N^2) cost to use the
sysctl files.

Getting a good internal data structure is the most important part
of a making sysctls fast, and keeping them fast.  Your patchset
fails in that.

So at that level, a great big NAK on the patchset as it currently
exists.


You have made a key observation that we only need better data
structures for the directories and can leave off that work
for the callers.

Thank you for that.

Additionally thank you for splitting up your changes to the
core of sysctl so there is a chance of reviewing those.

With respect to review and merging.  The priorities should be:
1) Figure out what the users really need to do, before we change
   everything so we only need one sweeping pass through the users
   that hopefully simplifies them.

2) Ensure you have a version of the new interfaces ready at the
   start of the patchset, so the sweeping changes can be made
   incrementally.

3) Clean up the users.

4) Make simplifications because you don't have any old users left.

I very much agree that the data structures that sysctl uses today
are far from ideal, and that we can make a lot of headway.

That said I think there is some good work in your patchset and I may
try and cherry pick out the good bits that we can merge now.

Eric

  parent reply	other threads:[~2011-05-09  3:11 UTC|newest]

Thread overview: 100+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-08 22:38 [v2 000/115] faster tree-based sysctl implementation Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 001/115] sysctl: remove .child from dev/parport/default Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 002/115] sysctl: parport: reorder .child assignments to simplify review Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 003/115] sysctl: remove .child from dev/parport/PORT/devices/DEVICE Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 004/115] sysctl: remove .child from dev/parport/PORT/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 005/115] sysctl: remove .child from dev/parport/PORT/devices/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 006/115] sysctl: remove .child from kernel/vsyscall64 (x86) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 007/115] sysctl: remove .child from abi/vsyscall32 (x86) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 008/115] sysctl: remove .child from crypto/fips_enabled Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 009/115] sysctl: remove .child from dev/cdrom/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 010/115] sysctl: remove .child from dev/hpet/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 011/115] sysctl: remove .child from dev/ipmi/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 012/115] sysctl: remove .child from dev/rtc/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 013/115] sysctl: remove .child from dev/mac_hid/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 014/115] sysctl: remove .child from dev/raid/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 015/115] sysctl: remove .child from xpc/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 016/115] sysctl: remove .child from xpc/hb Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 017/115] sysctl: remove .child from kernel/sclp (s390) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 018/115] sysctl: remove .child from dev/scsi Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 019/115] sysctl: remove .child from kernel/pty Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 020/115] sysctl: remove .child from coda/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 021/115] sysctl: remove .child from fscache/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 022/115] sysctl: remove .child from fs/nfs/ nlm_table table Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 023/115] sysctl: remove .child from fs/nfs/ nfs_cb_table Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 024/115] sysctl: remove .child from fs/ntfs-debug Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 025/115] sysctl: remove .child from fs/ocfs2/nm/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 026/115] sysctl: remove .child from fs/quota/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 027/115] sysctl: remove .child from fs/xfs/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 028/115] sysctl: remove .child from kernel/ (ipc) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 029/115] sysctl: remove .child from fs/mqueue Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 030/115] sysctl: sched: add sd_table_template Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 031/115] sysctl: remove .child from kernel/sched_domain/cpuX/domainY/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 032/115] sysctl: remove .child from kernel/ (utsname) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 033/115] sysctl: remove .child from sunrpc/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 034/115] sysctl: remove .child from sunrpc/svc_rdma Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 035/115] sysctl: remove .child from sunrpc/ (xprtrdma) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 036/115] sysctl: remove .child from sunrpc/ (xprtsock) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 037/115] sysctl: remove .child from bus/isa/ (arm) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 038/115] sysctl: remove .child from reboot/warm (arm) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 039/115] sysctl: remove .child from lasat/ (mips) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 040/115] sysctl: remove .child from appldata/ (s390) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 041/115] sysctl: remove .child from s390dbf/ Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 042/115] sysctl: remove .child from vm/ (s390) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 043/115] sysctl: remove .child from kernel/perfmon/ (ia64) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 044/115] sysctl: remove .child from kernel/ (ia64/kdump) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 045/115] sysctl: remove .child from kernel/powersave-nap (powerpc) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 046/115] sysctl: remove .child from pm/ (frv) Lucian Adrian Grijincu
2011-05-08 22:38 ` [v2 047/115] sysctl: remove .child from frv/ Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 048/115] sysctl: remove .child from sh64/unaligned_fixup/ Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 049/115] sysctl: delete unused register_sysctl_table function Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 050/115] sysctl: remove .child from ax25 table Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 051/115] sysctl: remove .child from net/ipv4/route and net/ipv4/neigh tables Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 052/115] sysctl: remove .child from net/ipv4/neigh table Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 053/115] sysctl: remove .child from net/ipv6/route, net/ipv6/icmp, net/ipv6 tables Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 054/115] sysctl: remove .child from net/llc tables Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 055/115] sysctl: call sysctl_init before the first sysctl registration Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 056/115] sysctl: no-child: manually register kernel/random Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 057/115] sysctl: no-child: manually register kernel/keys Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 058/115] sysctl: no-child: manually register fs/inotify Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 059/115] sysctl: no-child: manually register fs/epoll Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 060/115] sysctl: no-child: manually register root tables Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 061/115] sysctl: faster reimplementation of sysctl_check_table Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 062/115] sysctl: remove useless ctl_table->parent field Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 063/115] sysctl: simplify find_in_table Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 064/115] sysctl: sysctl_head_grab defaults to root header on NULL Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 065/115] sysctl: delete useless grab_header function Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 066/115] sysctl: rename ->used to ->ctl_use_refs Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 067/115] sysctl: rename sysctl_head_grab/finish to sysctl_use_header/unuse Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 068/115] sysctl: rename sysctl_head_next to sysctl_use_next_header Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 069/115] sysctl: split ->count into ctl_procfs_refs and ctl_header_refs Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 070/115] sysctl: rename sysctl_head_get/put to sysctl_proc_inode_get/put Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 071/115] sysctl: rename (un)use_table to __sysctl_(un)use_header Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 072/115] sysctl: simplify ->permissions hook Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 073/115] sysctl: group root-specific operations Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 074/115] sysctl: introduce ctl_table_group Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 075/115] sysctl: move removal from list out of start_unregistering Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 076/115] sysctl: faster tree-based sysctl implementation Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 077/115] sysctl: add duplicate entry and sanity ctl_table checks Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 078/115] sysctl: alloc ctl_table_header with kmem_cache Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 079/115] sysctl: single subheader path: optimisation for paths used only once Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 080/115] sysctl: single subheader path: net/ipv4/conf/DEVICE-NAME/ Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 081/115] sysctl: single subheader path: net/{ipv4|ipv6}/neigh/DEV/ Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 082/115] sysctl: single subheader path: net/ipv6/conf/DEVICE-NAME/ Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 083/115] sysctl: single subheader path: dev/parport/PORT/devices/DEVICE/ Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 084/115] sysctl: single subheader path: net/ax25/DEVICE Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 085/115] sysctl: single subheader path: kernel/sched_domain/CPU/DOMAIN/ Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 086/115] sysctl: single subheader path: net/decnet/conf/DEVNAME Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 087/115] sysctl: check netns-specific registration order respected Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 088/115] RFC: sysctl: convert read-write lock to RCU Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 089/115] RFC: sysctl: change type of ctl_procfs_refs to u8 Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 090/115] sysctl: warn if registration/unregistration order is not respected Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 091/115] sysctl: add register_sysctl_dir: register an empty sysctl directory Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 092/115] sysctl: sched: create empty dir with register_sysctl_dir Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 093/115] sysctl: ax25: " Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 094/115] sysctl: net/core: " Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 095/115] sysctl: net/ipv4/neigh: " Lucian Adrian Grijincu
2011-05-08 22:39 ` [v2 096/115] sysctl: net/ipv6/neigh: " Lucian Adrian Grijincu
2011-05-08 22:40 ` [v2 000/115] faster tree-based sysctl implementation David Miller
2011-05-09  3:11 ` Eric W. Biederman [this message]
2011-05-10  1:06   ` Lucian Adrian Grijincu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1ei48zhyz.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=adobriyan@gmail.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucian.grijincu@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=tavi@cs.pub.ro \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox