Re: [PATCH] x86: fault_{32|64}.c unify do_page_fault

[Harvey Harrison]

On Thu, 2008-01-03 at 04:45 +0300, Alexey Dobriyan wrote:
> On Wed, Jan 02, 2008 at 05:01:02PM -0800, Harvey Harrison wrote:
> > Begin to unify do_page_fault(), easy code movement first.
> > 
> > Signed-off-by: Harvey Harrison <harvey.harrison@xxxxxxxxx>
> > ---
> > Ingo, similar to the kprobes unification patches I did, it gets a bit
> > uglier before it gets better ;-)
> > 
> >  arch/x86/mm/fault_32.c |   38 +++++++++++++++++++++++++++++---------
> >  arch/x86/mm/fault_64.c |   23 ++++++++++++++++++-----
> >  2 files changed, 47 insertions(+), 14 deletions(-)
> > 
> > diff --git a/arch/x86/mm/fault_32.c b/arch/x86/mm/fault_32.c
> > index b1893eb..051a4ec 100644
> > --- a/arch/x86/mm/fault_32.c
> > +++ b/arch/x86/mm/fault_32.c
> > @@ -375,19 +375,26 @@ void __kprobes do_page_fault(struct pt_regs *regs, unsigned long error_code)
> >  	struct mm_struct *mm;
> >  	struct vm_area_struct *vma;
> >  	unsigned long address;
> > -	int write, si_code;
> > -	int fault;
> > +	int write, fault;
> > +#ifdef CONFIG_x86_64
> 
> There is no such thing as CONFIG_x86_64 .

My apologies, testing/compiling on X86_32 here.

> 
> Do you seriously think code is getting better and more readable because
> of this liberal #ifdef sprinkling in every possible direction?
> 

Well, this of course is not the end of the road, but it makes it
obvious where the differences between 32/64 bit lie and allows
further cleanups to unify these areas over time.  This is meant as
a no functionality change path at first.....and it does point out that
for the most part the files are _very_ similar to each other.

So my plan for now was to move forward with no functional changes and
esentially ifdef or reorder code until we get to identical fault_32/64.c
which then gets moved to a single fault.c

Then the cleanups happen in one place in one file and it should be easy
to audit the series at the end.  But for further patches I'll wait until
the series is further along and tested before submitting.  This was how
the kprobes unification went and I think it works fairly well this way.

But, if the end result is too ugly, it won't bother me at all if it
doesn't go in.

Harvey

--
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/