Re: [Xen-devel] Merging xen/dom0/backend/blktap2 ..

[Daniel Stodden <daniel.stodden@xxxxxxxxxx>]

On Tue, 2011-03-22 at 12:59 -0400, Konrad Rzeszutek Wilk wrote:

> > >
> > > The gntdev solved this by clearing the PTE on unmap (using the gnttab API 
> > > and
> > > the M2P/P2M override which clears the PTE).
> > >
> > > Would that work?
> >
> > Well, gntdev gets a normal munmap() from userland when it's time to tear
> > down the VMA. That always works, it's how it's meant to be. It's more of
> > a challenge for funny PTE types like foreign memory, but I guess that's
> > solved now. The latter is not even the case here.
> 
> Would it make more sense to have blktap in QEMU? I mean gntdev takes care
> of this, and it looks as if most of the manipulation is already done in 
> userspace?

Not at this point, and not in the foreseeable future. But it would make
sense to have blktap running guest I/O against gntdev, yes.

I used to have a prototype for the datapath, but there were quite some
gntdev changes and those must have bitrotted quite terribly since the
beginning of the year. I hope to get to look into it again in Q2.

> > > > If it hopelessly got in your way, one could back it out, the loss is
> > > > TAP=m.
> > > >
> > > > > >
> > > > > > > >
> > > > > > > > Does this sound acceptable?
> > > > > > >
> > > > > > > I looked briefly at blktap/next-2.6.38 and it looks like an 
> > > > > > > ongoing
> > > > > > > merge tree. Can you make a tree that is based off 2.6.38 (or some
> > > > > > > branch of my 'stable/' ones). Basically trying to have something 
> > > > > > > that is
> > > > > > > easy to merge and is self-containted within one branch.
> > > > > >
> > > > > > It is rooted on 2.6.32 and incrementally moves on up to currently
> > > > > > v2.6.38, no stray diffs except the forward porting where needed. 
> > > > > > There's
> > > > > > at least one conflict resolution in Kconfig around 36 or 37, iirc. 
> > > > > > Any
> > > > > > later kernel will look more or less the same.
> > > > > >
> > > > > > $ git list xenbits/blktap/next-2.6.38 ^v2.6.38
> > > > > > eb8040c Merge commit 'v2.6.38' into blktap/next-2.6.38
> > > > > > bf926e6 blktap: Forward port to 2.6.37
> > > > > > 9f65e90 Merge commit 'v2.6.37' into blktap/next-2.6.37
> > > > > > 7ef4e35 blktap: Forward port to 2.6.36
> > > > > > a55f064 Merge commit 'v2.6.36' into blktap/next-2.6.36
> > > > > > 994a546 blktap: Add discard queue limits.
> > > > > > ea2d7e7 Merge blktap into blktap/next-2.6.33
> > > > > > e37d737 blktap: Add BLKTAP_OP_TRIM command option.
> > > > > > 42276df blktap: Add BLKTAP_OP_FLUSH command option.
> > > > > > 5d10876 blktap: Add physical sector device info.
> > > > > > 8169b25 blktap: Drop the ring message timestamp.
> > > > > > 34574a8 blktap: Support non-R/W requests
> > > > > > a765257 blktap: Fix reference to freed struct request.
> > > > > > 7ccd87f blktap: Blktap userspace devices.
> > > > > >
> > > > > > You could rebase it on v2.6.38 if you want to reproduce the merging 
> > > > > > and
> > > > > > mangle commits, although I'm not sure why. For the foreseeable 
> > > > > > future,
> > > > > > I'd like to keep 2.6.32 and anything later going, so that repo will 
> > > > > > get
> > > > > > incoming stuff at 2.6.32 and, where possible, I'd expect the greater
> > > > >
> > > > > Ah, ambitious. I tried that with xen-pcifront and had to resort
> > > > > to making trees for different versions. But this could work for you.
> > > >
> > > > But it should work fine here. There's only a single source tree, which
> > > > is linux-2.6.git, and only a single component, which aims to yield the
> > > > same extensions where possible. Doesn't get much easier than that.
> > > >
> > > > I'd agree that components interfacing with 4 moving xen subsystems, as
> > > > in blkback etc. is a bit of a different matter.
> > > >
> > > > Still, killing history altogether can't be the answer.
> > >
> > > Seems to be the answer nowadays. David Miller asked Ian to squash the full
> > > history in one commit, and provide a "historic" git tree where all of the
> > > git commits reside.
> >
> > For an upstream intro, sure. I'd probably ask for the same if I were the
> > maintainer. No surprise.
> >
> > >From there on, even merging branches is probably fine again, provided
> > individual commits don't relate and there's no fix-this/forgot-that
> > stuff documenting how people got there. Nobody needs that in mainline.
> >
> > > >
> > > > > > kernels to typically just fast-forward from where they are now.
> > > > >
> > > > > The issue is git bisection. The dates on those commits is quite early 
> > > > > so
> > > > > I wonder if somebody did a git bisect whether they would hit those and
> > > > > end up with compilation issues. Perhaps the first patch should
> > > > > just introduce the driver, but not touch the Kconfig at all.
> > > > >
> > > > > And then the last one actually enables the Kconfig. And as you
> > > > > add updates, you move the patch with Kconfig to be the last one?
> > > > >
> > > > > >
> > > > > > If you want topic branches: I'm not planning to do those. Mainly 
> > > > > > because
> > > > > > it's a bad idea. If that entire thing would be about to go upstream 
> > > > > > with
> > > > > > limited time to land that might a different thing, but it's clearly 
> > > > > > not
> > > > > > there in its present state.
> > > > >
> > > > > What is a "bad" about it? What do you want to do when the driver is 
> > > > > ready
> > > > > for upstream? You would have to do this topic branch at some point, 
> > > > > wouldn't you?
> > > > > What is a topic branch to you? When I think topic branch I think:
> > > > > devel/xen-pciback-0.5 for example.
> > > >
> > > > When (iff) that driver gets ready for upstream I guess it'd have to be
> > > > folded down and optionally split, pretty much exactly as you describe.
> > > >
> > > > This is my third attempt to write a proper response to your concerns,
> > > > because so far I'm not super-experienced with git either.
> > > >
> > > > I was going to disagree because keeping separate queues is a bit of a
> > > > workflow killer.
> > > >
> > > > Then I was going to agree because commit atomicity for proper bisection
> > > > is a actually pretty good point, imho.
> > > >
> > > > The thing I hate about rebasing/cherry-picking is not even the extra
> > > > work, it's that the dupes mean one loses any kind of traction wrt what's
> > > > in a particular derived tree and not. So in order to be able to compare
> > > > two such branches one has to diff sources, git-blame, and understanding
> > > > a whole lot too much about the content.
> > >
> > > Or:
> > > git shortlog devel/xen-pciback-0.3..devel/xen-pciback-0.5  
> > > --grep="pciback"
> > >
> > > Which gives you some good idea of what was added... that is if one
> > > does not mess with the git commits and amend them (which I sometimes too
> > > since they are still "devel").
> >
> > I'm assuming pciback-0.5 supersedes/deprecates pciback-0.3?
> 
> Yes.

