On Mon 08-08-11 16:45:26, Dave Chinner wrote: > From: Dave Chinner <dchinner@xxxxxxxxxx> > > XFS holds locks that should be nested inside the inode->i_mutex when > generic_file_splice_write is called. This function takes the > i_mutex, and so we get a lock inversion that triggers lockdep > warnings and has been found to cause real deadlocks. > > XFS does not need the splice code to take the i_mutex to do the page > cache manipulation, so modify the generic splice code to use an > actor function for the code that currently requires the i_mutex to > be taken. Convert generic_file_splice_write() to use this new > interface supplying a generic actor function that performs the same > actions as the existing code. > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> The patch looks good to me. Reviewed-by: Jan Kara <jack@xxxxxxx> Honza > --- > fs/splice.c | 58 ++++++++++++++++++++++++++++++++++++++---------- > include/linux/splice.h | 5 ++++ > 2 files changed, 51 insertions(+), 12 deletions(-) > > diff --git a/fs/splice.c b/fs/splice.c > index fa2defa..a02dddc 100644 > --- a/fs/splice.c > +++ b/fs/splice.c > @@ -967,24 +967,24 @@ ssize_t splice_from_pipe(struct pipe_inode_info *pipe, struct file *out, > } > > /** > - * generic_file_splice_write - splice data from a pipe to a file > + * splice_write_to_file - splice data from a pipe to a file > * @pipe: pipe info > * @out: file to write to > * @ppos: position in @out > * @len: number of bytes to splice > * @flags: splice modifier flags > + * @actor: worker that does the splicing from the pipe to the file > * > * Description: > * Will either move or copy pages (determined by @flags options) from > * the given pipe inode to the given file. > * > */ > -ssize_t > -generic_file_splice_write(struct pipe_inode_info *pipe, struct file *out, > - loff_t *ppos, size_t len, unsigned int flags) > +ssize_t splice_write_to_file(struct pipe_inode_info *pipe, struct file *out, > + loff_t *ppos, size_t len, unsigned int flags, > + splice_write_actor actor) > { > struct address_space *mapping = out->f_mapping; > - struct inode *inode = mapping->host; > struct splice_desc sd = { > .total_len = len, > .flags = flags, > @@ -1001,13 +1001,8 @@ generic_file_splice_write(struct pipe_inode_info *pipe, struct file *out, > if (ret <= 0) > break; > > - mutex_lock_nested(&inode->i_mutex, I_MUTEX_CHILD); > - ret = file_remove_suid(out); > - if (!ret) { > - file_update_time(out); > - ret = splice_from_pipe_feed(pipe, &sd, pipe_to_file); > - } > - mutex_unlock(&inode->i_mutex); > + ret = actor(pipe, &sd); > + > } while (ret > 0); > splice_from_pipe_end(pipe, &sd); > > @@ -1032,7 +1027,46 @@ generic_file_splice_write(struct pipe_inode_info *pipe, struct file *out, > > return ret; > } > +EXPORT_SYMBOL(splice_write_to_file); > > +static ssize_t generic_file_splice_write_actor(struct pipe_inode_info *pipe, > + struct splice_desc *sd) > +{ > + struct file *out = sd->u.file; > + struct inode *inode = out->f_mapping->host; > + ssize_t ret; > + > + mutex_lock_nested(&inode->i_mutex, I_MUTEX_CHILD); > + ret = file_remove_suid(out); > + if (!ret) { > + file_update_time(out); > + ret = splice_from_pipe_feed(pipe, sd, pipe_to_file); > + } > + mutex_unlock(&inode->i_mutex); > + > + return ret; > +} > + > +/** > + * generic_file_splice_write - splice data from a pipe to a file > + * @pipe: pipe info > + * @out: file to write to > + * @ppos: position in @out > + * @len: number of bytes to splice > + * @flags: splice modifier flags > + * > + * Description: > + * Will either move or copy pages (determined by @flags options) from > + * the given pipe inode to the given file. > + * > + */ > +ssize_t > +generic_file_splice_write(struct pipe_inode_info *pipe, struct file *out, > + loff_t *ppos, size_t len, unsigned int flags) > +{ > + return splice_write_to_file(pipe, out, ppos, len, flags, > + generic_file_splice_write_actor); > +} > EXPORT_SYMBOL(generic_file_splice_write); > > static int write_pipe_buf(struct pipe_inode_info *pipe, struct pipe_buffer *buf, > diff --git a/include/linux/splice.h b/include/linux/splice.h > index 26e5b61..bd14281 100644 > --- a/include/linux/splice.h > +++ b/include/linux/splice.h > @@ -61,6 +61,8 @@ typedef int (splice_actor)(struct pipe_inode_info *, struct pipe_buffer *, > struct splice_desc *); > typedef int (splice_direct_actor)(struct pipe_inode_info *, > struct splice_desc *); > +typedef ssize_t (splice_write_actor)(struct pipe_inode_info *, > + struct splice_desc *); > > extern ssize_t splice_from_pipe(struct pipe_inode_info *, struct file *, > loff_t *, size_t, unsigned int, > @@ -82,6 +84,9 @@ extern ssize_t splice_to_pipe(struct pipe_inode_info *, > extern ssize_t splice_direct_to_actor(struct file *, struct splice_desc *, > splice_direct_actor *); > > +extern ssize_t splice_write_to_file(struct pipe_inode_info *, struct file *, > + loff_t *, size_t, unsigned int, > + splice_write_actor *); > /* > * for dynamic pipe sizing > */ > -- > 1.7.5.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html