Messages in this thread Patch in this message | | | Date | Thu, 2 Jul 2009 10:29:14 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 3/3] exec: Allow do_coredump to wait for user space pipe readers to complete (v6) |
| |
(add Roland)
Neil, I guess we both are tired of this thread, but I still have questions ;)
On 07/01, Neil Horman wrote: > > +static void wait_for_dump_helpers(struct file *file) > +{ > + struct pipe_inode_info *pipe; > + > + pipe = file->f_path.dentry->d_inode->i_pipe; > + > + pipe_lock(pipe); > + pipe->readers++; > + pipe->writers--; > + > + while (pipe->readers > 1) { > + wake_up_interruptible_sync(&pipe->wait); > + kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN); > + pipe_wait(pipe); > + } > + > + pipe->readers--; > + pipe->writers++; > + pipe_unlock(pipe); > + > +}
OK, I think this is simple enough and should work.
This is not exactly correct wrt signals, if we get TIF_SIGPENDING this becomes a busy-wait loop.
I'd suggest to do while (->readers && !signal_pending()), this is not exactly right too because we have other problems with signals, but this is another story.
> void do_coredump(long signr, int exit_code, struct pt_regs *regs) > { > struct core_state core_state; > @@ -1862,6 +1886,8 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) > current->signal->group_exit_code |= 0x80; > > close_fail: > + if (ispipe && core_pipe_limit) > + wait_for_dump_helpers(file);
Oh. I thought I misread the first version, but now I see I got it right. And now I confused again.
So, we only wait if core_pipe_limit != 0. Why?
The previous version, v4, called wait_for_dump_helpers() unconditionally. And this looks more right to me. Once again, even without wait_for_dump() the coredumping process can't be reaped until core_pattern app reads all data from the pipe.
I won't insist. However, anybody else please take a look?
core_pipe_limit != 0 limits the number of coredump-via-pipe in flight, OK.
But, should wait_for_dump_helpers() depend on core_limit_pipe != 0? Oleg.
| |