supporting discard/trim

[mischief]

It would be great if the driver could support passing trim commands. I believe it doesnt work right now because of a missing flag in the request_queue.

[ecashin]

On 03/13/2016 05:37 AM, Nick Owens wrote:

It would be great if the driver could support passing trim commands. I
believe it doesnt work right now because of a missing flag in the
request_queue.

Hi, Nick Owens. Did you already check whether a generic Linux block
device layer allows trim commands? If not, did you imagine an
out-of-band mechanism of some kind?

By out of band I mean outside the operations performed on the block
device itself. Examples include /sys operations and special files like
/dev/etherd/discover.

Ed

[mischief]

i'm pretty sure any old block device can support it.

it seems to be a matter of having the correct flags in the request_queue.

for example, nbd sets this here: https://github.com/torvalds/linux/blob/master/drivers/block/nbd.c#L750

the goal would be to mount the aoe blockdev with mount -odiscard ....

[ecashin]

I'm looking into it now, mostly reading Linux sources and patches so far. Have you already figured out how to get your AoE target to do the ATA that will show you're ready for the TRIM command?

[mischief]

yes, it was just a matter of fixing endianness of my ata identity. i missed the right one the first time through the spec :)

however, i seem to have run into a crash inside of the aoe driver when i try to use blkdiscard on the aoe device.

[mischief]

attached is the diff i was using to enable discard on the aoe block device if the remote ata target advertises trim in the identity.

once this is applied, simply running e.g. blkdiscard -v /dev/etherd/e1.1 will make the driver crash with a nil pointer deref. i did not have the time to dig into the nature of the issue.

0001-add-trim-support.txt

[ecashin]

Oh, super! Thanks for the patch. Because of the way the upstream works, can you please include a "Signed-off-by" as Documentation/SubmittingPatches describes?

Ideally, you could also fork this repo and create a pull request with a commit that is up to Andrew Morton's standards---He likes to immediately see why the users would care about the changes by reading the commit message. The short commit message specs are also described in SubmittingPatches, I think.

You might be thinking that I'm being a hypocrite, but my aoe-repo commits have terse messages only because I'm likely the one who will consolidate and summarize them before shipping them upstream. For the patches of others, it's more practical to front-load it.

Also, is your target available for others to use for testing purposes? I'm guessing it would be helpful to have it on my network as e1.1 before trying your blkdiscard command.

[mischief]

unfortunately, the application is not yet open source - but maybe soon.

it should be easy enough to patch vblade to make it claim TRIM support, but error when the command is sent.

[mischief]

attached is a patch for vblade suitable to trick the kernel's ata_id_has_trim function.

0001-fake-ata-identity.txt

[ecashin]

Thanks. I am going to re-do my virtual testbed a bit more and then use your patch with vblade to check it out.

[ecashin]

I finally got things all set up but with your patches applied to the vblade (on host two) and aoe driver (on host one) I am getting "Operation not supported" when I issue the blkdiscard command you mentioned (after building the latest utils-linux from source).

Do you think there's anything you might have forgotten to include, or maybe there's something different about my system. I'm testing with Ubuntu 14.04 on x86_64.