Date: Mon, 14 Apr 2008 21:21:20 +0400 (MSD) From: Chagin Dmitry <chagin.dmitry@gmail.com> To: Kostik Belousov <kostikbel@gmail.com> Cc: freebsd-emulation@freebsd.org, Chagin Dmitry <chagin.dmitry@gmail.com> Subject: Re: Call for review && test: linux_kdump-1.6 Message-ID: <20080414205317.E6842@ora.chd.net> In-Reply-To: <20080414163545.GJ18958@deviant.kiev.zoral.com.ua> References: <20080412145401.GA4139@freebsd.org> <20080413214624.S7426@ora.chd.net> <20080413183248.GA68642@freebsd.org> <20080413183659.GA18958@deviant.kiev.zoral.com.ua> <20080413231135.K1079@ora.chd.net> <20080413192614.GC18958@deviant.kiev.zoral.com.ua> <20080413234359.H1165@ora.chd.net> <20080414123137.GH18958@deviant.kiev.zoral.com.ua> <20080414134935.GI18958@deviant.kiev.zoral.com.ua> <20080414194041.Q6842@ora.chd.net> <20080414163545.GJ18958@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 14 Apr 2008, Kostik Belousov wrote: > On Mon, Apr 14, 2008 at 08:18:22PM +0400, Chagin Dmitry wrote: >> On Mon, 14 Apr 2008, Kostik Belousov wrote: >> >>> On Mon, Apr 14, 2008 at 03:31:37PM +0300, Kostik Belousov wrote: >>>> >>>> Ah, I see. Then, we shall never dump the ERESTART and EJUSTRETURN >>>> for the emulated ABIs. At least, this is true for Linux, I am not >>>> so sure about iBCS2 and SVR4. >>>> >> >> I do not absolutely agree with this statement. If the emulated syscalls >> should return native FreeBSD errno, that why to not write them to a >> ktrace file without conversion? Current linux_kdump port uses strerror >> because expects it. At least, it is convenient :) > Again, could you, please, elaborate ? ABI emulation shall return the > translated errors. And, the current behaviour is to dump translated > error codes, so linux_kdump must cope with it already. > ABI emulation shall return the translated errors to user-space, but not to kernel-space where dump written. But i have understood all, thanks! I simply thought, that while linux_kdump unique easier to change rules for all following. >> >> Your patch is correct for my version of linux_kdump, but does not solve >> a problem with the current port version. > I think we could commit the trap.c patch simultaneously with > the new linux_kdump. Even better, two versions of the linux_kdump > could coexist in the ports, with the right one being selected based > on the __FreeBSD_version. But I would leave this to the emulation@. > ok >> >> If it's possible, explain please, that is not correct in my last patch? > Your previous patch (cited above) prevents the dumping of the > EJUSTRETURN for the native FreeBSD syscalls, that is also wrong, IMHO. > > Assuming that your last patch is the one that moved the ktrsysret() > before the switch (error), I see two problems: > 1. It dumps the error before the ABI compat has translated the error. > This is definitely huge deviation with the present behaviour, see > above. > 2. It missed the amd64 trap.c. > #1 is corrected in my version. #2 is a trivial overlook. > >> >> thnx >> >>>> Could you test the patch below, instead ? >>> >>>> diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c >>>> index e7de579..55642d1 100644 >>>> --- a/sys/i386/i386/trap.c >>>> +++ b/sys/i386/i386/trap.c >>> >>> The patch is obviously wrong, it just prevents the Linux ENOENT to be >>> dumped. Please, try this one instead. > ^^^^^^^^This statement is about _my_ first patch. > >>> >>> diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c >>> index b6a454d..3b1368a 100644 >>> --- a/sys/amd64/amd64/trap.c >>> +++ b/sys/amd64/amd64/trap.c >>> @@ -861,9 +861,18 @@ syscall(struct trapframe *frame) >>> frame->tf_rip -= frame->tf_err; >>> frame->tf_r10 = frame->tf_rcx; >>> td->td_pcb->pcb_flags |= PCB_FULLCTX; >>> - break; >>> - >>> + /* FALLTHROUGH */ >>> case EJUSTRETURN: >>> +#ifdef KTRACE >>> + /* >>> + * The ABIs that use the negative error codes, like >>> + * Linux, would confuse the in-kernel errno values >>> + * with proper userspace errno. Clean these values to >>> + * avoid a confusion in the kdump. >>> + */ >>> + if (p->p_sysent->sv_errsize) >>> + error = 0; >>> +#endif >>> break; >>> >>> default: >>> diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c >>> index e7de579..6ec04b0 100644 >>> --- a/sys/i386/i386/trap.c >>> +++ b/sys/i386/i386/trap.c >>> @@ -1040,9 +1040,18 @@ syscall(struct trapframe *frame) >>> * int 0x80 is 2 bytes. We saved this in tf_err. >>> */ >>> frame->tf_eip -= frame->tf_err; >>> - break; >>> - >>> + /* FALLTHROUGH */ >>> case EJUSTRETURN: >>> +#ifdef KTRACE >>> + /* >>> + * The ABIs that use the negative error codes, like >>> + * Linux, would confuse the in-kernel errno values >>> + * with proper userspace errno. Clean these values to >>> + * avoid a confusion in the kdump. >>> + */ >>> + if (p->p_sysent->sv_errsize) >>> + error = 0; >>> +#endif >>> break; >>> >>> default: >>> >> >> -- >> Have fun! >> chd > -- Have fun! chd
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080414205317.E6842>