Messages in this thread | | | Date | Wed, 1 Jul 2009 07:52:57 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 3/3] exec: Allow do_coredump to wait for user space pipe readers to complete (v4) |
| |
On 06/30, Neil Horman wrote: > > void do_coredump(long signr, int exit_code, struct pt_regs *regs) > { > struct core_state core_state; > char corename[CORENAME_MAX_SIZE + 1]; > struct mm_struct *mm = current->mm; > struct linux_binfmt * binfmt; > - struct inode * inode; > - struct file * file; > + struct inode * inode = NULL; > + struct file * file = NULL;
why this change?
> @@ -1824,6 +1860,17 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) > corename); > goto fail_dropcount; > } > + > + /* > + * This lets us wait on a pipe after we close the writing > + * end. The extra reader count prevents the pipe_inode_info > + * from getting freed.
but it can't be freed until we close file?
> This extra count is reclaimed in > + * wait_for_dump_helpers > + */ > + pipe = file->f_path.dentry->d_inode->i_pipe; > + pipe_lock(pipe); > + pipe->readers++; > + pipe_unlock(pipe);
why should we inc ->readers in advance?
> + wait_for_dump_helpers(file);
why do we call it unconditionally and then check ISFIFO? We only need to wait when ispipe = T, and in that case we know that this file is pipe.
IOW, could you explain why the (much simpler) patch I sent doesn't work ?
Hmm. And in fact that pipe->readers++ above doesn't look right. What if the core_patter task exits? Since we incremented ->readers we can't notice the fact there are no readers, and f_op->write() will hang forever.
Oleg.
| |