Well, that probably works okay, but it's not the kind of thing I'm
after. I'm trying to maintain a couple branches crossing kernel
releases. Merging is flawed, but it's all in git which tracks commits
across branches.

> > > > The commit atomicity issue could be solved by making sure stay in a
> > > > branch, and only get merged, not pulled through by fast-forward. That
> > > > git-merge --no-ff option looks like it was meant to ensure that.
> > > >
> > > > There's another way to make stuff atomic: Amending merges with the
> > > > following port. A git-commit --amend can do that. Took me ages to find
> > > > that out, because for reasons unclear, git-rebase -i alone (as 'fixup'
> > > > changes) can't.
> > > >
> > > > I tried whether that's better. Can't recommend it. Git appears to have a
> > > > really hard time turning such ports/merges back into diffs, and having
> > > > those diffs is quite useful. It's not impossible, but format-patch alone
> > > > can't, which gets a bit counterintuitive.
> > > >
> > > > > The reason I am asking about this is b/c of my inept git skills. Doing
> > > > > "git pull daniel/blktap" and just having it fetch patches that are 
> > > > > relevant
> > > > > to blktap makes it soooo much easier to get an idea what is happening.
> > > >
> > > > Well, it's not like it's going to pull garbage. These are all blktap
> > > > exclusively, rooted at v2.6.38. Just not ported individually, but in
> > > > piles.
> > >
> > > OK. The desciption were a bit generic: "porting to 2.6.37" .. doesn't 
> > > really
> > > tell what you had to do, or what not. I did not look in the description, 
> > > so
> > > it might have had the details.

> > Well yeah. But my concern here, again, is not upstreaming. The problem
> > is that 2.6.38 is important, but not integral. There's 2.6.32, XCP,
> > XenClient and all those trees will remain under maintenance. A merge and
> > forward port is more efficient than rebasing.
> 
> Aah, ok. So more of keeping sanity with all of those products and having the
> same type of patches in all of those trees.

