linux-kernel - Re: [ltt-dev] [PATCH] nfs: add support for splice writes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1240335302.5390.20.camel@heimdal.trondhjem.org>
Date:	Tue, 21 Apr 2009 13:35:02 -0400
From:	Trond Myklebust <trond.myklebust@....uio.no>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Suresh Jayaraman <sjayaraman@...e.de>,
	Masahiro Tamori <masahiro.tamori@...il.com>,
	Mathieu Desnoyers <compudj@...stal.dyndns.org>,
	linux-nfs@...r.kernel.org, ltt-dev@...ts.casi.polymtl.ca,
	LKML <linux-kernel@...r.kernel.org>,
	linux-embedded@...r.kernel.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [ltt-dev] [PATCH] nfs: add support for splice writes

On Tue, 2009-04-21 at 10:48 -0400, Christoph Hellwig wrote:
> On Mon, Apr 20, 2009 at 09:17:23PM +0530, Suresh Jayaraman wrote:
> > +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
> > +				     struct file *filp, loff_t *ppos,
> > +				     size_t count, unsigned int flags)
> > +{
> > +	struct dentry *dentry = filp->f_path.dentry;
> > +
> > +	dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
> > +		dentry->d_parent->d_name.name, dentry->d_name.name,
> > +		(unsigned long) count, (unsigned long long) *ppos);
> > +
> > +	return generic_file_splice_write(pipe, filp, ppos, count, flags);
> > +}
> > +
> 
> You need all calls from nfs_file_write, too:
> 
>  - most importantly the nfs_revalidate_file_size for O_APPEND

Isn't O_APPEND illegal for splice_write()? It looks like it is from a
quick perusal of do_splice_from().

>  - the nfs_do_fsync for sync writes

generic_file_splice_write() calls generic_osync_inode(), which should
ensure sync writes even with NFS.
The one thing it won't do is propagate NFS write errors back to the
caller. If we do care about this, then we should certainly test for
nfs_need_sync_write() and then call nfs_do_fsync() (see
nfs_file_write()).

>  - probably the stats increment

We should talk to Chuck about this.

Cheers
  Trond

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/