Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1360083 > unrolled thread

Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

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.

Contents

  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

#1360083 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

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?

> As noted though, we've basically given up and are moving to a
> pure-userspace solution as quickly as we can.

That argues against worrying about this all in the kernel unless there
are other users.

                  Linus

[toc] | [next] | [standalone]

#1360094

On Thu, Mar 17, 2016 at 10:50 AM, Ric Wheeler <rwheeler@redhat.com> wrote:
>>
>> That argues against worrying about this all in the kernel unless there
>> are other users.
>
> Just a note, when Greg says "user space solution", Ceph is looking at
> writing directly to raw block devices which is kind of a through back to
> early enterprise database trends.

Right, I understand. But that makes it kind of pointless to worry
about NO_HIDE_STALE, since it wouldn't get used anyway. The issues
with filesystem preallocation just don't exist.

Of course, if there are other possible users, we should keep this on
the table, but ...

                 Linus

[toc] | [prev] | [next] | [standalone]

#1360096 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On 03/17/2016 01:47 PM, Linus Torvalds 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?
>
>> As noted though, we've basically given up and are moving to a
>> pure-userspace solution as quickly as we can.
> That argues against worrying about this all in the kernel unless there
> are other users.
>
>                    Linus

Just a note, when Greg says "user space solution", Ceph is looking at writing 
directly to raw block devices which is kind of a through back to early 
enterprise database trends.

Ric

[toc] | [prev] | [next] | [standalone]

#1360130 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Thu, Mar 17, 2016 at 10:47:29AM -0700, Linus Torvalds 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.

We haven't had internal tiers yelling at us for fallocate performance,
so I'm unlikely to suggest it, just because its a potential
privacy leak we'd have to educate people about.  What I'd be more likely
to use is code inside the filesystem like this:

somefs_fallocate() {
	if (trim_can_really_zero(my_device)) {
		trim
		allocate a regular extent
		return
	} else {
		do normal fallocate
	}
}

Then the out of tree patch (for google or whoever) becomes a hack to
flip trim_can_really_zero on a given block device.  The rest of us can
use explicit interfaces from the hardware when deciding what we want
preallocation to mean.

It gets messy for crcs in btrfs, so we'd need the old fashioned
preallocation anyway.  But the database workloads where this matters
aren't our target right now, so its more an ext4/xfs thing anyway.

-chris

[toc] | [prev] | [next] | [standalone]

#1360188

[Multipart message — attachments visible in raw view] — view raw

On Mar 17, 2016, at 12:35 PM, Chris Mason <clm@fb.com> wrote:
> 
> On Thu, Mar 17, 2016 at 10:47:29AM -0700, Linus Torvalds 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.
> 
> We haven't had internal tiers yelling at us for fallocate performance,
> so I'm unlikely to suggest it, just because its a potential
> privacy leak we'd have to educate people about.  What I'd be more likely
> to use is code inside the filesystem like this:
> 
> somefs_fallocate() {
> 	if (trim_can_really_zero(my_device)) {
> 		trim
> 		allocate a regular extent
> 		return
> 	} else {
> 		do normal fallocate
> 	}
> }

We were discussing almost this very same thing in the ext4 concall today.

Ted initially didn't think it was worthwhile to implement, but after looking
at the whitelist for SATA SSDs it seems that there are enough devices on the
market that support the ATA_HORKAGE_ZERO_AFTER_TRIM to make this approach
worthwhile to implement.

Also, if the ext4 extent size was limited it might even be possible to do
this efficiently enough with write_same on HDD devices.

> Then the out of tree patch (for google or whoever) becomes a hack to
> flip trim_can_really_zero on a given block device.  The rest of us can
> use explicit interfaces from the hardware when deciding what we want
> preallocation to mean.

This might be a bit trickier, since this would affect all zero/trim
operations, not just ones for uninitialized data extents.

> It gets messy for crcs in btrfs, so we'd need the old fashioned
> preallocation anyway.  But the database workloads where this matters
> aren't our target right now, so its more an ext4/xfs thing anyway.


Cheers, Andreas

[toc] | [prev] | [next] | [standalone]

