xfs: batched discard support

Given that everyone is so big in the discard discussion I'd like to present what I had started to prepare for XFS. I didn't plan to send it out until I get my hands onto a TRIM capable device (or at least get time to add support to qemu), and so far it's only been tested in dry-run mode.

The basic idea is to add an ioctl which walks the free space btrees in each allocation group and simply discard everythin that is free. Given that XFS doesn't gragment freespace very much that's a very efficient way to do it. In addition we also already support setting a threshold under which we don't bother to discard an extent, it's currently hardcoded in the helper tool. In the future we could also add things like a sequence number in the AG headers if anything has changed at all, but let's leave those optimizations until we need them.

XFS locks the allocation btree using the btree buffers, so we do not block allocations from any extent which we're not currenly discarding.

Now the caveat for that is that we really do want to do the discard synchronously, that is wait for the request to finish. That's what I've implemented in this patch, but it's the part I haven't been able to test so far. (and yes, this should be separate patch, but it's really just an RFC for now)

Mark, any chance to try it? Just create an XFS filesystem, age it a bit and then call the attached little trim.c program on the mountmoint (or any file inside the filesystem for that matter)

Christoph Hellwig wrote:

Quoted 3 lines

Mark, any chance to try it? Just create an XFS filesystem, age it a bit and then call the attached little trim.c program on the mountmoint (or any file inside the filesystem for that matter)

..

Looking at it now. Thanks, Christoph!

Quoted 1 line · Mark Lord

Christoph Hellwig wrote:

..

Quoted 3 lines

Quoted 3 lines

Mark, any chance to try it? Just create an XFS filesystem, age it a bit and then call the attached little trim.c program on the mountmoint (or any file inside the filesystem for that matter)

Quoted 3 lines

..

Looking at it now. Thanks, Christoph!

..

Fails to work on 64-bit kernel w/ 32-bit userspace (no compat ioctl). Rebuilding with 32-bit kernel now..

Quoted 6 lines · Mark Lord

Quoted 1 line · Mark Lord

Christoph Hellwig wrote:

..

Quoted 3 lines

Quoted 3 lines

Mark, any chance to try it? Just create an XFS filesystem, age it a bit and then call the attached little trim.c program on the mountmoint (or any file inside the filesystem for that matter)

Quoted 7 lines

Quoted 3 lines

..

Looking at it now. Thanks, Christoph!

..

Fails to work on 64-bit kernel w/ 32-bit userspace (no compat ioctl). Rebuilding with 32-bit kernel now..

The actual ioctl is compatible, just add the

	case XFS_IOC_TRIM:
		return xfs_ioc_trim(mp, arg);

to xfs_file_compat_ioctl(). I'll add this to the next spin of the patch.

Quoted 20 lines · Christoph Hellwig

Quoted 6 lines · Mark Lord

Quoted 1 line · Mark Lord

Christoph Hellwig wrote:

..

Quoted 3 lines

Quoted 3 lines

Mark, any chance to try it? Just create an XFS filesystem, age it a bit and then call the attached little trim.c program on the mountmoint (or any file inside the filesystem for that matter)

Quoted 7 lines

Quoted 3 lines

..

Looking at it now. Thanks, Christoph!

..

Fails to work on 64-bit kernel w/ 32-bit userspace (no compat ioctl). Rebuilding with 32-bit kernel now..

The actual ioctl is compatible, just add the

    case XFS_IOC_TRIM:
        return xfs_ioc_trim(mp, arg);

Quoted 1 line

to xfs_file_compat_ioctl(). I'll add this to the next spin of the patch.

..

Peachy. Rebuilding again now..

Don't wait up for this .. I'll finish it on Sunday. Bedtime. :)

Quoted 6 lines · Mark Lord

Quoted 1 line · Mark Lord

Christoph Hellwig wrote:

..

Quoted 3 lines

Quoted 3 lines

Mark, any chance to try it? Just create an XFS filesystem, age it a bit and then call the attached little trim.c program on the mountmoint (or any file inside the filesystem for that matter)

Quoted 7 lines

Quoted 3 lines

..

Looking at it now. Thanks, Christoph!

..

Fails to work on 64-bit kernel w/ 32-bit userspace (no compat ioctl). Rebuilding with 32-bit kernel now..

The actual ioctl is compatible, just add the

	case XFS_IOC_TRIM:
		return xfs_ioc_trim(mp, arg);

to xfs_file_compat_ioctl(). I'll add this to the next spin of the patch.

Okay, this gives me ENOSYS now --> discard/trim support is missing from the lower layers.

What other patches do I need to make this work?

The latest from Matthew's discard tree (May 2009) don't appear to be sufficient, even after updating them for 2.6.31-rc6.

Quoted 21 lines · Christoph Hellwig

