[lkml]   [2007]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 0/4] coredump: core dump masking support v4
    Thank you for your kind comments.
    I'm sorry for my late reply.

    Andrew Morton wrote:

    > On Fri, 02 Mar 2007 13:41:30 +0900
    > "Kawai, Hidehiro" <> wrote:
    >>This patch series is version 4 of the core dump masking feature,
    >>which provides a per-process flag not to dump anonymous shared
    >>memory segments.
    > First up, please convince us that this problem cannot be solved in
    > userspace.
    > Note that we now support dumping core over a pipe to a
    > userspace application which can perform filtering such as this (see
    > Documentation/sysctl/kernel.txt:core_pattern).

    I understand. Thank you for your suggestion. I'll reply about it in
    another mail, but it may take a few days.

    > Assuming that your argument is successful...
    > - The unpleasing trylock in proc_coredump_omit_anon_shared_write() is
    > there, I believe, to handle the case where a coredump is presently in
    > progress. We don't want to change the filtering rule while the dump is
    > happening.
    > What I suggest you do instead is to take a copy of
    > mm->coredump_omit_anon_shared into a local variable with one single read
    > per coredump. Pass that local down into all the callees which need to
    > see it. That way, no locking is needed.

    Previous v3 patchset does what you suggest, and here are links to the

    [PATCH 2/4] coredump: ELF: enable to omit anonymous shared memory

    [PATCH 3/4] coredump: ELF-FDPIC: enable to omit anonymous shared memory

    However, there was an opposite opinion. To pass the flag status, I
    added omit_anon_shared argument to elf_fdpic_dump_segments(). Then,
    David pointed that the argument was unncecessary, because the function
    also receives mm_struct *mm which includes coredump_omit_anon_shared.
    But mm->coredump_omit_anon_shared can be changed while core dumping, and
    it may causes the core file to be corrupted. So in v4 patchset I used
    r/w semaphore to prevent mm->coredump_omit_anon_shared from being changed.

    If I add an addtional argument to elf_fdpic_dump_segments() again, I
    have to explain it to David. I'll tell him that removing mm argument
    from the function will be a solution since it refers current->mm directly
    and the mm argument is never used.

    > - These games we're playing with the atomicity of the bitfields in the
    > mm_struct need to go away.
    > First up, please prepare a standalone patch which removes
    > mm_struct.dumpable and adds `unsigned long mm_struct.flags'. Include a
    > comment telling people that they must use atomic bitops (set_bit, clear_bit) on
    > mm_struct.flags.

    OK. I'll do it in the next version.

    > - Finally, the code as you have it here is very specific to your specific
    > requirement: don't dump shared memory segments.
    > But if we're going to implement in-kernel core-dump filtering of this
    > nature, we should design it extensibly, even if we don't actually
    > implement those extensions at this time.

    I understood. Since I had done so initially, I'll revert it to.

    > Because other people might (reasonably) wish to omit anonymous memory,
    > or private mappings, or file-backed VMAs, or whatever.
    > So maybe /proc/pid/coredump_omit_anon_shared should become
    > /proc/pid/core_dumpfilter, which is a carefully documented bitmask.

    There are people who wish to dump VMAs which are not dumped by default.
    Taking this into account, some bits of core_dumpfilter will be set by
    default. This means users have to be aware of the default bitmask
    when they change the bitmask. Perhaps changing the bitmask requires
    3 steps:

    1. read the default bitmask
    2. change bits of the mask
    3. write it to the proc entry

    So I think it is better if we provide /proc/pid/core_flags (default:
    all bits are 0) instead of core_dumpfilter. With this interface,
    users who use only one bit of the bitmask (this will be a common case)
    just have to write 2^n to the proc entry. It takes only one step:

    1. write a value to the proc entry

    If we can implement at the same cost, core_flags will be better
    because it is useful for users. What would you think about that?

    By the way, Robin Holt wrote as follows:

    > Can you make this a little more transparent? Having a magic bitmask does
    > not seem like the best way to do stuff. Could you maybe make a core_flags
    > directory with a seperate file for each flag. It could still map to a
    > single field in the mm, but be broken out for the proc filesystem.

    Do you think Robin's suggestion is acceptable?

    Best regards,
    Hidehiro Kawai
    Hitachi, Ltd., Systems Development Laboratory

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2007-03-28 14:41    [W:0.027 / U:0.576 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site