Exactly. So the idea was to feed all patches into next-2.6.32 and from
there carry them on to 2.6.38. Later branches start as soon as there are
block layer changes which justify starting a new one.

If konrad/2.6.38 commits patches before I got to pick them up, I'd still
cherry-pick them back, as far as applicable, and push them up to
next-2.6.38 again. That's causing dupes on your side, but they should
merge fine. The opportunity should be rare.

The only reasonable alternative to pulling in merged branches, if merged
trees definitely don't work for you, is a for-konrad tree folding down
every merge into a single update. That's really simple to do, so if
that's preferred the question would be whether you want me to prepare
those.

Porting piles of patches individually isn't efficient nor justified.
Most people will just want a stable tree to pull from with all updates,
fix more issues, mail diffs, and move on. And upstreaming modules would
fold down even more aggressively, so the effort of porting individually
is quite moot.

> >
> > > > The reality is it looks like git can't deal with it any better that what
> > > > I got there.
> > > >
> > > > So I'd rather ask you open a konrad/blktap-2.6.38, tracking that branch,
> > > > and carefully merge that in, in a way not risking bisection.
> > >
> > > OK. Bisection is important - especially during those rc-X cycles where
> > > stuff stops working and is a tangle to figure out what went wrong.
> > >
> > > And it sounds to me that you want to defeer the idea of upstreaming
> > > for some time, and when you are comfortable with blktap2 being in great
> > > shape - then start the process.
> >
> > There's more work, and it's far beyond mmap. I'd first have to talk to
> > people about some of the implications which I can't just fix in there.
> >
> > The whole blkdev-in-userspace thing looks like an obviously good idea.
> > Like FUSE. It's been envisioned as just that. But while it works fine
> > under guests, upstreaming would implicitly promote it to carry host I/O.
> 
> What do you mean by 'host' here? The guest or dom0?

Any kernel running blktap, and wishing to run I/O not on behalf of
guests but it's own applications. That's the general use case for what
blktap suggests it does. But there dependency cycles which must be
avoided, or broken.

> > Now, in that area, XCP carries a couple extra patches to help dom0 with
> > provisioning of disk images under memory congestion. You really don't
> > want to know about those.
> 
> Sounds quite enterprise worthy type of feature.

Well, it typically depends on how you e.g. bootstrap VMs. Firing up the
VM and launching the installer is always fine. Getting yourself under
serious memory pressure, then having an RPC coming in which takes a
master image and blows it through the dom0 pagecache into a VHD is
different.

> > > When it comes to upstreaming process the fashion looks to post one
> > > nice patch that includes the driver. Infrastructure patches should be
> > > done as seperate patches. So at that point it probably won't matter
> > > at all whether you have little bits of patches, or just one big one.
> > > The maintainer (Jens Axboe) will probably just pick the big one.
> > >
> > > >
> > > > If that's okay, next question would probably be if one shouldn't try do
> > > > the same with blkback >:)
> > >
> > > For authorship I prefer to keep branches. So say I post the blbkback
> > > driver "as is" for review. People come back with reviews, or what not - I 
> > > create
> > > a new branch with a new patch with the review feedback. That way when
> > > I am finally done, I've this "historic" branch that has all the names of
> > > people who contributed to it.
> > >
> > > There is a twist to all of this. Linus himself prefers to ingest whole
> > > git branches. Other maintainers, like just one single patch.
> > >
> > > Let me ping Jens Axboe and find out what he prefers. That should
> > > give us a good idea of where to continue.
> >
> > Again, I'd be all for folding, but only when it's actually time to do
> > so. :}
> 
> Right. I think that is something we will have to address later on. As I 
> understand
> you are looking at this from "how to keep all my patches synced across all 
> those
> branches".
> 
> And then later on jump to implement blktap in QEMU. But my worry is that it 
> wont
> happen. Not because of lazines - but rather priority shift. There are going 
> to be bugs,
> request for new features, testing, stabilizing. This will all take time that 
> will
> be taken away from upstreaming "blktap-ish".
> 
> Maybe we should just drop the idea of upstreaming this? At which point I 
> shouldn't
> carry it in my devel branch (which is for patches we want to upstream) but 
> instead
> create a branch that would be used for those folks who want all patches that
> went upstream + some goodies that aren't going to be upstreamed?

We can drop the idea for a while, yes. I'm not sure if that justifies
just dropping it out of the tree. That's ultimately up to your
judgement, but the code footprint across the component branches is
already next to zero. Blktap isn't in xen/ or core stuff anymore. Export
zap_page_range being the single exception, but that residual isn't very
prone to crosstalk.

Thanks,
Daniel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel