Re: [PATCH 23/41] dm: implement REQ_FLUSH/FUA support for bio-baseddm

From: Mikulas Patocka
Date: Fri Sep 10 2010 - 16:06:59 EST




On Fri, 10 Sep 2010, Mike Snitzer wrote:

> On Fri, Sep 10 2010 at 3:05pm -0400,
> Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
>
> >
> >
> > On Fri, 10 Sep 2010, Mike Snitzer wrote:
> > > And in fact, this first patch basically is as minimal as it gets
> > > relative to bio-based DM's conversion to FLUSH+FUA.
> > >
> > > Please direct your energy and talent in a positive way rather than
> > > starting a potential flame.
> > >
> > > Thanks,
> > > Mike
> >
> > I don't want to flame. I mean this:
> >
> > * person X writes a patch P.
> > * person Y reads P, sees that the condition C is true and writes patch Q
> > that dependes on condition C.
> > * person X changes a patch P, so that the patch is correct but condition C
> > is no longer true.
> >
> > Now, there is a bug in the patch Q and NEITHER X NOR Y can find out about
> > that bug.
> >
> > That's why parallel development doesn't work.
> >
> > If you develop on things in the kernel, it is different.
> > * person X writes a patch P and puts it in the kernel.
> > * person Y reads the kernel code, sees that the condition C is true and
> > writes a patch Q that assumes that the condition C is true. He puts this
> > patch to the kernel too.
> > * person X wants to change his code so that the condition C isn't true,
> > but it is now his responsibility to search the rest of the kernel to see
> > if it depends on the condition C. He searches the code and finds Q.
> >
> > This is not a flamewar, just a technical explanation, why I don't want to
> > develop on interfaces that are not in the kernel.
>
> We're reasonable people and can certainly prevent a flamewar but what
> you're doing is an elaborate distraction. The energy it took you to
> write and reason through your logic above could've been used to just
> review the DM FLUSH+FUA patches.

No. If I reviewed 40 patches perfectly, I would take long long time (the
previous 2-line patch that I reviewed took me a week to review --- but I
found a flaw that the other people who reviewed it quickly didn't find).

So I reviewed only "dm" patch and found out that it is too big.

Make a smaller patch with barrier -> FLUSH logic only. And then you can
make additional patches with function/variable renames or logic changes.

> The various interfaces are hardened now and staged for inclussion in
> 2.6.37. Jens has already pulled the entire 40+ patchset into his
> for-next branch for wider linux-next testing.
>
> Tejun, Christoph and others have done an amazing job with this
> conversion. The fact that Tejun tackled the heavy lifting of DM's
> conversion was unexpected but 100% appreciated (by me and I assume
> others). Please don't dismiss, or misrepresent the status of, this
> FLUSH+FUA work.

I am not dismissing anything. I agree with barrier -> flush change. It
simplifies things a lot.

But I have my work rules that I learned: I use no git kernels and no
external patches (except Alasdair's patchset that I want to test). I only
use -rc or final kernels. I need a stable computer --- I don't want to
solve problems like "does it crash because I pulled something or does it
crash because I made a bug in my code?" So, put that into 2.6.37-rc1 and
I'll optimize flushes in dm for -rc2 or -rc3.

Mikulas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/