Page MenuHomeFreeBSD

geom/zero: Add support for unmapped I/O
AcceptedPublic

Authored by 0mp on Thu, Oct 9, 4:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 4, 5:45 AM
Unknown Object (File)
Sun, Nov 2, 5:33 AM
Unknown Object (File)
Fri, Oct 31, 6:49 PM
Unknown Object (File)
Fri, Oct 31, 6:38 PM
Unknown Object (File)
Thu, Oct 30, 2:24 PM
Unknown Object (File)
Thu, Oct 30, 10:25 AM
Unknown Object (File)
Tue, Oct 28, 7:09 PM
Unknown Object (File)
Tue, Oct 28, 5:23 PM
Subscribers

Details

Reviewers
markj
bnovkov
Summary

geom/zero: Add support for unmapped I/O

This patch adds support for unmapped I/O to gzero(4).

Let's consider the following script to illustrate the change in
gzero(4)'s behavior:

dd="dd if=/dev/gzero of=/dev/null bs=512 count=100000"
dtrace -q -c "$dd" -n '
    fbt::pmap_qenter:entry,
    fbt::uiomove_fromphys:entry,
    fbt::memset:entry
    /execname == "dd"/
    {
        @[probefunc] = count();
    }
'

Let's run that script 4 times:

==> 1: unmapped I/O not supported (fallback to mapped I/O), kern.geom.zero.clear=1
51200000 bytes transferred in 1.795809 secs (28510829 bytes/sec)
  pmap_qenter                                                  100000
  memset                                                       400011

==> 2: unmapped I/O not supported (fallback to mapped I/O), kern.geom.zero.clear=0
51200000 bytes transferred in 0.701079 secs (73030337 bytes/sec)
  memset                                                       300011

==> 3: unmapped I/O supported, kern.geom.zero.clear=1
51200000 bytes transferred in 0.771680 secs (66348750 bytes/sec)
  uiomove_fromphys                                             100000
  memset                                                       300011

==> 4: unmapped I/O supported, kern.geom.zero.clear=0
51200000 bytes transferred in 0.621303 secs (82407407 bytes/sec)
  memset                                                       300011

If kern.geom.zero.clear=0, then nothing really changes as no copying takes
place. Otherwise, we see by adding unmapped I/O support we avoid calls to
pmap_qenter(), which was called by GEOM to turn unmapped I/O requests into
mapped ones before passing them for processing to gzero(4).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 68366
Build 65249: arc lint + arc unit

Event Timeline

0mp requested review of this revision.Thu, Oct 9, 4:17 PM
0mp planned changes to this revision.Tue, Oct 28, 2:15 PM
0mp retitled this revision from geom/zero: Implement unmapped IO to geom/zero: Add support for unmapped I/O.Fri, Oct 31, 5:33 PM
0mp edited the summary of this revision. (Show Details)
0mp added reviewers: markj, bnovkov.
  • Fix implementation
  • Add comments
  • Add commit message
sys/geom/zero/g_zero.c
68–71

This is out of scope for now.

122–128

Bojan suggested that I actually check for !g_zero_clear here and break early, but we need to fall through to BIO_WRITE to set bio_completed and error, so breaking early is not an option.

sys/geom/zero/g_zero.c
103

We should break from the loop once resid == 0.

117

This comment is very pedantic. I understand that it's useful to write it out when doing this kind of thing for the first time, but I don't think it's necessary to spell it out. The comment before the loop is good, but the rest are overkill IMO.

121
154–155
0mp marked 3 inline comments as done.
  • Address feedback
0mp added inline comments.
sys/geom/zero/g_zero.c
103

I'll bring that check back.

I've removed it because I assume that if my math around length is correct, then the check is redundant under the assumption that the amount of memory given to us via bio_ma , bio_ma_n`, and bio_ma_offset is exactly equal to bp->bio_length.

117

Great! Thanks for the feedback. I'll remove the other comments. Bojan told me earlier that he'd like to see more comments explaining what is going on here since it's rare to see a for loop around uiomove_fromphys(). However, it seems like I went a bit too far with the comments, haha.

This revision is now accepted and ready to land.Wed, Nov 5, 10:50 AM

Thanks, Bojan! I'll wait a couple of days for Mark's feedback before committing.