From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sgi.com (relay1.corp.sgi.com [137.38.102.111]) by oss.sgi.com (Postfix) with ESMTP id A0A567F37 for ; Sat, 18 Jan 2014 14:27:30 -0600 (CST) Received: from cuda.sgi.com (cuda2.sgi.com [192.48.176.25]) by relay1.corp.sgi.com (Postfix) with ESMTP id 880E68F8066 for ; Sat, 18 Jan 2014 12:27:27 -0800 (PST) Received: from ZenIV.linux.org.uk (zeniv.linux.org.uk [195.92.253.2]) by cuda.sgi.com with ESMTP id MIt5Vt6xnLpkeoRw (version=TLSv1 cipher=AES256-SHA bits=256 verify=NO) for ; Sat, 18 Jan 2014 12:27:26 -0800 (PST) Date: Sat, 18 Jan 2014 20:27:17 +0000 From: Al Viro Subject: Re: [PATCH 0/5] splice: locking changes and code refactoring Message-ID: <20140118202717.GJ10323@ZenIV.linux.org.uk> References: <20131212181459.994196463@bombadil.infradead.org> <20140113141416.GA30117@infradead.org> <20140113235646.GR10323@ZenIV.linux.org.uk> <20140114132207.GA25170@infradead.org> <20140114172033.GU10323@ZenIV.linux.org.uk> <20140118064040.GE10323@ZenIV.linux.org.uk> <20140118074649.GF10323@ZenIV.linux.org.uk> <20140118201031.GI10323@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140118201031.GI10323@ZenIV.linux.org.uk> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: xfs-bounces@oss.sgi.com Sender: xfs-bounces@oss.sgi.com To: Linus Torvalds Cc: Jens Axboe , Steve French , Sage Weil , Mark Fasheh , xfs@oss.sgi.com, Christoph Hellwig , Joel Becker , linux-fsdevel On Sat, Jan 18, 2014 at 08:10:31PM +0000, Al Viro wrote: > On Sat, Jan 18, 2014 at 11:59:56AM -0800, Linus Torvalds wrote: > > On Fri, Jan 17, 2014 at 11:46 PM, Al Viro wrote: > > > On Fri, Jan 17, 2014 at 11:22:04PM -0800, Linus Torvalds wrote: > > >> On Fri, Jan 17, 2014 at 10:40 PM, Al Viro wrote: > > >> > > > >> > Objections, comments? > > >> > > >> I certainly object to the "map, then unmap" approach. No VM games. > > > > > > Um... > > > > > > If we are going to copy that data (and all users of generic_file_splice_write() > > > do that memcpy() to page cache), we have to kmap the source ;-/ > > > > Yeah, the kmap/kunmap we have to do. But that's a no-op on 64-bit, and > > has to be done one page at a time (well, I guess you could do a > > couple). > > > > But you can't do that *around* the default_file_splice_write(), so I > > thought you meant some kind of "map into user space". And I absolutely > > *detest* that kind of approach. > > Ouch... No, I hadn't meant that kind of insanity, but I'd missed the > problem with scarcity of mappings completely... Ouch^2: default_file_write_splice_write() keeps calling write_pipe_buf(), which does this: data = buf->ops->map(pipe, buf, 0); ret = __kernel_write(sd->u.file, data + buf->offset, sd->len, &tmp); buf->ops->unmap(pipe, buf, data); IOW, ->write() (with whatever locks there might be) wrapped into kmap_atomic()/kunmap_atomic(). And anybody can do that - just a splice to file on procfs will hit that codepath... Or on 9p, for that matter, or fat, or afs, or cifs, etc. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: [PATCH 0/5] splice: locking changes and code refactoring Date: Sat, 18 Jan 2014 20:27:17 +0000 Message-ID: <20140118202717.GJ10323@ZenIV.linux.org.uk> References: <20131212181459.994196463@bombadil.infradead.org> <20140113141416.GA30117@infradead.org> <20140113235646.GR10323@ZenIV.linux.org.uk> <20140114132207.GA25170@infradead.org> <20140114172033.GU10323@ZenIV.linux.org.uk> <20140118064040.GE10323@ZenIV.linux.org.uk> <20140118074649.GF10323@ZenIV.linux.org.uk> <20140118201031.GI10323@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Christoph Hellwig , Jens Axboe , Mark Fasheh , Joel Becker , linux-fsdevel , xfs@oss.sgi.com, Sage Weil , Steve French To: Linus Torvalds Return-path: Received: from zeniv.linux.org.uk ([195.92.253.2]:52875 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751381AbaARU1Z (ORCPT ); Sat, 18 Jan 2014 15:27:25 -0500 Content-Disposition: inline In-Reply-To: <20140118201031.GI10323@ZenIV.linux.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Sat, Jan 18, 2014 at 08:10:31PM +0000, Al Viro wrote: > On Sat, Jan 18, 2014 at 11:59:56AM -0800, Linus Torvalds wrote: > > On Fri, Jan 17, 2014 at 11:46 PM, Al Viro wrote: > > > On Fri, Jan 17, 2014 at 11:22:04PM -0800, Linus Torvalds wrote: > > >> On Fri, Jan 17, 2014 at 10:40 PM, Al Viro wrote: > > >> > > > >> > Objections, comments? > > >> > > >> I certainly object to the "map, then unmap" approach. No VM games. > > > > > > Um... > > > > > > If we are going to copy that data (and all users of generic_file_splice_write() > > > do that memcpy() to page cache), we have to kmap the source ;-/ > > > > Yeah, the kmap/kunmap we have to do. But that's a no-op on 64-bit, and > > has to be done one page at a time (well, I guess you could do a > > couple). > > > > But you can't do that *around* the default_file_splice_write(), so I > > thought you meant some kind of "map into user space". And I absolutely > > *detest* that kind of approach. > > Ouch... No, I hadn't meant that kind of insanity, but I'd missed the > problem with scarcity of mappings completely... Ouch^2: default_file_write_splice_write() keeps calling write_pipe_buf(), which does this: data = buf->ops->map(pipe, buf, 0); ret = __kernel_write(sd->u.file, data + buf->offset, sd->len, &tmp); buf->ops->unmap(pipe, buf, data); IOW, ->write() (with whatever locks there might be) wrapped into kmap_atomic()/kunmap_atomic(). And anybody can do that - just a splice to file on procfs will hit that codepath... Or on 9p, for that matter, or fat, or afs, or cifs, etc.