#1360195 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Thu, Mar 17, 2016 at 02:49:06PM -0600, Andreas Dilger wrote:
> On Mar 17, 2016, at 12:35 PM, Chris Mason <clm@fb.com> wrote:
> > 
> > On Thu, Mar 17, 2016 at 10:47:29AM -0700, Linus Torvalds 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.
> > 
> > We haven't had internal tiers yelling at us for fallocate performance,
> > so I'm unlikely to suggest it, just because its a potential
> > privacy leak we'd have to educate people about.  What I'd be more likely
> > to use is code inside the filesystem like this:
> > 
> > somefs_fallocate() {
> > 	if (trim_can_really_zero(my_device)) {
> > 		trim
> > 		allocate a regular extent
> > 		return
> > 	} else {
> > 		do normal fallocate
> > 	}
> > }
> 
> We were discussing almost this very same thing in the ext4 concall today.
> 
> Ted initially didn't think it was worthwhile to implement, but after looking
> at the whitelist for SATA SSDs it seems that there are enough devices on the
> market that support the ATA_HORKAGE_ZERO_AFTER_TRIM to make this approach
> worthwhile to implement.

We'll end up with people complaining it makes fallocate slower because
of the trims, so it's not a perfect solution.  But I much prefer it to
fallocate-stale.

> 
> Also, if the ext4 extent size was limited it might even be possible to do
> this efficiently enough with write_same on HDD devices.
> 
> > Then the out of tree patch (for google or whoever) becomes a hack to
> > flip trim_can_really_zero on a given block device.  The rest of us can
> > use explicit interfaces from the hardware when deciding what we want
> > preallocation to mean.
> 
> This might be a bit trickier, since this would affect all zero/trim
> operations, not just ones for uninitialized data extents.

Thinking more, my guess is that google will just keep doing what they
are already doing ;)  But there could be a flag in sysfs dedicated to
trim-for-fallocate so admins can see what their devices are reporting.
readonly in mainline, if someone wants to patch it in their large data
center it wouldn't be hard.

-chris

[toc] | [prev] | [next] | [standalone]

#1360326 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Thu, Mar 17, 2016 at 02:00:18PM -0700, Chris Mason wrote:
> 
> Thinking more, my guess is that google will just keep doing what they
> are already doing ;)  But there could be a flag in sysfs dedicated to
> trim-for-fallocate so admins can see what their devices are reporting.
> readonly in mainline, if someone wants to patch it in their large data
> center it wouldn't be hard.

That's true, because one of the major use cases is SATA drives where
trim isn't available.  Even for SAS drives where you have WRITE SAME,
you wouldn't want to use it for large fallocate regions.

So I see using reliable trim as a zeroing mechanism to be orthogonal
to the question of NO_HIIDE_STALE.

I do think that using TRIM in various causes where we are doing an
fallocate does make sense for non-rotational devices.  In general TRIM
should be fast enough that that I'd be surprised that people would be
complaining --- especially since most of the time, fallocate isn't on
the timing-critical path of most applications.

							- Ted

[toc] | [prev] | [next] | [standalone]

#1360728

"Theodore Ts'o" <tytso@mit.edu> writes:

> I do think that using TRIM in various causes where we are doing an
> fallocate does make sense for non-rotational devices.  In general TRIM
> should be fast enough that that I'd be surprised that people would be
> complaining --- especially since most of the time, fallocate isn't on
> the timing-critical path of most applications.

TRIM/UNMAP isn't just supported on solid state devices, though.  I do
recall some enterprise thinly provisioned storage that would take ages
to discard large regions.  I think that caused us to change the defaults
for mkfs, right?

Cheers,
Jeff

[toc] | [prev] | [next] | [standalone]

#1360903

>>>>> "Jeff" == Jeff Moyer <jmoyer@redhat.com> writes:

Jeff> TRIM/UNMAP isn't just supported on solid state devices, though.  I
Jeff> do recall some enterprise thinly provisioned storage that would
Jeff> take ages to discard large regions.  I think that caused us to
Jeff> change the defaults for mkfs, right?

I think those have largely been fixed. But, yes.

-- 
Martin K. Petersen	Oracle Linux Engineering

[toc] | [prev] | [next] | [standalone]

#1360381

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]

#1360393

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] | [standalone]

Back to top | Article view | linux.kernel

csiph-web