Re: [PATCH] remove double indirection on tlb parameter to free_pgd_range()& Co

[Hugh Dickins]

On Thu, 15 May 2008, Jan Beulich wrote:
> The double indirection here is not needed anywhere and hence (at least)
> confusing.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>

Please hold off on this.  I was writing a mail to say that it should be
okay (might as well include what I wrote then below), then checked and
noticed that Andrea/Christoph have an mmu_notifier patch which removes
that argument to free_pgtables (it could alternatively keep the argument,
but make use of the double indirection).  It's not yet clear whether
or when their patch goes in, but let's not obstruct their functional
enhancements with this cleanup.

What I originally wrote was...
I had to look up my original comment on it:

    Pass mmu_gather** in the
    public interfaces, since we might want to add latency lockdrops later;
    but no attempt to do so yet, going by vma should itself reduce latency.

By "latency lockdrops" I meant the tlb_finish_mmu/tlb_gather_mmu pair
you find in unmap_vmas: I expected free_pgtables to need the same soon.

free_pgtables does have a latency issue, but not so bad that we've rushed
in to reduce it, at the expense of increasing TLB flushes, and ugliness.

We'd several of us like to rework the mmu_gathering so as not to disable
preemption: I got stuck and BenH took over, a patch much like yours below
was a part of what I had - I too was glad to get rid of the **s.

So, assuming we won't suddenly need a quick hack fix to free_pgtables
latency before one of us fixes the wider mmu_gathering issues, your
patch should be fine (and could be reverted if a sudden need occurs).

But... could I ask you to omit that move from include/linux/mm.h to
mm/internal.h?  To me that's nothing but an irritation, and internal.h
is the last place I think of to look for anything.  There's a thousand
other things internal to mm which aren't in it, and personally I wouldn't
even be thrilled by a janitorial project to move stuff over to it in bulk.

But, returning to the start, please let's hold yours back after all.

Thanks,
Hugh
--
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/