Quoted 6 lines · Mark Lord

Quoted 1 line · Mark Lord

Christoph Hellwig wrote:

..

Quoted 3 lines

Quoted 3 lines

Mark, any chance to try it? Just create an XFS filesystem, age it a bit and then call the attached little trim.c program on the mountmoint (or any file inside the filesystem for that matter)

Quoted 7 lines

Quoted 3 lines

..

Looking at it now. Thanks, Christoph!

..

Fails to work on 64-bit kernel w/ 32-bit userspace (no compat ioctl). Rebuilding with 32-bit kernel now..

The actual ioctl is compatible, just add the

    case XFS_IOC_TRIM:
        return xfs_ioc_trim(mp, arg);

to xfs_file_compat_ioctl(). I'll add this to the next spin of the patch.

..

Okay, this gives me ENOSYS now --> discard/trim support is missing from the lower layers.

What other patches do I need to make this work?

The latest from Matthew's discard tree (May 2009) don't appear to be sufficient, even after updating them for 2.6.31-rc6.

Slow, but presumably thorough.
Subsequent runs were equally slow.

The problem is, it still issues TRIMs to the LLD one extent at a time. Compare this with doing it all in a single TRIM command with the wiper.sh script (filesystem unmounted):

[~] time wiper.sh /dev/sdb3 --commit

wiper.sh: Linux SATA SSD TRIM utility, version 1.9b, by Mark Lord. Preparing for offline TRIM of free space on /dev/sdb3 (xfs
non-mounted).
This operation could destroy your data. Are you sure (y/N)? y Syncing disks..
Beginning TRIM operations..
Trimming 168 free extents encompassing 8793136 sectors (4294 MB) Done.

real 0m1.249s
user 0m0.110s
sys 0m0.063s

That includes the time for me to type 'y' and hit enter. :)

Okay, I got Matthews patches updated onto 2.6.31, and fixed the incompatibilities between those and the XFS TRIM patch (from Christoph), plus a sector_t printk issue.

My apologies for attachments, but I am attaching the updated Christoph patch, as well as my hacked-up forward-port of Matthew's patches.

Not pretty, but they work. :)

Now.. running Christoph's "xfs trim" on a 4.6GB mostly already-trimmed XFS partition gave this for the first time around:

The problem is, it still issues TRIMs to the LLD one extent at a time. Compare this with doing it all in a single TRIM command with the wiper.sh script (filesystem unmounted):

Quoted 10 lines · Mark Lord

Okay, I got Matthews patches updated onto 2.6.31, and fixed the incompatibilities between those and the XFS TRIM patch (from Christoph), plus a sector_t printk issue.

My apologies for attachments, but I am attaching the updated Christoph patch, as well as my hacked-up forward-port of Matthew's patches.

Not pretty, but they work. :)

Now.. running Christoph's "xfs trim" on a 4.6GB mostly already-trimmed XFS partition gave this for the first time around:

Quoted 3 lines

The problem is, it still issues TRIMs to the LLD one extent at a time. Compare this with doing it all in a single TRIM command with the wiper.sh script (filesystem unmounted):

I could do a variant which issues a single TRIM, but that would require us to lock out all other allocations for the time the trim takes. I'll hack that up once I get some time.

Given that everyone is so big in the discard discussion I'd like to present what I had started to prepare for XFS. I didn't plan to send it out until I get my hands onto a TRIM capable device (or at least get time to add support to qemu), and so far it's only been tested in dry-run mode.

The basic idea is to add an ioctl which walks the free space btrees in each allocation group and simply discard everythin that is free. [...]

A general interface design question: you added a new ioctl XFS_IOC_TRIM case. It's a sub-case of an ugly-looking demultiplexing xfs_file_ioctl().

What is your threshold for turning something into a syscall? When are ioctls acceptable in your opinion?

I'm asking this because we are facing a similar problem with perfcounters: we need to extend the ioctl functionality there but introducing a new syscall looks wasteful.

So i'm torn about the 'syscall versus ioctl' issue, i'd like to avoid making interface design mistakes and i'd like to solicit some opinions about this. I've attached the perfcounters ioctl patch below.

Quoted 4 lines

So i'm torn about the 'syscall versus ioctl' issue, i'd like to avoid making interface design mistakes and i'd like to solicit some opinions about this. I've attached the perfcounters ioctl patch below.

Only add a syscall if it has _one_ clear defined purpose, which has kernel-wide meaning.

Quoted 3 lines · Christoph Hellwig

Quoted 1 line

So i'm torn about the 'syscall versus ioctl' issue, i'd like to avoid making interface design mistakes and i'd like to solicit some opinions about this. I've attached the perfcounters ioctl patch below.

Only add a syscall if it has _one_ clear defined purpose, which has kernel-wide meaning.

