Re: [PATCH 00/03] kexec: Avoid overwriting the current pgd (V2)

[ebiederm@xmission.com (Eric W. Biederman)]

Magnus Damm <magnus@valinux.co.jp> writes:

> Hi Eric,
>
> On Wed, 2006-05-24 at 20:56 -0600, Eric W. Biederman wrote:
>> 
>> C code is much more accessible to other programmers than arch specific
>> assembly.  The code on the control page was almost written in C, and
>> I'm still not quite convinced that it would be wrong to do that.
>
> I agree with you that it is of course better to implement something in C
> if possible compared to writing it in architecture-specific assembly.
>
> But I do not agree that wrapping architecture-specific assembly code in
> C functions makes the code more understandable. I'd really like to meet
> the kernel hacker that is aware of how x86 segmentation works but is
> unable to read x86 assembly.

For some young programmers it may be a matter of reading ability.
For older programmers it is more likely to be a matter of reading
speed. 

Regardless that is how the code is now, and how it came out of the series
of code reviews I had to go through when I wrote it.  I had requests
to do more in C and I never had a request to do more in assembly.
Proving there was no sane way to write the control code page in
C was actually difficult.

If there is a legitimate reason to change the code that is fine.  But
as it looked as simply a change without a good reason that is not
fine.

The big problem was you did several things with a single patch,
and that made the review much more difficult than it had to be.

Having to check if you correctly modified the page tables, while also
having to check for segmentation, and the interrupt descriptor
transformations was distracting.

>> > - I'm currently working on making kexec to work under xen/dom0. And by
>> > moving the segment handling code into the assembly file we reduce the
>> > amount of duplicated code.
>> 
>> Not the reason I would have expected.  So you are only differring the
>> two implementations by the contents of the control code page?
>
> Nah, there's a fairly large framework to pass pages to the hypervisor,
> converting pfn:s to mfn:s, building page tables etc. We will resend the
> patches later on today to xen-devel if you're interested.

Ok.  I might have to look.

Eric