Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756918AbZFCXjq (ORCPT ); Wed, 3 Jun 2009 19:39:46 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756194AbZFCXjY (ORCPT ); Wed, 3 Jun 2009 19:39:24 -0400 Received: from e6.ny.us.ibm.com ([32.97.182.146]:53107 "EHLO e6.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756015AbZFCXjV (ORCPT ); Wed, 3 Jun 2009 19:39:21 -0400 Subject: Re: [PATCH 07/23] vfs: Teach sendfile,splice,tee,and vmsplice to use file_hotplug_lock From: Badari Pulavarty To: "Eric W. Biederman" Cc: Al Viro , linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, Hugh Dickins , Tejun Heo , Alexey Dobriyan , Linus Torvalds , Alan Cox , Greg Kroah-Hartman , Nick Piggin , Andrew Morton , Christoph Hellwig , "Eric W. Biederman" , "Eric W. Biederman" In-Reply-To: <1243893048-17031-7-git-send-email-ebiederm@xmission.com> References: <1243893048-17031-7-git-send-email-ebiederm@xmission.com> Content-Type: text/plain Date: Wed, 03 Jun 2009 16:39:23 -0700 Message-Id: <1244072363.6383.15.camel@badari-desktop> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6572 Lines: 254 On Mon, 2009-06-01 at 14:50 -0700, Eric W. Biederman wrote: > From: Eric W. Biederman > > Signed-off-by: Eric W. Biederman > --- > fs/read_write.c | 28 +++++++++---- > fs/splice.c | 111 +++++++++++++++++++++++++++++++++++++----------------- > 2 files changed, 95 insertions(+), 44 deletions(-) > > diff --git a/fs/read_write.c b/fs/read_write.c > index 718baea..c473d74 100644 > --- a/fs/read_write.c > +++ b/fs/read_write.c > @@ -861,21 +861,24 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos, > goto out; > if (!(in_file->f_mode & FMODE_READ)) > goto fput_in; > + retval = -EIO; > + if (!file_hotplug_read_trylock(in_file)) > + goto fput_in; > retval = -EINVAL; > in_inode = in_file->f_path.dentry->d_inode; > if (!in_inode) > - goto fput_in; > + goto unlock_in; > if (!in_file->f_op || !in_file->f_op->splice_read) > - goto fput_in; > + goto unlock_in; > retval = -ESPIPE; > if (!ppos) > ppos = &in_file->f_pos; > else > if (!(in_file->f_mode & FMODE_PREAD)) > - goto fput_in; > + goto unlock_in; > retval = rw_verify_area(READ, in_file, ppos, count); > if (retval < 0) > - goto fput_in; > + goto unlock_in; > count = retval; > > /* > @@ -884,16 +887,19 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos, > retval = -EBADF; > out_file = fget_light(out_fd, &fput_needed_out); > if (!out_file) > - goto fput_in; > + goto unlock_in; > if (!(out_file->f_mode & FMODE_WRITE)) > goto fput_out; > + retval = -EIO; > + if (!file_hotplug_read_trylock(out_file)) > + goto fput_out; > retval = -EINVAL; > if (!out_file->f_op || !out_file->f_op->sendpage) > - goto fput_out; > + goto unlock_out; > out_inode = out_file->f_path.dentry->d_inode; > retval = rw_verify_area(WRITE, out_file, &out_file->f_pos, count); > if (retval < 0) > - goto fput_out; > + goto unlock_out; > count = retval; > > if (!max) > @@ -902,11 +908,11 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos, > pos = *ppos; > retval = -EINVAL; > if (unlikely(pos < 0)) > - goto fput_out; > + goto unlock_out; > if (unlikely(pos + count > max)) { > retval = -EOVERFLOW; > if (pos >= max) > - goto fput_out; > + goto unlock_out; > count = max - pos; > } > > @@ -933,8 +939,12 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos, > if (*ppos > max) > retval = -EOVERFLOW; > > +unlock_out: > + file_hotplug_read_unlock(out_file); > fput_out: > fput_light(out_file, fput_needed_out); > +unlock_in: > + file_hotplug_read_unlock(in_file); > fput_in: > fput_light(in_file, fput_needed_in); > out: > diff --git a/fs/splice.c b/fs/splice.c > index 666953d..fc6b3a5 100644 > --- a/fs/splice.c > +++ b/fs/splice.c > @@ -1464,15 +1464,21 @@ SYSCALL_DEFINE4(vmsplice, int, fd, const struct iovec __user *, iov, > > error = -EBADF; > file = fget_light(fd, &fput); > - if (file) { > - if (file->f_mode & FMODE_WRITE) > - error = vmsplice_to_pipe(file, iov, nr_segs, flags); > - else if (file->f_mode & FMODE_READ) > - error = vmsplice_to_user(file, iov, nr_segs, flags); > + if (!file) > + goto out; > > - fput_light(file, fput); > - } > + if (!file_hotplug_read_trylock(file)) > + goto fput_file; > > + if (file->f_mode & FMODE_WRITE) > + error = vmsplice_to_pipe(file, iov, nr_segs, flags); > + else if (file->f_mode & FMODE_READ) > + error = vmsplice_to_user(file, iov, nr_segs, flags); > + > + file_hotplug_read_unlock(file); > +fput_file: > + fput_light(file, fput); > +out: > return error; > } > > @@ -1489,21 +1495,39 @@ SYSCALL_DEFINE6(splice, int, fd_in, loff_t __user *, off_in, > > error = -EBADF; > in = fget_light(fd_in, &fput_in); > - if (in) { > - if (in->f_mode & FMODE_READ) { > - out = fget_light(fd_out, &fput_out); > - if (out) { > - if (out->f_mode & FMODE_WRITE) > - error = do_splice(in, off_in, > - out, off_out, > - len, flags); > - fput_light(out, fput_out); > - } > - } > + if (!in) > + goto out; > > - fput_light(in, fput_in); > - } > + if (!(in->f_mode & FMODE_READ)) > + goto fput_in; > + > + error = -EIO; > + if (!file_hotplug_read_trylock(in)) > + goto fput_in; > + > + error = -EBADF; > + out = fget_light(fd_out, &fput_out); > + if (!out) > + goto unlock_in; > + > + if (!(out->f_mode & FMODE_WRITE)) > + goto fput_out; > + > + error = -EIO; > + if (!file_hotplug_read_trylock(out)) > + goto fput_out; > + > + error = do_splice(in, off_in, out, off_out, len, flags); > > + file_hotplug_read_unlock(out); > +fput_out: > + fput_light(out, fput_out); > +unlock_in: > + file_hotplug_read_unlock(in); > +fput_in: > + fput_light(in, fput_in); > + > +out: > return error; > } > > @@ -1703,27 +1727,44 @@ static long do_tee(struct file *in, struct file *out, size_t len, > > SYSCALL_DEFINE4(tee, int, fdin, int, fdout, size_t, len, unsigned int, flags) > { > - struct file *in; > - int error, fput_in; > + struct file *in, *out; > + int error, fput_in, fput_out; > > if (unlikely(!len)) > return 0; > > error = -EBADF; > in = fget_light(fdin, &fput_in); > - if (in) { > - if (in->f_mode & FMODE_READ) { > - int fput_out; > - struct file *out = fget_light(fdout, &fput_out); > - > - if (out) { > - if (out->f_mode & FMODE_WRITE) > - error = do_tee(in, out, len, flags); > - fput_light(out, fput_out); > - } > - } > - fput_light(in, fput_in); > - } > + if (!in) > + goto out; > + > + if (!(in->f_mode & FMODE_READ)) > + goto unlock_in; <<<<<<< Shouldn't this be goto fput_in; ? btw, its confusing to have labels and variables with same name: fput_in and fput_out. You may want to rename labels ? > > + error = -EIO; > + if (!file_hotplug_read_trylock(in)) > + goto fput_in; > + > + error = -EBADF; > + out = fget_light(fdout, &fput_out); > + if (!out) > + goto unlock_in; > + > + if (!(out->f_mode & FMODE_WRITE)) > + goto fput_out; > + > + if (!file_hotplug_read_trylock(out)) > + goto fput_out; > + > + error = do_tee(in, out, len, flags); > + > + file_hotplug_read_unlock(out); > +fput_out: > + fput_light(out, fput_out); > +unlock_in: > + file_hotplug_read_unlock(in); > +fput_in: > + fput_light(in, fput_in); > +out: > return error; > } Thanks, Badari -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/