Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC]: add discard support for vhd #265

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AlexLanzano
Copy link

Discard is enabled when the frontend sets feature-discard to 1, the discard-granularity to the device sector size, and the discard-alignment to 0. These values will be written to the xenstore.

Discard is enabled if the following conditions are true:

  1. The device backend supports discard
  2. the device is writable
  3. the device is not a cdrom

How tapdisk handles discard requests:

  1. Requests are pulled off the ring from xenio_blkif_get_request(). Depending on the protocol,
    the discard request will be converted into a blkif_request_discard.
  2. The request is then converted to a td_vbd_request via tapdisk_xenblkif_make_vbd_request()
    and the vbd request is queued using tapdisk_vbd_queue_request()
  3. tapdisk_vbd_recheck_state() sees the discard request waiting to be issued.
  4. tapdisk_vbd_issue_request_discard() packages the vbd request into a td_request
  5. The request will then be handed off to the block driver to queue the request.

The portion of my patch explained above is a slightly modified version of a previous PR 1a51656. Credit goes to Rober Breker for implementing a lot of the discard request handling for tapdisk.

How the vhd block driver handles discard requests:

  1. vhd_queue_discard() receives the request.
  2. The request will be separated into smaller requests where the sectors to be discard would
    be equal to the block length.
  3. The smaller requests will be queued in preallocated request array.
  4. tapdisk_rwio_discard() pulls requests off of the request array and fallocate() is called to
    deallocate the block.

I had to change the tapdisk-queue driver from using LIO to RWIO because currently linux AIO does not support discard requests. This change causes a 50% sequential write degradation. I've attempted to remedy this by using LIO when there are no discard requests on the queue and revert back to RWIO when a discard is queued. Sadly, the IO scheduler doesn't like when you mix LIO and RWIO.

Are there any other approaches I could take? Is discard request support something you guys are interested in having?

drivers/block-vhd.c Show resolved Hide resolved
drivers/block-vhd.c Outdated Show resolved Hide resolved
tapback/tapback.c Outdated Show resolved Hide resolved
@MarkSymsCtx
Copy link
Contributor

Yes, this is interesting but the serious performance degradation would need to be resolved. Given that the current performance of tapdisk isn't as good as we would like degrading it further even with the benefit of trim would be a step in the wrong direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants