makefs: make msdos creation go fast

[imp]

Summary

Add missing brelse(bp). Without it the cache grows and we have a n^2
lookup. I'm not entirely sure why we read the block before we write it
back out, since the only side effect of that is to allocate memory,
clear the memory, read it in from disk, throw it away with the contents
of the file being written out. We likely should just do a getblk() here
instead, but even with all that, this takes the time it takes to create
a 150MB msdos fs image down from 5 minutes to 30 seconds.

Before:
317.663u 0.685s 5:18.34 100.0% 198+360k 0+19io 1009pf+0w
After:
7.330u 23.841s 0:31.17 100.0% 198+360k 0+250522io 4pf+0w

Sponsored by: Netflix

[imp]

usr.sbin/makefs/msdos/msdosfs_vnops.c

496

This bread likely should be a getblk instead.
And we should likely remove its memset(0) and require everybody to initialize things...

[emaste]

kernel bufwrite() does:

if ((oldflags & B_ASYNC) == 0) {
        int rtval = bufwait(bp);
        brelse(bp);
        return (rtval);

should makefs bwrite not do this as well, rather than adding the brelse here?

[imp]

In D39025#889220, @emaste wrote:

kernel bufwrite() does:

if ((oldflags & B_ASYNC) == 0) {
        int rtval = bufwait(bp);
        brelse(bp);
        return (rtval);

should makefs bwrite not do this as well, rather than adding the brelse here?

I don't think so... Well, maybe in an ideal world, but implementing all of buf(9) here
would be quite the undertaking.

There's no notion of async writes at the moment, so this would be hard to add from that perspective.
There's no notion of 'dirty' buffers at the moment. There's a conscious attempt to make everything
synchronous, even async writes (which is why #define bdwrite(bp) bwrite(bp) is there). There's
also no 'flushing' of dirty buffers in brelse (it just throws the data away).

The code tries, but I think fails, to do a brelse(bp) at the right points. I see several places where
we do the right thing. We mostly get this right because most of the places that we fail to do
the brelse are places we're writing metadata out. I think that we may leak a little data, but haven't
measured that assertion...

the code in ffs/* only ever writes metadata. all the data data is written in ffs.c, which does
the proper dance.

msdos mirrors this, somewhat, but takes fewer liberties with multiple buffers than ffs does,
which is why I can get away with my zero-copy mmap caching, and ffs needs the write-back
because there's time the code assumes two copies of the data exist and zero copy invalidates
that assumption (though I've not found where, exactly, that's needed and I'd have
thought that the getblk returning a cached bp would fix that)...

Also getblk doesn't really take a vp like it does in the kernel, so it relies on single threading to
ensure that the lblkno -> blkno mapping remains 'stable enough' for the rest of the code
to work. This isn't commented, and it's partially speculation on my part based on reading
the code (and understanding it incompletely) and trying to debug other bugs...

And yes, all of these crazy assumptions should be enshrined in comments somewhere.

[emaste]

NetBSD makefs' bwrite() calls brelse() internally and so better mimics bwrite(9); we should do the same. But right now we don't, so I'm fine with this change but we should have a comment about deviating from the kernel interface.

[emaste]

NetBSD 0a62dad69f6288b05e812c0fe293e61a37c25edd is the relevant commit

commit 0a62dad69f6288b05e812c0fe293e61a37c25edd
Author: christos <[e-mail]>
Date:   Sun Jan 27 20:05:46 2013 +0000

    This works well enough to populate plain files in the root dir. creating
    directories fails.

It moved brelse following bwrite out of ffs_write_file, ffs_alloccg, msdosfs_wfile and into bwrite.

[emaste]

So to repeat what I said on IRC, the timeline is basically:

1. NetBSD initially implemented makefs with a bwrite API that didn't quite match the kernel
2. msdosfs support was added
3. NetBSD removed the extra brelse calls (including from msdosfs) and did it directly within bwrite in the commit mentioned above
4. I brought over NetBSD's msdosfs support, but missed the API change

The underlying bug here is mine in step 4: I needed to either (a) add the brelse matching older NetBSD, or (b) update our bwrite. Your change here is (a). We should still do (b), and I can take a look at that later on, or if you want to pick it up that's fine with me.