One clear defined purpose which comes to mind is a "trim" or "punch" system call, for making holes in files as well as trimming block devices. Several other OSes have that capability on files.

I don't remember - does TRIM guarantee the blocks read zeros afterwards?

It would be tidy if it does, as it could have the same meaning with files.

I don't remember - does TRIM guarantee the blocks read zeros afterwards?

I don't remember - does TRIM guarantee the blocks read zeros afterwards?

[resending, after fixing the Cc: list; somebody trimmed it earlier]

Jamie Lokier wrote:
..

Quoted 1 line

I don't remember - does TRIM guarantee the blocks read zeros afterwards?

..

No, it doesn't.

A drive can optionally support "deterministic TRIM", whereby it will return consistent data for any given trimmed sector afterwards, but that doesn't mean zeros.

-ml

Note that returning consistent data is critical for devices that are used in a RAID group since you will need each RAID block that is used to compute the parity to continue to return the same data until you overwrite it with new data :-)

If we have a device that does not support this (or is misconfigured not to do this), we should not use those devices in an MD group & do discard against it...

Quoted 6 lines · Ric Wheeler

Note that returning consistent data is critical for devices that are used in a RAID group since you will need each RAID block that is used to compute the parity to continue to return the same data until you overwrite it with new data :-)

Quoted 3 lines

If we have a device that does not support this (or is misconfigured not to do this), we should not use those devices in an MD group & do discard against it...

..

Well, that's a bit drastic. But the RAID software should at least not issue TRIM commands in ignorance of such.

Would it still be okay to do the TRIMs when the entire parity stripe (across all members) is being discarded? (As opposed to just partial data there being dropped)

Would it still be okay to do the TRIMs when the entire parity stripe (across all members) is being discarded? (As opposed to just partial data there being dropped)

Quoted 9 lines · Mark Lord

[resending, after fixing the Cc: list; somebody trimmed it earlier]

Jamie Lokier wrote:
..

Quoted 1 line

I don't remember - does TRIM guarantee the blocks read zeros afterwards?

..

No, it doesn't.

Quoted 4 lines

A drive can optionally support "deterministic TRIM", whereby it will return consistent data for any given trimmed sector afterwards, but that doesn't mean zeros.

-ml

Note that returning consistent data is critical for devices that are used in a RAID group since you will need each RAID block that is used to compute the parity to continue to return the same data until you overwrite it with new data :-)

If we have a device that does not support this (or is misconfigured not to do this), we should not use those devices in an MD group & do discard against it...

Quoted 8 lines · Ric Wheeler

Note that returning consistent data is critical for devices that are used in a RAID group since you will need each RAID block that is used to compute the parity to continue to return the same data until you overwrite it with new data :-)

If we have a device that does not support this (or is misconfigured not to do this), we should not use those devices in an MD group & do discard against it...

..

Well, that's a bit drastic. But the RAID software should at least not issue TRIM commands in ignorance of such.

Would it still be okay to do the TRIMs when the entire parity stripe (across all members) is being discarded? (As opposed to just partial data there being dropped)

Quoted 11 lines · Mark Lord

Quoted 9 lines · Ric Wheeler

Note that returning consistent data is critical for devices that are used in a RAID group since you will need each RAID block that is used to compute the parity to continue to return the same data until you overwrite it with new data :-)

If we have a device that does not support this (or is misconfigured not to do this), we should not use those devices in an MD group& do discard against it...

Quoted 9 lines

..

Well, that's a bit drastic. But the RAID software should at least not issue TRIM commands in ignorance of such.

Would it still be okay to do the TRIMs when the entire parity stripe (across all members) is being discarded? (As opposed to just partial data there being dropped)

I think there might be a related usecase that could benefit from TRIM/UNMAP/whatever support in file systems even if the physical devices do not support that. I have a RAID5 at work with LVM over it. This week I deleted an old logical volume of some 200GB that has been moved to a different volume group, tomorrow I will start to replace all the disks in the raid with bigger ones. So if the LVM told the raid "hey, this space is totally garbage from now on" the raid would not have to do any calculation when it has to rebuild that but could simply write fixed patterns to all disks (e.g. 0 to first data, 0 to second data and 0 as "0 xor 0" to parity). With the knowledge that some of the underlying devices would support "write all to zero" this operation could be speed up even more, with "write all fixed pattern" every unused chunk would go down to a single write operation (per disk) on rebuild regardless which parity algorithm is used.

And even if things are in use the RAID can benefit from such things. If we just define that every unmapped space will always be 0 when read and I write to a raid volume and the other part of the checksum calculation is unmapped checksumming becomes easy as we already know half of the values before: 0. So we can save the reads from the second data stripe and most of the calculation. "dd if=/dev/md0" on an unmapped space is more or less the same as "dd if=/dev/zero" than.

