Remove the now unused generic_file_splice_write_nolock() function.
It's conceptually broken anyway, because splice may need to wait for
pipe events so holding locks across the whole operation is wrong.
Signed-off-by: Miklos Szeredi <[email protected]>
---
fs/splice.c | 59 -----------------------------------------------------
include/linux/fs.h | 2 -
2 files changed, 61 deletions(-)
Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c 2009-04-14 18:36:32.000000000 +0200
+++ linux-2.6/fs/splice.c 2009-04-14 19:01:16.000000000 +0200
@@ -811,65 +811,6 @@ ssize_t splice_from_pipe(struct pipe_ino
}
/**
- * generic_file_splice_write_nolock - generic_file_splice_write without mutexes
- * @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. The caller is responsible
- * for acquiring i_mutex on both inodes.
- *
- */
-ssize_t
-generic_file_splice_write_nolock(struct pipe_inode_info *pipe, struct file *out,
- loff_t *ppos, size_t len, unsigned int flags)
-{
- struct address_space *mapping = out->f_mapping;
- struct inode *inode = mapping->host;
- struct splice_desc sd = {
- .total_len = len,
- .flags = flags,
- .pos = *ppos,
- .u.file = out,
- };
- ssize_t ret;
- int err;
-
- err = file_remove_suid(out);
- if (unlikely(err))
- return err;
-
- ret = __splice_from_pipe(pipe, &sd, pipe_to_file);
- if (ret > 0) {
- unsigned long nr_pages;
-
- *ppos += ret;
- nr_pages = (ret + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
-
- /*
- * If file or inode is SYNC and we actually wrote some data,
- * sync it.
- */
- if (unlikely((out->f_flags & O_SYNC) || IS_SYNC(inode))) {
- err = generic_osync_inode(inode, mapping,
- OSYNC_METADATA|OSYNC_DATA);
-
- if (err)
- ret = err;
- }
- balance_dirty_pages_ratelimited_nr(mapping, nr_pages);
- }
-
- return ret;
-}
-
-EXPORT_SYMBOL(generic_file_splice_write_nolock);
-
-/**
* generic_file_splice_write - splice data from a pipe to a file
* @pipe: pipe info
* @out: file to write to
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h 2009-04-14 16:35:15.000000000 +0200
+++ linux-2.6/include/linux/fs.h 2009-04-14 19:01:16.000000000 +0200
@@ -2150,8 +2150,6 @@ extern ssize_t generic_file_splice_read(
struct pipe_inode_info *, size_t, unsigned int);
extern ssize_t generic_file_splice_write(struct pipe_inode_info *,
struct file *, loff_t *, size_t, unsigned int);
-extern ssize_t generic_file_splice_write_nolock(struct pipe_inode_info *,
- struct file *, loff_t *, size_t, unsigned int);
extern ssize_t generic_splice_sendpage(struct pipe_inode_info *pipe,
struct file *out, loff_t *, size_t len, unsigned int flags);
extern long do_splice_direct(struct file *in, loff_t *ppos, struct file *out,
--
Miklos Szeredi wrote:
> Remove the now unused generic_file_splice_write_nolock() function.
> It's conceptually broken anyway, because splice may need to wait for
> pipe events so holding locks across the whole operation is wrong.
Did the conceptual brokenness affect userspace behaviour of
splice/sendfile at all?
Thanks,
-- Jamie
On Tue, 14 Apr 2009, Jamie Lokier wrote:
> Miklos Szeredi wrote:
> > Remove the now unused generic_file_splice_write_nolock() function.
> > It's conceptually broken anyway, because splice may need to wait for
> > pipe events so holding locks across the whole operation is wrong.
>
> Did the conceptual brokenness affect userspace behaviour of
> splice/sendfile at all?
Splice: yes, sendfile: no. Sendfile uses an internal pipe and it
always makes sure there's no blocking on that pipe.
With splice it's up to the process feeding the pipe to make sure there
are buffers for the splice to consume. If it the pipe is not fed
properly then the splice call might hang, waiting for data, keeping
any locks surrounding generic_file_splice_write_nolock() locked.
Which in turn will block other operations from proceeding. Not what
we want.
Thanks,
Miklos