Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1354530 > unrolled thread
| Started by | Gregory Farnum <greg@gregs42.com> |
|---|---|
| First post | 2016-03-09 23:30 +0100 |
| Last post | 2016-03-12 01:40 +0100 |
| Articles | 16 on this page of 56 — 16 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Gregory Farnum <greg@gregs42.com> - 2016-03-09 23:30 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-10 00:10 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Ric Wheeler <ricwheeler@gmail.com> - 2016-03-10 16:00 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-10 19:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-10 22:50 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Ric Wheeler <rwheeler@redhat.com> - 2016-03-11 05:50 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-03-11 15:10 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-11 16:30 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-11 18:30 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Andy Lutomirski <luto@amacapital.net> - 2016-03-11 18:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-11 19:30 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Dave Chinner <david@fromorbit.com> - 2016-03-11 23:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-12 01:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-12 01:50 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-12 08:30 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Thomas Schoebel-Theuer <tst@schoebel-theuer.de> - 2016-03-12 11:20 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Dave Chinner <david@fromorbit.com> - 2016-03-14 00:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Ric Wheeler <rwheeler@redhat.com> - 2016-03-14 11:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-14 15:50 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Dave Chinner <david@fromorbit.com> - 2016-03-15 21:20 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-15 21:50 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-15 22:30 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Dave Chinner <david@fromorbit.com> - 2016-03-15 23:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-16 00:00 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-03-16 03:00 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Andreas Dilger <adilger@dilger.ca> - 2016-03-16 22:50 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-17 01:20 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Eric Sandeen <esandeen@redhat.com> - 2016-03-17 01:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-17 02:00 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Gregory Farnum <greg@gregs42.com> - 2016-03-17 06:20 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-17 13:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-17 18:50 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-17 19:00 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Ric Wheeler <rwheeler@redhat.com> - 2016-03-17 19:00 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Chris Mason <clm@fb.com> - 2016-03-17 19:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Andreas Dilger <adilger@dilger.ca> - 2016-03-17 21:50 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Chris Mason <clm@fb.com> - 2016-03-17 22:10 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-18 04:30 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Jeff Moyer <jmoyer@redhat.com> - 2016-03-18 16:20 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks "Martin K. Petersen" <martin.petersen@oracle.com> - 2016-03-18 21:10 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Gregory Farnum <greg@gregs42.com> - 2016-03-18 08:00 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-18 08:30 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Dave Chinner <david@fromorbit.com> - 2016-03-17 02:10 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-03-17 03:50 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks NeilBrown <neilb@suse.com> - 2016-03-19 00:00 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-16 00:10 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-16 00:20 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Dave Chinner <david@fromorbit.com> - 2016-03-16 01:10 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Dave Chinner <david@fromorbit.com> - 2016-03-16 01:00 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-16 01:10 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Eric Sandeen <esandeen@redhat.com> - 2016-03-16 01:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Chris Mason <clm@fb.com> - 2016-03-16 02:00 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Chris Mason <clm@fb.com> - 2016-03-16 23:30 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Ric Wheeler <rwheeler@redhat.com> - 2016-03-17 14:50 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Eric Sandeen <esandeen@redhat.com> - 2016-03-15 23:40 +0100
Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-12 01:40 +0100
Page 3 of 3 — ← Prev page 1 2 [3]
| From | Gregory Farnum <greg@gregs42.com> |
|---|---|
| Date | 2016-03-18 08:00 +0100 |
| Message-ID | <rdWy6-20M-5@gated-at.bofh.it> |
| In reply to | #1360083 |
On Thu, Mar 17, 2016 at 10:47 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Wed, Mar 16, 2016 at 10:18 PM, Gregory Farnum <greg@gregs42.com> wrote: >> >> So we've not asked for NO_HIDE_STALE on the mailing lists, but I think >> it was one of the problems Sage had using xfs in his BlueStore >> implementation and was a big part of why it moved to pure userspace. >> FileStore might use NO_HIDE_STALE in some places but it would be >> pretty limited. When it came up at Linux FAST we were discussing how >> it and similar things had been problems for us in the past and it >> would've been nice if they were upstream. > > Hmm. > > So to me it really sounds like somebody should cook up a patch, but we > shouldn't put it in the upstream kernel until we get numbers and > actual "yes, we'd use this" from outside of google. > > I say "outside of google", because inside of google not only do we not > get numbers, but google can maintain their own patch. > > But maybe Ted could at least post the patch google uses, and somebody > in the Ceph community might want to at least try it out... > >> What *is* a big deal for >> FileStore (and would be easy to take advantage of) is the thematically >> similar O_NOMTIME flag, which is also about reducing metadata updates >> and got blocked on similar stupid-user grounds (although not security >> ones): http://thread.gmane.org/gmane.linux.kernel.api/10727. > > Hmm. I don't hate that patch, because the NOATIME thing really does > wonders on many loads. NOMTIME makes sense. > > It's not like you can't do this with utimes() anyway. > > That said, I do wonder if people wouldn't just prefer to expand on and > improve on the lazytime. > > Is there some reason you guys didn't use that? I wasn't really involved in this stuff but I gather from looking at http://www.spinics.net/lists/xfs/msg36869.html that any durability command other than fdatasync is going to write out the mtime updates to the inodes on disk. Given our durability requirements and the guarantees offered about when things actually hit disk, that doesn't work for us. We run an fsync on the order of every 30 seconds, and we do various combinations of fsync, fdatasync, flush_file_range, (and, well, any command we're provided) to try and bound the amount of dirty data and prevent fs/disk throughput pauses when we do that full sync. Anybody trying to do anything similar would want a switch that prevents operations from updating the mtime on disk no matter what. -Greg
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2016-03-18 08:30 +0100 |
| Message-ID | <rdX18-2qq-9@gated-at.bofh.it> |
| In reply to | #1360381 |
On Thu, Mar 17, 2016 at 11:52 PM, Gregory Farnum <greg@gregs42.com> wrote:
>
> I wasn't really involved in this stuff but I gather from looking at
> http://www.spinics.net/lists/xfs/msg36869.html that any durability
> command other than fdatasync is going to write out the mtime updates
> to the inodes on disk. Given our durability requirements and the
> guarantees offered about when things actually hit disk, that doesn't
> work for us.
Fair enough. Yes, the lazytime thing doesn't help if you actually sync
the file explicitly, then you'd really do need something like NOMTIME
in order to not dirty the inode itself at all (together with
preallocation - otherwise the inode will be dirty due to the block
allocation updates).
Linus
[toc] | [prev] | [next] | [standalone]
| From | Dave Chinner <david@fromorbit.com> |
|---|---|
| Date | 2016-03-17 02:10 +0100 |
| Subject | Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks |
| Message-ID | <rduBQ-g6-5@gated-at.bofh.it> |
| In reply to | #1358469 |
On Tue, Mar 15, 2016 at 06:51:39PM -0700, Darrick J. Wong wrote: > On Tue, Mar 15, 2016 at 06:52:24PM -0400, Theodore Ts'o wrote: > > On Wed, Mar 16, 2016 at 09:33:13AM +1100, Dave Chinner wrote: > > > > > > Stale data escaping containment is a security issue. Enabling > > > generic kernel mechanisms to *enable containment escape* is > > > fundamentally wrong, and relying on userspace to Do The Right Thing > > > is even more of a gamble, IMO. > > > > We already have generic kernel mechanisms such as "the block device". P > > > > > It's a practical concern because if we enable this functionality in > > > fallocate because it will get used by more than just special storage > > > apps. i.e. this can't be waved away with "only properly managed > > > applications will use it" arguments. > > > > It requires a mount option. How is this going to allow random > > applications to use this feature, again? > > > > > I also don't make a habit of publicising the fact that since we > > > disabled the "-d unwritten=X" mkfs parameter (because of speed racer > > > blogs such as the above and configuration cargo-culting resulting in > > > unsuspecting users exposing stale data unintentionally) that the > > > functionality still exists in the kernel code and that it only takes > > > a single xfs_db command to turn off unwritten extents in XFS. i.e. > > > we can easily make fallocate on XFS expose stale data, filesystem > > > wide, without requiring mount options, kernel or application > > > modifications. > > > > So you have something even more dangerous in XFS and it's in the > > kernel tree? Has Red Hat threatened to have a distro-specific patch > > xfs_db is the XFS debugger, so you can only enable that bit of functionality > with magical commands, which IMHO isn't much different than people messing > with their ext4 filesystems with debugfs. You had better know what you're > doing and if you break the filesystem you can eat both pieces. :P > > > to comment out this code to make sure irresponsible users can't use > > it? What I've been suggesting has even more controls that what you > > have. And I've been keeping it as an out-of-tree kernel patch mainly > > because you've been arguing that it's such a horrible thing. > > One could lock it down even more -- hide it behind a Kconfig option that > depends on CONFIG_EXPERT=y and itself defaults to n, require a mount option, > only allow the file owner to call no-hide-stale and only if the file is 0600 > (or the appropriate group equivalents like Ted's existing patch), and upon > adding stale extents, set an inode flag that locks uid/gid/mode/flags. > Obviously root can still get to the file, but at least there's hard evidence > that one is entering the twilight zone. > > > > Making Google's hack more widely available through the fallocate > > > API is entirely dependent on proving that: > > > > Ceph is about to completely bypass the file system because of your > > intransigence, and reimplement a userspace file system. They seem to > > believe it's necessary. I'll let them make the case, because they > > seem to think it's necessary. And if not, if Linus sides with you, > > and doesn't want to take the patch, I'll just keep it as a > > Google-specific out-of-tree patch. I don't *need* to have this thing > > upstream. > > Frankly, I second Eric Sandeen's comments -- just how bad is ext4's > unwritten extent conversion for these folks? > > I ran this crappy looping microbenchmark against a ramdisk: > fallocate 400M > write 400M > fsync > rewrite the 400M > fsync > on kernel 4.5. > > For writing 400M through the page cache in 4k chunks, > ext4: ~460MB/s -> ~580MB/s (~20%) > XFS: ~660 -> ~870 (~25%) > btrfs: ~130 -> ~200 (~35%) > > For writing 400M in 80M chunks, > ext4: ~480MB/s -> ~720MB/s (~30%) > XFS: ~1GB/s -> ~1.5GB/s (~35%) > btrfs: ~590MB/s -> ~590MB/s (no change) > > For directio writing 400MB in 4k chunks, > ext4: 25MB/s -> 26MB/s (~5%) > XFS: 25 -> 27 (~8%) > btrfs: 22 -> 18 (...) > > For directio writing 1200MB in 80M chunks, > ext4: ~2.9GB/s -> ~3.3GB/s (~13%) > XFS: 3.2 -> 3.5 (~9%) > btrfs: 2.3 -> 2.2 (...) That's not comparing apples to apples. overwrite does not require allocation/extent manipulation at all so it is comparing performance of completely different file extent operations. The operations we should be comparing are "first write" operations, namely "write over hole with allocation" vs "write over preallocation". Overwrite performance should be the same regardless of the method used for the intial write/allocation. Cheers, Dave. -- Dave Chinner david@fromorbit.com
[toc] | [prev] | [next] | [standalone]
| From | "Darrick J. Wong" <darrick.wong@oracle.com> |
|---|---|
| Date | 2016-03-17 03:50 +0100 |
| Subject | Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks |
| Message-ID | <rdwaB-15J-5@gated-at.bofh.it> |
| In reply to | #1359504 |
On Thu, Mar 17, 2016 at 12:01:16PM +1100, Dave Chinner wrote: > On Tue, Mar 15, 2016 at 06:51:39PM -0700, Darrick J. Wong wrote: > > On Tue, Mar 15, 2016 at 06:52:24PM -0400, Theodore Ts'o wrote: > > > On Wed, Mar 16, 2016 at 09:33:13AM +1100, Dave Chinner wrote: > > > > > > > > Stale data escaping containment is a security issue. Enabling > > > > generic kernel mechanisms to *enable containment escape* is > > > > fundamentally wrong, and relying on userspace to Do The Right Thing > > > > is even more of a gamble, IMO. > > > > > > We already have generic kernel mechanisms such as "the block device". P > > > > > > > It's a practical concern because if we enable this functionality in > > > > fallocate because it will get used by more than just special storage > > > > apps. i.e. this can't be waved away with "only properly managed > > > > applications will use it" arguments. > > > > > > It requires a mount option. How is this going to allow random > > > applications to use this feature, again? > > > > > > > I also don't make a habit of publicising the fact that since we > > > > disabled the "-d unwritten=X" mkfs parameter (because of speed racer > > > > blogs such as the above and configuration cargo-culting resulting in > > > > unsuspecting users exposing stale data unintentionally) that the > > > > functionality still exists in the kernel code and that it only takes > > > > a single xfs_db command to turn off unwritten extents in XFS. i.e. > > > > we can easily make fallocate on XFS expose stale data, filesystem > > > > wide, without requiring mount options, kernel or application > > > > modifications. > > > > > > So you have something even more dangerous in XFS and it's in the > > > kernel tree? Has Red Hat threatened to have a distro-specific patch > > > > xfs_db is the XFS debugger, so you can only enable that bit of functionality > > with magical commands, which IMHO isn't much different than people messing > > with their ext4 filesystems with debugfs. You had better know what you're > > doing and if you break the filesystem you can eat both pieces. :P > > > > > to comment out this code to make sure irresponsible users can't use > > > it? What I've been suggesting has even more controls that what you > > > have. And I've been keeping it as an out-of-tree kernel patch mainly > > > because you've been arguing that it's such a horrible thing. > > > > One could lock it down even more -- hide it behind a Kconfig option that > > depends on CONFIG_EXPERT=y and itself defaults to n, require a mount option, > > only allow the file owner to call no-hide-stale and only if the file is 0600 > > (or the appropriate group equivalents like Ted's existing patch), and upon > > adding stale extents, set an inode flag that locks uid/gid/mode/flags. > > Obviously root can still get to the file, but at least there's hard evidence > > that one is entering the twilight zone. > > > > > > Making Google's hack more widely available through the fallocate > > > > API is entirely dependent on proving that: > > > > > > Ceph is about to completely bypass the file system because of your > > > intransigence, and reimplement a userspace file system. They seem to > > > believe it's necessary. I'll let them make the case, because they > > > seem to think it's necessary. And if not, if Linus sides with you, > > > and doesn't want to take the patch, I'll just keep it as a > > > Google-specific out-of-tree patch. I don't *need* to have this thing > > > upstream. > > > > Frankly, I second Eric Sandeen's comments -- just how bad is ext4's > > unwritten extent conversion for these folks? > > > > I ran this crappy looping microbenchmark against a ramdisk: > > fallocate 400M > > write 400M > > fsync > > rewrite the 400M > > fsync > > on kernel 4.5. > > > > For writing 400M through the page cache in 4k chunks, > > ext4: ~460MB/s -> ~580MB/s (~20%) > > XFS: ~660 -> ~870 (~25%) > > btrfs: ~130 -> ~200 (~35%) > > > > For writing 400M in 80M chunks, > > ext4: ~480MB/s -> ~720MB/s (~30%) > > XFS: ~1GB/s -> ~1.5GB/s (~35%) > > btrfs: ~590MB/s -> ~590MB/s (no change) > > > > For directio writing 400MB in 4k chunks, > > ext4: 25MB/s -> 26MB/s (~5%) > > XFS: 25 -> 27 (~8%) > > btrfs: 22 -> 18 (...) > > > > For directio writing 1200MB in 80M chunks, > > ext4: ~2.9GB/s -> ~3.3GB/s (~13%) > > XFS: 3.2 -> 3.5 (~9%) > > btrfs: 2.3 -> 2.2 (...) > > That's not comparing apples to apples. overwrite does not require > allocation/extent manipulation at all so it is comparing performance > of completely different file extent operations. The operations we > should be comparing are "first write" operations, namely "write over > hole with allocation" vs "write over preallocation". Overwrite > performance should be the same regardless of the method used for the > intial write/allocation. Eh, ok, let's compare writing an fallocated region vs writing an empty file: (laptop this time, so the numbers aren't the same) For writing 400M in 4k chunks, ext4: ~720MB/s -> 620MB/s (~14%) XFS: ~560MB/s -> 540MB/s (~4%) btrfs: ~260 -> 580MB/s For writing 400M in 80M chunks, ext4: ~960MB/s -> ~730MB/s (~24%) XFS: ~1000MB/s -> ~980MB/s (~2%) btrfs: ~950MB/s -> ~930MB/s (~3%) For directio writing 1200MB in 80M chunks, ext4: ~2.9GB/s -> ~2.8GB/s (~4%) XFS: 3.2 -> 3.2 btrfs: 2.3 -> 2.3 --D
[toc] | [prev] | [next] | [standalone]
| From | NeilBrown <neilb@suse.com> |
|---|---|
| Date | 2016-03-19 00:00 +0100 |
| Message-ID | <rebx8-2Fl-15@gated-at.bofh.it> |
| In reply to | #1358318 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Mar 16 2016, Theodore Ts'o wrote: > On Wed, Mar 16, 2016 at 09:33:13AM +1100, Dave Chinner wrote: >> >> Stale data escaping containment is a security issue. Enabling >> generic kernel mechanisms to *enable containment escape* is >> fundamentally wrong, and relying on userspace to Do The Right Thing >> is even more of a gamble, IMO. > > We already have generic kernel mechanisms such as "the block device". P This is a bit of an 'off-the-wall' suggestion, but I agree that these things that might be of value to user-space file servers do seem a lot like block devices. So why not make them look exactly like block devices? i.e. a new open flag O_BLOCKDEV which, when combined with O_CREAT creates a thing that is managed in the filesystem much like a file, but that appears to user-space like a block device. The major/minor numbers would be essentially meaningless - the filesystem wouldn't call init_special_inode() like it does on normal block devices, it would retain control itself. That would make the content invisible to backups and rsync and all the things that Dave has raised as potential concerns. And it would be no surprise if the contents included stale data because that is exactly what you get when you create a new logical volume with LVM2. The block device would initially be of size zero, but could be resized using fallocate (which soon will work on block devices), which can request zeros, leave holes, or with Teds new FALLOC flag (that would only be permitted on block devices) could allocate uninitialized space. Rules for using O_BLOCKDEV would still need to be clarified - mount option, access to underlying block device, CAP_MKNOD .. whatever. I think that being able to use a filesystem as a logical volume manager is an extremely interesting idea.... we might even end up with a filesystem interface on device-mapper :-) NeilBrown
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2016-03-16 00:10 +0100 |
| Message-ID | <rd6ga-so-13@gated-at.bofh.it> |
| In reply to | #1358303 |
On Tue, Mar 15, 2016 at 3:33 PM, Dave Chinner <david@fromorbit.com> wrote:
>
>> There's no "group based containment wall" that is some kind of
>> absolute protection border.
>
> Precisely my point - it's being pitched as a generic containment
> mechanism, but it really isn't.
No it hasn't.
It has been pitched as
"Companies are already doing this, and other groups are interested in it too".
What part of that is hard to understand?
At no point was it "generic containment" except in your mind.
Everybody else was very clear about the fact that stale data would be
visible. The only issue was to try to mitigate subtle mistakes.
And no, "Root reads a file that has possibly stale data in it" is not
a "subtle mistake". That's a rather obvious thing.
> Making Google's hack more widely available through the fallocate
> API is entirely dependent on proving that:
>
> a) the performance problem still exists;
> b) the performance problem exists across multiple
> filesytsems and is not isolated to just ext4 or one specific
> set of workloads;
> c) the performance problem cannot be fixed;
> d) ext4 can't implement a simple feature check to turn off
> unwritten extents similar to XFS; and
> e) if all else fails, that the API hack does not compromise the
> security of general users unaware that applications might
> be using this functionality.
>
> a), b), c) and d) have not been demonstrated, discussed or iterated -
The thing is, the onus of those shouldn't be on the people who already
have a solution that they are happy with.
The onus of that work should be on the people who are arguing against
it, and have alternate models they want to push.
So you can't ask people who already solved their issue to then try to
argue against their solution.
I do agree that it would be damn useful to have
(a) numbers. No question about that.
(b) we should have a better idea of exactly what the user needs are.
Because if we do expose this kind of functionality, it should be as
limited as possible - but that "as possible" very much depends on what
the usage models are.
And yes, "keep the patch entirely inside google" is obviously one good
way to limit the interface. But if there are really other groups that
want to explore this, then that sounds like a pretty horrible model
too.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2016-03-16 00:20 +0100 |
| Message-ID | <rd6pQ-vW-17@gated-at.bofh.it> |
| In reply to | #1358324 |
On Tue, Mar 15, 2016 at 4:06 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> And yes, "keep the patch entirely inside google" is obviously one good
> way to limit the interface. But if there are really other groups that
> want to explore this, then that sounds like a pretty horrible model
> too.
Side note: I really don't see how your argument of "XFS has been able
to do something like this for over a decade, using an even uglier
trick that is hidden and not documented" is at all an argument for
your position.
You're saying "nobody else should be doing what I've been doing for a
long time", and backing that argument up with "but I don't document
it, and it's completely different because it's done at mkfs/debugfs
time rather than mount-time".
But now that people are talking about a filesystem-independent way of
doing the same thing, now it's suddenly poisonous.
Dave, I call BS on your arguments. Or maybe I misunderstood it. But it
does smell very "do what I say, not what I do".
Linus
[toc] | [prev] | [next] | [standalone]
| From | Dave Chinner <david@fromorbit.com> |
|---|---|
| Date | 2016-03-16 01:10 +0100 |
| Subject | Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks |
| Message-ID | <rd7cd-15y-1@gated-at.bofh.it> |
| In reply to | #1358331 |
On Tue, Mar 15, 2016 at 04:14:32PM -0700, Linus Torvalds wrote: > On Tue, Mar 15, 2016 at 4:06 PM, Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > And yes, "keep the patch entirely inside google" is obviously one good > > way to limit the interface. But if there are really other groups that > > want to explore this, then that sounds like a pretty horrible model > > too. > > Side note: I really don't see how your argument of "XFS has been able > to do something like this for over a decade, using an even uglier > trick that is hidden and not documented" is at all an argument for > your position. > > You're saying "nobody else should be doing what I've been doing for a > long time", and backing that argument up with "but I don't document > it, and it's completely different because it's done at mkfs/debugfs > time rather than mount-time". You can read it that way, but that is not the message I was trying to get across. The message I was trying to get across is that there are people out there that have been using hacks like what google uses for as long as there have been filesystems around that support unwritten extents. And that they do this even though the benefits of such hacks are marginal and frequently can't be backed up with numbers. We don't support filesystems with unwritten extents disabled. I don't like the fact that there are people who turn them off on XFS filesystems, but we are stuck with it as it is part of the supported on disk format that. The best we can do is to try to prevent unsuspecting users from shooting themselves in the foot with such features, which is what we did by removing the mkfs option back in 2007. Like I said, most people don't know or understand the history.... > But now that people are talking about a filesystem-independent way of > doing the same thing, now it's suddenly poisonous. It's always been poisonous. > Dave, I call BS on your arguments. Or maybe I misunderstood it. But it > does smell very "do what I say, not what I do". We're stuck with a lot of historical functionality in XFS that we can't easily remove or disable. Learn from history, don't repeat it. Cheers, Dave. -- Dave Chinner david@fromorbit.com
[toc] | [prev] | [next] | [standalone]
| From | Dave Chinner <david@fromorbit.com> |
|---|---|
| Date | 2016-03-16 01:00 +0100 |
| Subject | Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks |
| Message-ID | <rd72x-M3-11@gated-at.bofh.it> |
| In reply to | #1358324 |
On Tue, Mar 15, 2016 at 04:06:10PM -0700, Linus Torvalds wrote: > On Tue, Mar 15, 2016 at 3:33 PM, Dave Chinner <david@fromorbit.com> wrote: > > > >> There's no "group based containment wall" that is some kind of > >> absolute protection border. > > > > Precisely my point - it's being pitched as a generic containment > > mechanism, but it really isn't. > > No it hasn't. > > It has been pitched as > > "Companies are already doing this, and other groups are interested in it too". > > What part of that is hard to understand? > > At no point was it "generic containment" except in your mind. > Everybody else was very clear about the fact that stale data would be > visible. The only issue was to try to mitigate subtle mistakes. > > And no, "Root reads a file that has possibly stale data in it" is not > a "subtle mistake". That's a rather obvious thing. > > > Making Google's hack more widely available through the fallocate > > API is entirely dependent on proving that: > > > > a) the performance problem still exists; > > b) the performance problem exists across multiple > > filesytsems and is not isolated to just ext4 or one specific > > set of workloads; > > c) the performance problem cannot be fixed; > > d) ext4 can't implement a simple feature check to turn off > > unwritten extents similar to XFS; and > > e) if all else fails, that the API hack does not compromise the > > security of general users unaware that applications might > > be using this functionality. > > > > a), b), c) and d) have not been demonstrated, discussed or iterated - > > The thing is, the onus of those shouldn't be on the people who already > have a solution that they are happy with. > > The onus of that work should be on the people who are arguing against > it, and have alternate models they want to push. I hate having to quote the guidelines we are supposed to work by, but this seems pretty clear cut. Section 2 of Documentation/SubmittingPatches: "Describe your problem. Whether your patch is a one-line bug fix or 5000 lines of a new feature, there must be an underlying problem that motivated you to do this work. Convince the reviewer that there is a problem worth fixing and that it makes sense for them to read past the first paragraph. Describe user-visible impact. Straight up crashes and lockups are pretty convincing, but not all bugs are that blatant. Even if the problem was spotted during code review, describe the impact you think it can have on users. Keep in mind that the majority of Linux installations run kernels from secondary stable trees or vendor/product-specific trees that cherry-pick only specific patches from upstream, so include anything that could help route your change downstream: provoking circumstances, excerpts from dmesg, crash descriptions, performance regressions, latency spikes, lockups, etc. Quantify optimizations and trade-offs. If you claim improvements in performance, memory consumption, stack footprint, or binary size, include numbers that back them up. But also describe non-obvious costs. Optimizations usually aren't free but trade-offs between CPU, memory, and readability; or, when it comes to heuristics, between different workloads. Describe the expected downsides of your optimization so that the reviewer can weigh costs against benefits. Once the problem is established, describe what you are actually doing about it in technical detail. It's important to describe the change in plain English for the reviewer to verify that the code is behaving as you intend it to." It is pretty clear that the onus is on the patch submitter to provide justification for inclusion, not for the reviewer/Maintainer to have to prove that the solution is unworkable. "Google uses this" is not sufficient justification. > So you can't ask people who already solved their issue to then try to > argue against their solution. I'm not asking Ted to argue against his solution. I'm the person who is trying to poke holes in the "solution" being presented... > I do agree that it would be damn useful to have > > (a) numbers. No question about that. > > (b) we should have a better idea of exactly what the user needs are. > Because if we do expose this kind of functionality, it should be as > limited as possible - but that "as possible" very much depends on what > the usage models are. It's not "damn useful" - it's an absolute requirement that numbers are provided to back up the assertion that there is no other way to solve this problem. Once it is established that there is no other way to solve the performance problem, then we can talk whether we want to trade off data safety and complexity for performance.... Cheers, Dave. -- Dave Chinner david@fromorbit.com
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2016-03-16 01:10 +0100 |
| Message-ID | <rd7ce-15y-9@gated-at.bofh.it> |
| In reply to | #1358416 |
On Tue, Mar 15, 2016 at 4:52 PM, Dave Chinner <david@fromorbit.com> wrote:
>
> It is pretty clear that the onus is on the patch submitter to
> provide justification for inclusion, not for the reviewer/Maintainer
> to have to prove that the solution is unworkable.
I agree, but quite frankly, performance is a good justification.
So if Ted can give performance numbers, that's justification enough.
We've certainly taken changes with less.
And with your "we should _not_ do this" argument, the onus is clearly on you.
> "Google uses this" is not sufficient justification.
Not per se, no, but it's a very traditional and time-honored model for
"should we merge this".
It's traditionally been things like "Redhat merged it in their distro
kernel, because they have customers that want/need it". That is *the*
reason many big projects got merged, ranging from filesystems to
drivers etc. Take reiserfs, for example: it got merged because SuSE
was actively using it.
So "this feature is being used in real life" is a big hint that the
standard upstream kernel may be missing something important. People
arguing against things like that has been a big problem in the past.
It took people _years_ to get over the whole Android thing. We need to
merge stuff that people are using and depend on, because _not_ merging
them just causes more and more distance between peoples kernels, and
makes it even harder to merge in the future.
I do agree that we want to have hard numbers.
And I do think that we should strive for the whole "we want to merge"
phase to be a time when we also look at "can we improve the
interfaces". But that can - and often does - go too far. Again, we had
_years_ of pointless masturbation over the whole Android thing, just
because people were making up new interfaces.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Eric Sandeen <esandeen@redhat.com> |
|---|---|
| Date | 2016-03-16 01:40 +0100 |
| Subject | Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks |
| Message-ID | <rd7Ff-1gK-1@gated-at.bofh.it> |
| In reply to | #1358448 |
On 3/15/16 7:06 PM, Linus Torvalds wrote: > On Tue, Mar 15, 2016 at 4:52 PM, Dave Chinner <david@fromorbit.com> wrote: >> > >> > It is pretty clear that the onus is on the patch submitter to >> > provide justification for inclusion, not for the reviewer/Maintainer >> > to have to prove that the solution is unworkable. > I agree, but quite frankly, performance is a good justification. > > So if Ted can give performance numbers, that's justification enough. > We've certainly taken changes with less. I've been away from ext4 for a while, so I'm really not on top of the mechanics of the underlying problem at the moment. But I would say that in addition to numbers showing that ext4 has trouble with unwritten extent conversion, we should have an explanation of why it can't be solved in a way that doesn't open up these concerns. XFS certainly has different mechanisms, but is the demonstrated workload problematic on XFS (or btrfs) as well? If not, can ext4 adopt any of the solutions that make the workload perform better on other filesystems? Adding the risk and complexity of the stale data exposure should be considered as a last resort, after the above questions have been answered. -Eric
[toc] | [prev] | [next] | [standalone]
| From | Chris Mason <clm@fb.com> |
|---|---|
| Date | 2016-03-16 02:00 +0100 |
| Subject | Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks |
| Message-ID | <rd7YC-1nQ-15@gated-at.bofh.it> |
| In reply to | #1358452 |
On Tue, Mar 15, 2016 at 07:30:14PM -0500, Eric Sandeen wrote: > On 3/15/16 7:06 PM, Linus Torvalds wrote: > > On Tue, Mar 15, 2016 at 4:52 PM, Dave Chinner <david@fromorbit.com> wrote: > >> > > >> > It is pretty clear that the onus is on the patch submitter to > >> > provide justification for inclusion, not for the reviewer/Maintainer > >> > to have to prove that the solution is unworkable. > > I agree, but quite frankly, performance is a good justification. > > > > So if Ted can give performance numbers, that's justification enough. > > We've certainly taken changes with less. > > I've been away from ext4 for a while, so I'm really not on top of the > mechanics of the underlying problem at the moment. > > But I would say that in addition to numbers showing that ext4 has trouble > with unwritten extent conversion, we should have an explanation of > why it can't be solved in a way that doesn't open up these concerns. > > XFS certainly has different mechanisms, but is the demonstrated workload > problematic on XFS (or btrfs) as well? If not, can ext4 adopt any of the > solutions that make the workload perform better on other filesystems? When I've benchmarked this in the past, doing small random buffered writes into an preallocated extent was dramatically (3x or more) slower on xfs than doing them into a fully written extent. That was two years ago, but I can redo it. On a fio card, this gets 16,000 iops on a preallocated extent and 40,000 iops if you run it a second time. It's not random writes, but the fsync probably means the preallocated conversion is more expensive. That's on a 4.0 kernel, but I'll rerun it on nvme on newer kernels. fio --name=fsync --rw=write --fsync=1 --bs=4k --filename=/xfs/fio_4096 --size=4g --overwrite=0 I'm happy to run variations on things, just let me know what workloads are interesting. -chris
[toc] | [prev] | [next] | [standalone]
| From | Chris Mason <clm@fb.com> |
|---|---|
| Date | 2016-03-16 23:30 +0100 |
| Subject | Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks |
| Message-ID | <rds6Z-6Wr-1@gated-at.bofh.it> |
| In reply to | #1358458 |
On Tue, Mar 15, 2016 at 05:51:17PM -0700, Chris Mason wrote: > On Tue, Mar 15, 2016 at 07:30:14PM -0500, Eric Sandeen wrote: > > On 3/15/16 7:06 PM, Linus Torvalds wrote: > > > On Tue, Mar 15, 2016 at 4:52 PM, Dave Chinner <david@fromorbit.com> wrote: > > >> > > > >> > It is pretty clear that the onus is on the patch submitter to > > >> > provide justification for inclusion, not for the reviewer/Maintainer > > >> > to have to prove that the solution is unworkable. > > > I agree, but quite frankly, performance is a good justification. > > > > > > So if Ted can give performance numbers, that's justification enough. > > > We've certainly taken changes with less. > > > > I've been away from ext4 for a while, so I'm really not on top of the > > mechanics of the underlying problem at the moment. > > > > But I would say that in addition to numbers showing that ext4 has trouble > > with unwritten extent conversion, we should have an explanation of > > why it can't be solved in a way that doesn't open up these concerns. > > > > XFS certainly has different mechanisms, but is the demonstrated workload > > problematic on XFS (or btrfs) as well? If not, can ext4 adopt any of the > > solutions that make the workload perform better on other filesystems? > > When I've benchmarked this in the past, doing small random buffered writes > into an preallocated extent was dramatically (3x or more) slower on xfs > than doing them into a fully written extent. That was two years ago, > but I can redo it. So I re-ran some benchmarks, with 4K O_DIRECT random ios on nvme (4.5 kernel). This is O_DIRECT without O_SYNC. I don't think xfs will do commits for each IO into the prealloc file? O_SYNC makes it much slower, so hopefully I've got this right. The test runs for 60 seconds, and I used an iodepth of 4: prealloc file: 32,000 iops overwrite: 121,000 iops If I bump the iodepth up to 512: prealloc file: 33,000 iops overwrite: 279,000 iops For streaming writes, XFS converts prealloc to written much better when the IO isn't random. You can start seeing the difference at 16K sequential O_DIRECT writes, but really its not a huge impact. The worst case is 4K: prealloc file: 227MB/s overwrite: 340MB/s I can't think of sequential workloads where this will matter, since they will either end up with bigger IO or the performance impact won't get noticed. -chris
[toc] | [prev] | [next] | [standalone]
| From | Ric Wheeler <rwheeler@redhat.com> |
|---|---|
| Date | 2016-03-17 14:50 +0100 |
| Subject | Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks |
| Message-ID | <rdGtj-844-5@gated-at.bofh.it> |
| In reply to | #1359399 |
On 03/16/2016 06:23 PM, Chris Mason wrote: > On Tue, Mar 15, 2016 at 05:51:17PM -0700, Chris Mason wrote: >> On Tue, Mar 15, 2016 at 07:30:14PM -0500, Eric Sandeen wrote: >>> On 3/15/16 7:06 PM, Linus Torvalds wrote: >>>> On Tue, Mar 15, 2016 at 4:52 PM, Dave Chinner <david@fromorbit.com> wrote: >>>>>> It is pretty clear that the onus is on the patch submitter to >>>>>> provide justification for inclusion, not for the reviewer/Maintainer >>>>>> to have to prove that the solution is unworkable. >>>> I agree, but quite frankly, performance is a good justification. >>>> >>>> So if Ted can give performance numbers, that's justification enough. >>>> We've certainly taken changes with less. >>> I've been away from ext4 for a while, so I'm really not on top of the >>> mechanics of the underlying problem at the moment. >>> >>> But I would say that in addition to numbers showing that ext4 has trouble >>> with unwritten extent conversion, we should have an explanation of >>> why it can't be solved in a way that doesn't open up these concerns. >>> >>> XFS certainly has different mechanisms, but is the demonstrated workload >>> problematic on XFS (or btrfs) as well? If not, can ext4 adopt any of the >>> solutions that make the workload perform better on other filesystems? >> When I've benchmarked this in the past, doing small random buffered writes >> into an preallocated extent was dramatically (3x or more) slower on xfs >> than doing them into a fully written extent. That was two years ago, >> but I can redo it. > So I re-ran some benchmarks, with 4K O_DIRECT random ios on nvme (4.5 > kernel). This is O_DIRECT without O_SYNC. I don't think xfs will do > commits for each IO into the prealloc file? O_SYNC makes it much > slower, so hopefully I've got this right. > > The test runs for 60 seconds, and I used an iodepth of 4: > > prealloc file: 32,000 iops > overwrite: 121,000 iops > > If I bump the iodepth up to 512: > > prealloc file: 33,000 iops > overwrite: 279,000 iops > > For streaming writes, XFS converts prealloc to written much better when > the IO isn't random. You can start seeing the difference at 16K > sequential O_DIRECT writes, but really its not a huge impact. The worst > case is 4K: > > prealloc file: 227MB/s > overwrite: 340MB/s > > I can't think of sequential workloads where this will matter, since they > will either end up with bigger IO or the performance impact won't get > noticed. > > -chris I think that these numbers are the interesting ones, see a 4x slow down is certainly significant. If you do the same patch after hacking XFS preallocation as Dave suggested with xfs_db, do we get most of the performance back? Ric
[toc] | [prev] | [next] | [standalone]
| From | Eric Sandeen <esandeen@redhat.com> |
|---|---|
| Date | 2016-03-15 23:40 +0100 |
| Subject | Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks |
| Message-ID | <rd5N7-8tc-5@gated-at.bofh.it> |
| In reply to | #1358224 |
On 3/15/16 3:14 PM, Dave Chinner wrote: > What we are missing is actual numbers that show that exposing stale > data is a /significant/ win for these applications that are > demanding it. And then we need evidence proving that the problem is > actually systemic and not just a hack around a bad implementation of > a feature... Thanks Dave; I totally agree on this point. We've spent more than enough time talking about how and if to implement stale data exposure, but nowhere in this thread has there been any actual performance data indicating why we should do it at all. -Eric
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2016-03-12 01:40 +0100 |
| Message-ID | <rbFL3-63Y-3@gated-at.bofh.it> |
| In reply to | #1356243 |
On Fri, Mar 11, 2016 at 2:30 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Fri, Mar 11, 2016 at 10:25:30AM -0800, Linus Torvalds wrote:
>>
>> So you'd have to explicitly say "my setup is ok with hole punching".
>
> Except it's not hole punching that is the problem. [..]
> The problem here is
> preallocation of unwritten blocks that expose the stale data if the
> filesystem skips marking those blocks as unwritten.
Right you are.
> It's all well and good to restrict access to the fallocate() call to
> limit who can expose stale data, but it doesn't remove the fact it
> is easy for stale data to unintentionally escape the privileged
> group once it has been exposed because there is no record of the
> fact the file contains uninitialised blocks....
Good point.
It's not just that the user in question *couldn't* have exposed it
other ways by reading the raw device and then writing that info into a
file. You're right that the "don't initialize unwritten blocks" thing
has a more insidious problem of making it easy to unintentionally
expose such data just because you missed an error path (or the process
died without ever doing the proper over-write)..
It would be much better if we could somehow mitigate just _how_ easy
it is to screw up.
One way to do that would be to not just require that the user that
discards the initializing writes have read access to the underlying
device, but perhaps also have some strict requirement that you can
discard only if the file you are working with is legible only to you?
That would limit the damage, and keep the stale data "private" to the
user who is already able to read the raw data off the device. Sure,
you can then mark the file read-by-world by others later, but at that
point you're kind of *consciously* exposing that stale data (and at
that point, you have hopefully cleaned it all up and replaced the
stale data with real data).
But would that perhaps not be reasonable for the kind of use cases
that google has now?
Linus
[toc] | [prev] | [standalone]
Page 3 of 3 — ← Prev page 1 2 [3]
Back to top | Article view | linux.kernel
csiph-web