Magnus Damm wrote: > On 3/28/07, Vivek Goyal <[EMAIL PROTECTED]> wrote: >> On Wed, Mar 28, 2007 at 03:55:24PM +0900, Magnus Damm wrote: >>> This means that we will end up with many architecture-specific >>> differences that may not be needed but they are there because no one >>> really had enough knowledge or time to see the big picture. >>> >>> So, I think we should solve this by a simple and well documented fix >>> now. And long-term we can try to share the /proc/kcore code between >>> ia64 and x86_64. >>> >> Probably we can share that code later, but atleast for the time being >> we should either replicate that kcore reading code or right now perform >> the step of calculating the virtual address by adding physical address >> in arch dependent section. > > Yes, putting things in the arch-dependent code sounds like the best > choice. I'm a bit reluctant to duplicate the kcore code - that is more > of a feature than a fix IMO. We want to be at least as good as the > last kexec-tools release and currently it looks like we are > unsuccessful with that mission on ia64. > >>>> I think in arch dependent code you should set >>>> kern_vaddr=LOAD_OFFSET + kernel_physical start. >>> Right. Patches are welcome. =) >>> >> I don't understand that what's the point of giving the review comments >> on a particular patch if a person is not willing to change the patch. So >> only way to give review comments is that one should also send a patch >> otherwise don't talk about it. :-( > > First of all, I really appreciate your comments Vivek. Secondly, the > patch was just my stab at fixing up the original patch written by > Jonathan. I see this as an iterative process so if anyone has any > better patch/idea then I'd be happy to go with that instead. And I may > of course come up with a better alternative myself - thanks to your > comments - but I'm unfortunately a bit low on time. And that's the > reason why I asked for patches. Sorry if that sounded like I wasn't > willing to change the patch. > >> It should not be that big a patch that we should keep it a future item >> and right now put a patch which is not intitutive at all. > > Right. Let's come up with a small arch-specific patch that we all feel > good about. =) > >> For kexec-tools-testing there are no hard deadlines for any release. If >> we delay it by a day or two just becuase we can put a better patch which >> everybody can understand down the line, I will definitely go by that. But >> the argument, I understand that we are not doing the right thing but >> we will still do it and if somebody has objections, he should provide a >> patch, I am not sure I understand that. > > You are right that there are no hard deadlines for the > kexec-tools-testing release. The only limitation for me is that I have > my last working day in the office on Friday and I sort of wanted to > have the release done by then. > > I think we can have both when it comes to long-term and short-term. > Duplicating the /proc/kcore code is wrong IMO, if we should add > /proc/kcore support for IA64 then it should be done correctly - ie the > same code base should be used for x86_64. This will unfortunately take > longer time than a few days so someone else than me will have to work > on that. I propose that we postpone the /proc/kcore change and go with > a simple IA64-specific for now. > > But OTOH, ia64 kdump seems to be broken at the moment. So adding > /proc/kcore support will not make things any worse. Except we have > more lines of duplicated code... > >> Why I am pressing so hard for this change is that I am more concerned >> about that at least logic of something should be clear. By looking at >> the patch, one can't make any guesses about what kern_offset field >> is and why it is required. > > I agree that it's important to keep sane logic and by that I realize > that my patch may be a step in the wrong direction. I saw it as a > simple fix to unbreak ia64 and get the release out of the door. The > best solution is probably to keep on using kern_vaddr_start and > perform the offset calculation in the crashdump-ia64.c. > > Jonathan, if I repost a better patch, do you have time to test?
We will test it. We have fixed a number of IA64 kdump bugs since 2.6.20. While we agree we need a better long term solution, we should fix any bug we find in the short term. This will help testing and uncovering other problems, if any, in the git tree. Thanks, - jay > > Thanks! > > / magnus > _______________________________________________ > fastboot mailing list > [email protected] > https://lists.linux-foundation.org/mailman/listinfo/fastboot > _______________________________________________ fastboot mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/fastboot