I only fear that these things are too obviously as I would be the first to have this idea ;)

Greetings,

Eike

Quoted 8 lines · Mark Lord

Quoted 6 lines · Ric Wheeler

Note that returning consistent data is critical for devices that are used in a RAID group since you will need each RAID block that is used to compute the parity to continue to return the same data until you overwrite it with new data :-)

Quoted 8 lines

Quoted 3 lines

If we have a device that does not support this (or is misconfigured not to do this), we should not use those devices in an MD group & do discard against it...

..

Well, that's a bit drastic. But the RAID software should at least not issue TRIM commands in ignorance of such.

If the storage can return different data in a sequence of READ requests of the same sector (with no writes), there is nothing RAID could do. It would see total garbage...

Quoted 3 lines

Would it still be okay to do the TRIMs when the entire parity stripe (across all members) is being discarded? (As opposed to just partial data there being dropped)

This should be safe if the MD bitmaps would prevent us from trying to READ/regenerate parity for that stripe...

ric

Quoted 3 lines · Ingo Molnar

A general interface design question: you added a new ioctl XFS_IOC_TRIM case. It's a sub-case of an ugly-looking demultiplexing xfs_file_ioctl().

ioctl is per defintion a multiplexer.

Quoted 11 lines

What is your threshold for turning something into a syscall? When are ioctls acceptable in your opinion?

I'm asking this because we are facing a similar problem with
perfcounters: we need to extend the ioctl functionality there but introducing a new syscall looks wasteful.

So i'm torn about the 'syscall versus ioctl' issue, i'd like to avoid making interface design mistakes and i'd like to solicit some opinions about this. I've attached the perfcounters ioctl patch below.

Only add a syscall if it has _one_ clear defined purpose, which has kernel-wide meaning.

Do not add an syscall that is just another multiplexer without structure. Most likely it will just be even worse than sys_ioctl.

Also really don't bother adding a system call that is specific to one singler driver or filesystem. Besides horrible logistics - you'd need some always built-in stub calling out to the possibly modular drivers/filesystem - it also simply doesn't make any semantical sense. I can't say I like the ioctl use in perfcounters much, but adding a special syscalls instead would be even more horrible.

As for the trim support this really just was an RFC to start bringing some code into play instead of the endless masturbation about hat code that doesn't exist happens on hardware most people don't have. The interface will most ceetainly change and I hope we will have a common interface for all filesystems (or at least those that care).

Quoted 1 line

ioctl is per defintion a multiplexer.

Yes, and? There's two variants of multiplexing:

- multiplex something rather straightforward and related
- multiplex unrelated fields, data, structures

I consider the second one 'ugly', the first one 'ok-ish'. YMMV.

I'd like to understand the technical basis of your critisism and i'd like to address any deficiencies of the perfcounters code. You said you dont like the ioctl solution we have, but that you'd like a separate syscall even less.

Perfcounters are a kernel-wide concept, encompassing 100% of all Linux installations, not just some special hardware.

[ chaged the subject and droped the cc list down as this is not
about discard support anymore]

Quoted 8 lines

Quoted 1 line

ioctl is per defintion a multiplexer.

Yes, and? There's two variants of multiplexing:

- multiplex something rather straightforward and related
- multiplex unrelated fields, data, structures

I consider the second one 'ugly', the first one 'ok-ish'. YMMV.

Exactly. And ioctl is per defintion the latter. [...]

[...] sys_ioctl is a way to give drivers (and in Linux filesystem, too) a way to implement their own syscall table extension for file descritors they control. While there might be a few "ok-ish" or "nice" ioctl handlers most of them are ugly. That's why people don't like them too much, btw :)

Quoted 4 lines

I'd like to understand the technical basis of your critisism and i'd like to address any deficiencies of the perfcounters code. You said you dont like the ioctl solution we have, but that you'd like a separate syscall even less.

I really don't like any ioctl solution very much. But sometimes it's the lesser of the evils, often by a bug margin.

Quoted 2 lines

Perfcounters are a kernel-wide concept, encompassing 100% of all Linux installations, not just some special hardware.

That doesn't really matter. The tty code is also used by 100% of the Linux installation, and still it implements the tty controls as ioctls.

Generally having system calls that don't operate on file descriptor genericly (that doesn't mean implemented everywhere, but beeing a generic concept dispatched through file operations) is pretty ugly. We have a lot of them anyway, e.g. the socket system calls, eventfd, inotify. They don't really buy us anything over ioctls, but they are accepted. So if you design a clean system call working on perfcounters file descriptor no one could have hard objection (as in actually saying no). But if you really just do another stupid muliplexer just go for ioctl instead of reinventing the dispatching and multiplexing.