On 8/8/07, Gilles Chanteperdrix <[EMAIL PROTECTED]> wrote:
> Jan Kiszka wrote:
> > Jan Kiszka wrote:
> > > Philippe Gerum wrote:
> > >> On Tue, 2007-08-07 at 17:09 +0200, Gilles Chanteperdrix wrote:
> > >>> On 8/7/07, Jan Kiszka <[EMAIL PROTECTED]> wrote:
> > >>>> Gilles Chanteperdrix wrote:
> > >>>>> The fact that you are in a hurry should not be an excuse to propose a
> > >>>>> fix which is much worse than the bug itself.
> > >>>> Please explain.
> > >>> Using GFP_ATOMIC is not a good idea, because the kernel needs the
> > >>> GFP_ATOMIC pools for allocation that take place from really atomic
> > >>> contexts such as interruption handlers for instance. fork should not
> > >>> be an atomic context.
> > >>>
> > >>> So the only reason I see why you propose this patch is because you are
> > >>> in a hurry. Ok, before we fix this properly, you can use GFP_ATOMIC
> > >>> locally, but please do not make
> > >>> this an official patch.
> > >>>
> > >> I'm afraid you are both right. Draining pages from the atomic pool might
> > >> starve truely atomic contexts, and releasing the lock guarding the
> > >> pagetable pages for the source mm seems contradictory with what the
> > >> current code tries to achieve in holding it.
> > >>
> > >> Here is a patch preallocating the page for the cow-breaking code, which
> > >> relies on the lock breaking pattern already in place. Not exactly
> > >> pretty, slightly tested here (UP, spinlock debug), but looks ok so far.
> > >> Needs SMP testing before any attempt is made to merge it.
> > >
> > > Funny. I just finished my own patch of this kind and made it pass a
> > > basic COW stress test here. Find it below, it is very similar. Will
> > > now analyse yours to see which one looks nicer and/or is more correct.
> >
> > Yours is clearly nicer, but it required/allowed a few more cleanups.
> > Find my variant attached (against our local 2.6.20 kernel).
> >
> > From my POV: problem solved. 8)
>
> It seems you solved this problem before I even had a chance to have
> a look at the code.
Finally looking at the I-pipe patch, I see something suspicious:
the patch replaces a "return err" with a "return 0" in zeromap_pud_range.
--
Gilles Chanteperdrix
_______________________________________________
Adeos-main mailing list
[email protected]
https://mail.gna.org/listinfo/adeos-main
