Messages in this thread | | | Date | Fri, 26 Jul 2013 21:04:32 +0900 | From | Hidehiro Kawai <> | Subject | Re: [RESEND RFC PATCH 0/5] Add a hash value for each line in /dev/kmsg |
| |
Hello,
(2013/07/25 23:56), Kazuo Ito wrote:> Hello, > > On Thu, Jul 25, 2013 at 5:37 PM, Hidehiro Kawai < > hidehiro.kawai.ez@hitachi.com> wrote: > >> .gitignore | 1 >> Makefile | 7 + >> drivers/base/core.c | 57 ++------- >> drivers/usb/storage/debug.c | 2 >> include/asm-generic/vmlinux.lds.h | 5 + >> include/linux/device.h | 73 +++++++---- >> include/linux/kmsghash.h | 19 +++ >> include/linux/printk.h | 31 ++++- >> kernel/printk.c | 81 +++++++++--- >> lib/Kconfig.debug | 22 +++ >> lib/dynamic_debug.c | 4 - >> net/core/dev.c | 2 >> scripts/Makefile | 1 >> scripts/link-vmlinux.sh | 5 + >> scripts/msghash/.gitignore | 1 >> scripts/msghash/Makefile | 7 + >> scripts/msghash/msghash.c | 241 >> +++++++++++++++++++++++++++++++++++++ >> scripts/msghash/msghash.sh | 45 +++++++ >> tools/include/tools/jhash.h | 192 +++++++++++++++++++++++++++++ >> 19 files changed, 698 insertions(+), 98 deletions(-) >> create mode 100644 include/linux/kmsghash.h >> create mode 100644 scripts/msghash/.gitignore >> create mode 100644 scripts/msghash/Makefile >> create mode 100644 scripts/msghash/msghash.c >> create mode 100755 scripts/msghash/msghash.sh >> create mode 100644 tools/include/tools/jhash.h >> > > As for one that once tried to implement a similar idea (in vein > at LinuxCon Japan a couple of years ago), I heartily welcome this patch > while feeling amount of changes might be perceived to be too much, > requiring some tradeoff analysis with other implementation options.
I agreed. The current implementation has advantages and drawbacks, and there are some implementation options. We have to find a convincing way.
> This patch precalculates hash values and wraps > callers of printk around in macros so that it hardly ever > causes runtime overhead, it could avoid hash collisions > if needs be, and many creative uses of printk > can be dealt with, in particular nested formats in %pV, > which was my downfall... The cost is numbers of required changes. > > Calculating hash values in vprink_* would make spreading changes > across all the printk-derived functions and macros unnecessary, > but would preclude use of elaborate hash functions and, in cases > like dev_printk(), would require a way to find or extract > an appropriate format string. The latter requires complex pattern > matching, lots of guess-work, has to deal with %pV, and will never > be perfect.
Actually, in my first prototype implementation (but not published), hash values are calculated dynamically when vprintk_emit is called. Changed lines are less than this implementation, and it can also handle %pV cases. But as you say, extracting messages and calculating hashes preliminarily is the difficult part. I thought this difficulty will make people not use this feature. So I took a way manipulating metadata in printk macros.
Regards, -- Hidehiro Kawai Hitachi, Yokohama Research Laboratory Linux Technology Center
| |