Re: [PATCH v8] coredump: Add bit 9 of coredump_filter for pre-exit files before dumping
Mateusz Guzik <[email protected]>
| Newsgroups | org.kernel.vger.linux-arch,org.kernel.vger.linux-doc,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <CAGudoHG8ff=8RA+8e6JUtESDxBHFjZ65UiCaM4X_6qSU6-Q9Nw@mail.gmail.com> |
On Tue, Aug 11, 2026 at 11:09 AM Christian Brauner <[email protected]> wrote: > > On Sat, Aug 08, 2026 at 01:27:32PM +0800, Xin Zhao wrote: > > A coredump typically takes seconds or even longer to complete. If we > > happen to hold a write lock with flock just before triggering the > > coredump, that write lock will not be released during the entire coredump > > process. As a result, other processes attempting to acquire the same write > > lock may experience significant delays. Another typical scenario is that > > some custom management modules for shared memory also need to release the > > reference counts of the related buffers as soon as possible, rather than > > waiting until the coredump is complete. > > > > Add a new bit(9) of coredump_filter to tag whether need to dump fd list. > > We set it by default because tools like systemd-coredump go through the > > fds. Some other coredump pipe programs like minicoredump do not use fds by > > default. If you are sure that your coredump backend does not use the fds, > > you can clear bit 9, which will allow some file resources without VMA > > references to be released earlier. > > > > In fput(), check FP_DUMPCORE task flags to NOT release file by task work, > > otherwise file put operation will NOT execute util coredump finish. > > > > Test Case One - flock > > Test program send signal SIGABRT to the program which owns the flock, > > output the wait time(unit ms) to successfully attach the flock. > > Test program malloc 500MB heap and memset it. > > If NOT set bit9 of coredump_filter, waitms is 11280. > > If set bit9 of coredump_filter, waitms is 0. > > > > Test Case Two - ion buffer > > Test programs include ion buffer publisher and ion buffer subscriber. > > Ion buffer publisher output the ion buffer hold_time if the subscriber > > NOT send ack to publisher and NOT release it. The subscriber will trig > > coredump by itself in some time. > > If NOT set bit9 of subscriber coredump_filter, max hold_time is 19591ms. > > If set bit9 of subscriber coredump_filter, max hold_time is 320ms. > > > > Signed-off-by: Xin Zhao <[email protected]> > > --- > > > > Change in v8: > > - Delete the duplicate description of default value of coredump_filter in > > proc.rst, as suggested by Randy Dunlap. > > > > Change in v7: > > - Fix description error in proc.rst. Since bit9 has been added, the > > supported memory types should now be 10, > > as suggested by David Hildenbrand (Arm). > > - Link to v7: https://lore.kernel.org/all/[email protected]/ > > > > Change in v6: > > - Fix operator precedence in PF_KTHREAD/PF_DUMPCORE check. > > - Link to v6: https://lore.kernel.org/all/[email protected]/ > > > > Change in v5: > > - Not add another bootargs for the feature, > > as suggested by Christian Brauner and Lorenzo Stoakes. > > Add bit9 of coredump_filter to tag whether need to dump fd list. > > Set bit9 to 1 as default. > > - Al Viro, Christian Brauner and Lorenzo Stoakes point out so many > > problems of the code related to umap that was added in v4, delete all of > > it which is unnecessary. The management of reference counting for shared > > memory generally does not need to be released through the release > > operation of files that have VMA references. Traversing all the threads > > within the process and executing exit_files() is sufficient. > > - Fulfill comments and commit log, > > as suggested by Pedro Falcato and Lorenzo Stoakes. > > - Link to v5: https://lore.kernel.org/all/[email protected]/ > > > > Change in v4: > > - Christian pointed out that the coredump process will traverse file > > descriptors (fd), so certain fds should not be closed by default. > > Rework the whole feature, add /proc/<pid>/coredump_pre_exit for user > > pre-exit resources selection, default is NOT pre-exit anything. > > - Mateusz suggested that walking the fd table and release the file-lock is > > reasonable. No longer release all the fd(s). Based on user config, only > > the flock fd(s) and the fd(s) correspondent to file-backed shared memory > > will be released at most. > > - Link to v4: https://lore.kernel.org/all/[email protected]/ > > > > Change in v3: > > - Add comment and commit-log to explain why do the MMF_DUMP_MAPPED_SHARED > > mm_flags_test() check, note that memory mapped files keep their own > > separate references to the files. The case to work around is that early > > unlocking a flock on a file allows other processes to lock and modify > > the mapped data protected by the flock, > > as suggested by Pedro Falcato. > > - Link to v3: https://lore.kernel.org/all/[email protected]/ > > > > Change in v2: > > - Get rid of the implement of adding new fcntl API, the issue does not > > worth inflicting the cost on everyone, > > as suggested by Al Viro. > > - Call exit_files() in coredump_wait(), > > as suggested by Eric W. Biederman. > > Add MMF_DUMP_MAPPED_SHARED mm_flags_test() check to filter cases that > > need to dump file-backed shared memory. > > - Link to v2: https://lore.kernel.org/lkml/[email protected]/ > > > > v1: > > - Link to v1: https://lore.kernel.org/all/[email protected]/ > > --- > > Documentation/filesystems/proc.rst | 17 ++++++++++++----- > > fs/coredump.c | 21 +++++++++++++++++++++ > > fs/file_table.c | 7 ++++++- > > include/linux/mm_types.h | 6 ++++-- > > 4 files changed, 43 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst > > index db6167bef..aee03858b 100644 > > --- a/Documentation/filesystems/proc.rst > > +++ b/Documentation/filesystems/proc.rst > > @@ -1927,7 +1927,7 @@ will be dumped when the <pid> process is dumped. coredump_filter is a bitmask > > of memory types. If a bit of the bitmask is set, memory segments of the > > corresponding memory type are dumped, otherwise they are not dumped. > > > > -The following 9 memory types are supported: > > +The following 10 memory types are supported: > > > > - (bit 0) anonymous private memory > > - (bit 1) anonymous shared memory > > @@ -1939,6 +1939,7 @@ The following 9 memory types are supported: > > - (bit 6) hugetlb shared memory > > - (bit 7) DAX private memory > > - (bit 8) DAX shared memory > > + - (bit 9) fd list > > > > Note that MMIO pages such as frame buffer are never dumped and vDSO pages > > are always dumped regardless of the bitmask status. > > @@ -1946,13 +1947,19 @@ The following 9 memory types are supported: > > Note that bits 0-4 don't affect hugetlb or DAX memory. hugetlb memory is > > only affected by bit 5-6, and DAX is only affected by bits 7-8. > > > > -The default value of coredump_filter is 0x33; this means all anonymous memory > > -segments, ELF header pages and hugetlb private memory are dumped. > > + Note that bit 9 is set by default because tools like systemd-coredump go > > + through the fds. If you do not set bit 9, files that are not referenced by > > + any VMA are released before dumping core. Some file release logic, such as > > + exiting flock or releasing references to shared buffers is executed much > > + earlier. > > + > > +The default value of coredump_filter is 0x233; this means all anonymous memory > > +segments, ELF header pages, hugetlb private memory and fd list are dumped. > > > > If you don't want to dump all shared memory segments attached to pid 1234, > > -write 0x31 to the process's proc file:: > > +write 0x231 to the process's proc file:: > > > > - $ echo 0x31 > /proc/1234/coredump_filter > > + $ echo 0x231 > /proc/1234/coredump_filter > > > > When a new process is created, the process inherits the bitmask status from its > > parent. It is useful to set up coredump_filter before the program runs. > > diff --git a/fs/coredump.c b/fs/coredump.c > > index bb6fdb1f4..ed4d30916 100644 > > --- a/fs/coredump.c > > +++ b/fs/coredump.c > > @@ -521,6 +521,25 @@ static int zap_threads(struct task_struct *tsk, > > return nr; > > } > > > > +/* > > + * If do not dump fd list, files that are not referenced by any VMA > > + * can be released before dumping core. Therefore, some file release > > + * logic, such as exiting flock or releasing references to shared > > + * buffers is executed much earlier. Note that do_coredump() often > > + * takes several seconds or even longer to execute. > > + */ > > +static void coredump_pre_exit(void) > > +{ > > + struct task_struct *tsk = current, *t; > > + > > + if (mm_flags_test(MMF_DUMP_FD_LIST, tsk->mm)) > > + return; > > + > > + for_each_thread(tsk, t) { > > + exit_files(t); > > + } > > +} > > + > > static int coredump_wait(int exit_code, struct core_state *core_state) > > { > > struct task_struct *tsk = current; > > @@ -1124,6 +1143,8 @@ static void do_coredump(struct core_name *cn, struct coredump_params *cprm, > > if (cn->mask & COREDUMP_REJECT) > > return; > > > > + coredump_pre_exit(); > > + > > /* get us an unshared descriptor table; almost always a no-op */ > > /* The cell spufs coredump code reads the file descriptor tables */ > > if (unshare_files()) > > diff --git a/fs/file_table.c b/fs/file_table.c > > index 16e52e7fc..6ed8ace8c 100644 > > --- a/fs/file_table.c > > +++ b/fs/file_table.c > > @@ -566,7 +566,12 @@ static void __fput_deferred(struct file *file) > > return; > > } > > > > - if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) { > > + /* > > + * coredump_pre_exit() may release files before dumping core. > > + * Cannot use task_work in the case, needs to release files > > + * earlier. > > + */ > > + if (likely(!in_interrupt() && !(task->flags & (PF_KTHREAD | PF_DUMPCORE)))) { > > Still NAK to this whole appraoch. Would you be ok with a variant which merely unlocks prior to dumping? File descriptors remain in place like they always did, but that bit about them is changed.