On 22.02.2008, at 0:47, Kostik Belousov wrote: > On Thu, Feb 21, 2008 at 09:26:16AM +0900, Alexander Nedotsukov wrote: >> Hi, >> >> May I ask to revisit this issue? To me ENXIO is not semantically >> correct in this particular case. It also turns out that doing >> workaround in userspace may not be that good as we used to think. I >> propose is to fix VT_WAITACTIVE so it simply wait for bound device >> activation. For my understanding this change should not have any >> impact on existing code. I also removed really strange console >> cleanup >> bit sticked in a long time ago (see ioctl() part). >> It will be nice to see this patch > > >> (successfully tested by our affected users) committed to all >> branches. >> >> Thanks, >> Alexander. > > I mostly agree with the patch, given it is tested. > > The first (not important) note is that change of the wait channel from > the address of the private structure to the address of the cdev could > cause more spurious wakeups then before. I expect you usermode code to > deal with it properly. Do you know any potential wakeup()s around the kernel? I did not see any spurious wakeups myself nor user reported it so far. However they are not welcome so if it considered to be unsafe we can use address of cdev pointer (&SC_DEV()) which is private to syscons. > > > Second one is that I do not understand the reason to have > sc_clean_up() > there except that it might be to help the screen switching. The commit > message for the rev. 1.307, where it seems to be introduced, is not > useful at all. Any comments ? In fact it was introduced in rev. 1.293 for the first time. According to the commit log it was a part of screensaver interaction improvement. I doubt that it was actually required for VT_WAITACTIVE case. I CCed yokota_at_ to shed the light on it. > >> >> On 24.01.2008, at 21:42, Kostik Belousov wrote: >> >>> On Wed, Jan 23, 2008 at 04:32:13PM -0500, Joe Marcus Clarke wrote: >>>> >>>> On Wed, 2008-01-23 at 23:11 +0200, Kostik Belousov wrote: >>>>> On Wed, Jan 23, 2008 at 02:55:59PM -0500, Joe Marcus Clarke wrote: >>>>>> >>>>>> On Wed, 2008-01-23 at 20:34 +0100, Pawel Worach wrote: >>>>>>> Kostik Belousov wrote: >>>>>>>> On Tue, Jan 22, 2008 at 07:26:53PM +0100, Pawel Worach wrote: >>>>>>>>> Kostik Belousov wrote: >>>>>>>>>> On Sun, Jan 20, 2008 at 04:42:36AM +0100, Pawel Worach wrote: >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> While starting console-kit-daemon (sysutils/consolekit >>>>>>>>>>> 0.2.3) during >>>>>>>>>>> boot or in single-user mode the system panics. If I start it >>>>>>>>>>> post-boot >>>>>>>>>>> it runs fine. This is on 8.0-CURRENT from about 12 hours >>>>>>>>>>> ago, another >>>>>>>>>>> user also reported the same on RELENG_7. Any other >>>>>>>>>>> information I can >>>>>>>>>>> provide ? >>>>>>>>>>> >>>>>>>>>>> Fatal trap 12: page fault while in kernel mode >>>>>>>>>>> fault virtual address = 0x4 >>>>>>>>>>> fault code = supervisor read, page not present >>>>>>>>>>> instruction pointer = 0x20:0xc04d2ab4 >>>>>>>>>>> stack pointer = 0x28:0xe6499b18 >>>>>>>>>>> frame pointer = 0x28:0xe6499b80 >>>>>>>>>>> code segment = base 0x0, limit 0xfffff, type 0x1b >>>>>>>>>>> = DPL 0, pres 1, def32 1, gran 1 >>>>>>>>>>> processor eflags = interrupt enabled, resume, IOPL = 0 >>>>>>>>>>> current process = 134 (console-kit-daemon) >>>>>>>>>>> Physical memory: 1014 MB >>>>>>>>>>> Dumping 43 MB: 28 12 >>>>>>>>>>> >>>>>>>>>>> #8 0xc07a34a2 in trap (frame=0xe6499ad8) at >>>>>>>>>>> /usr/src/sys/i386/i386/trap.c:489 >>>>>>>>>>> #9 0xc079183b in calltrap () at /usr/src/sys/i386/i386/ >>>>>>>>>>> exception.s:146 >>>>>>>>>>> #10 0xc04d2ab4 in scioctl (dev=0xc3b20d00, cmd=537163270, >>>>>>>>>>> data=0xe6499c70 "\002", flag=1, td=0xc3d3c880) >>>>>>>>>>> at /usr/src/sys/dev/syscons/syscons.c:1073 >>>>>>>>>>> #11 0xc051ed1a in giant_ioctl (dev=0xc3b20d00, >>>>>>>>>>> cmd=537163270, >>>>>>>>>>> data=0xe6499c70 "\002", fflag=1, td=0xc3d3c880) >>>>>>>>>>> at /usr/src/sys/kern/kern_conf.c:349 >>>>>>>>>>> #12 0xc0598194 in cnioctl (dev=0xc3b20d00, cmd=537163270, >>>>>>>>>>> data=0xe6499c70 "\002", flag=1, td=0xc3d3c880) >>>>>>>>>>> ---Type <return> to continue, or q <return> to quit--- >>>>>>>>>>> at /usr/src/sys/kern/tty_cons.c:521 >>>>>>>>>> Unless the virtual screen is opened, the screen state >>>>>>>>>> scr_stat structure >>>>>>>>>> is not allocated. The following patch would fix the panic, >>>>>>>>>> but I do not >>>>>>>>>> know how the console-kit would react to the ENXIO from the >>>>>>>>>> VT_WAITACTIVE ioctl. Please, test it. >>>>>>>>> Thanks! The patch works. >>>>>>>> >>>>>>>> To clarify: do you see any problems with the console-kit after >>>>>>>> the patch ? >>>>>>>> In particular, can you verify that the program functions >>>>>>>> correctly, esp. >>>>>>>> on the virtual terminals 1, 2 ... , whatever this means ? >>>>>>> >>>>>>> The panic is of course gone, while chatting a bit with Marcus >>>>>>> (CCd) it >>>>>>> looks like console-kit does not do any error handling at all. >>>>>>> I've not >>>>>>> looked at what c-k does so maybe Marcus can answer the question >>>>>>> better. >>>>>> >>>>>> It tries to install a wait thread on each available VT. That >>>>>> thread >>>>>> sets the WAITACTIVE ioctl, and waits for its VT to become >>>>>> active. When >>>>>> it does, it sets the CK active VT accordingly, and reattaches the >>>>>> wait. >>>>>> >>>>>> When an error occurs in the ioctl, no wait is attached, and CK >>>>>> will not >>>>>> know when a particular VT becomes active. This will essentially >>>>>> cripple >>>>>> CK (assuming the VT really does become available at a later >>>>>> point). >>>>>> >>>>>> Now, admittedly, there is no error correction in CK for this >>>>>> situation. >>>>>> It would be trivial to add something that periodically attempts >>>>>> to >>>>>> reestablish a failed wait. However, I am very curious why only a >>>>>> few >>>>>> users are seeing this panic (me excluded on two different >>>>>> machines). It >>>>>> seems to me that the scp should be initialized when >>>>>> sc_attach_unit() is >>>>>> called during device probing. I don't see anything special in >>>>>> init(8)'s >>>>>> code that would cause these VT devices to become initialized. I >>>>>> would >>>>>> assume that if one can open(2) them, then they should be >>>>>> available for >>>>>> ioctls? >>>>> >>>>> From my reading of the code, scp would be non-NULL after the first >>>>> open >>>>> of the corresponding /dev/ttyvX. sc_attach_unit() creates the scp >>>>> for >>>>> the console and the consolectl devices. >>>>> >>>>> VT_WAITACTIVE ioctl is performed on arbitrary syscons /dev node, >>>>> and >>>>> can wait for any other screens, in particular, the screens that >>>>> are >>>>> not opened at the moment (the reason for the reported panic). >>>> >>>> Right, and this is where I was confused. I had thought from an old >>>> reading of the CK code that CK opened each ttyvX device to perform >>>> the >>>> ioctl. It does not. Instead, it opens /dev/console, and performs >>>> the >>>> ioctl for each ttyvX on that fd. That does explain the panic, but >>>> not >>>> exactly why I did not see it. I'm guessing a race condition, but I >>>> can't be sure. >>>> >>>>> >>>>> The patch I posted may be improved by making the VT_WAITACTIVE >>>>> ioctl >>>>> to wait for the scp being allocated, and only then for the screen >>>>> to be >>>>> switched too. Please, test. >>>> >>>> I really appreciate your attention to this. Funny thing is, CK >>>> 0.2.4 >>>> was just released, and it is no longer started out of rc.d. I've >>>> also >>>> added error correcting in the CK code path. The problem may >>>> disappear >>>> depending on when CK is executed out of D-BUS. However, it would >>>> be >>>> good to prevent this in the kernel. Pawel said he would try and >>>> test >>>> this patch. >>> >>> This must be fixed in the kernel, at least because it allows any >>> console >>> user to panic the system. The question I have to you is what >>> approach shall >>> we use: >>> - return the ENXIO (1) >>> - do the wait for the open, then wait for the switch (2) >>> >>> I would prefer (1), both due to putting the decision to the >>> userspace, and >>> to not have the algorithm that could be implemented in userspace, in >>> the >>> kernel. On the other hand, as the maintainer of the one of the major >>> API >>> consumer there, you may have the different opinion, that is crusial. >> >Received on Fri Feb 22 2008 - 15:00:04 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:27 UTC