[uml-devel] Re: Fw: [patch] uml: terminal cleanup

[uml-devel] Re: Fw: [patch] uml: terminal cleanup

[Blaisorblade]

On Sunday 28 November 2004 10:09, Andrew Morton wrote:

> Date: Wed, 24 Nov 2004 19:53:41 +0100
> From: Gerd Knorr <kraxel@bytesex.org>
> To: Andrew Morton <akpm@osdl.org>, Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>, uml devel
> <user-mode-linux-devel@lists.sourceforge.net> Subject: [patch] uml:
> terminal cleanup

>   Hi,
>
> This is a major cleanup of the uml terminal drivers and console handling
> (console as in "where the kernel messages go to", not as in "linux
> virtual terminals").  The changes in detail:


>  (2) Ditched the early-console-init hackery in stdio_console.c
>      (open_console(NULL) + related stuff) into the waste basket, not
>      needed any more as you can use the new stderr console driver to
>      get the kernel messages if your kernel crashes very early in the
>      boot process.
>
>  (3) Handle console initialitation for the uml stdio console and
>      virtual serial lines the normal way using the console->setup()
>      function.  Now all kernel messages appear on your console device
>      once it is initialized without any dirty tricks.


> Index: linux-2004-11-23/arch/um/drivers/chan_user.c
> ===================================================================
> --- linux-2004-11-23.orig/arch/um/drivers/chan_user.c 2004-11-24
> 12:50:38.560153813 +0100 +++
> linux-2004-11-23/arch/um/drivers/chan_user.c 2004-11-24 18:15:00.027153605
> +0100 @@ -110,7 +110,7 @@ static int winch_thread(void *arg)
>   }
>  }
>
> -static int winch_tramp(int fd, void *device_data, int *fd_out)
> +static int winch_tramp(int fd, struct tty_struct *tty, int *fd_out)
>  {
>   struct winch_data data;
>   unsigned long stack;
> @@ -144,7 +144,7 @@ static int winch_tramp(int fd, void *dev
>   return(pid);
>  }
>
> -void register_winch(int fd, void *device_data)
> +void register_winch(int fd, struct tty_struct *tty)
>  {
>   int pid, thread, thread_fd;
>   int count;
Hmm, I saw you have put a forward declaration of struct tty_struct, however it 
does not seem nice anyway... I don't think that using void* is nice either, 
but both things have pitfalls.

> -void line_close(struct line *lines, struct tty_struct *tty)
> +void line_close(struct tty_struct *tty, struct file * filp)
>  {
> - struct line *line;
> - int n;
> -
> - if(tty == NULL) n = 0;
> - else n = tty->index;
> - line = &lines[n];
> + struct line *line = tty->driver_data;
>
>   down(&line->sem);
>   line->count--;
> -
> - /* I don't like this, but I can't think of anything better.  What's
> -  * going on is that the tty is in the process of being closed for
> -  * the last time.  Its count hasn't been dropped yet, so it's still
> -  * at 1.  This may happen when line->count != 0 because of the initial
> -  * console open (without a tty) bumping it up to 1.
> -  */
> - if((line->tty != NULL) && (line->tty->count == 1))
> -  line->tty = NULL;
> - if(line->count == 0)
> -  line_disable(line, -1);
> + if (tty->count == 1) {
> +  line_disable(tty, -1);
> +  tty->driver_data = NULL;
> + }
>   up(&line->sem);
>  }
Why did you delete this comment? I don't think you fixed the problem, you just 
worked it around the other way...

Or better, you removed the initial console open, if it refers to the item #2 
above... but the code isn't clean...

-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade





-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[uml-devel] Re: Fw: [patch] uml: terminal cleanup

[Gerd Knorr]

> > -void register_winch(int fd, void *device_data)
> > +void register_winch(int fd, struct tty_struct *tty)
> >  {
> >   int pid, thread, thread_fd;
> >   int count;
> Hmm, I saw you have put a forward declaration of struct tty_struct, however it 
> does not seem nice anyway... I don't think that using void* is nice either, 
> but both things have pitfalls.

It's enougth to just have "struct tty_struct;" declared, at least for
code paths which don't access the elements of the the struct and just
pass it through (like the usermode code does in that case).  That
shouldn't create much trouble as you don't need the full tty_struct
declaration for the userspace code, but the compiler can still do type
checking on the arguments (thats why I didn't want it being void*).

> > -void line_close(struct line *lines, struct tty_struct *tty)
> > +void line_close(struct tty_struct *tty, struct file * filp)
> >  {
> > - struct line *line;
> > - int n;
> > -
> > - if(tty == NULL) n = 0;
> > - else n = tty->index;
> > - line = &lines[n];
> > + struct line *line = tty->driver_data;
> >
> >   down(&line->sem);
> >   line->count--;
> > -
> > - /* I don't like this, but I can't think of anything better.  What's
> > -  * going on is that the tty is in the process of being closed for
> > -  * the last time.  Its count hasn't been dropped yet, so it's still
> > -  * at 1.  This may happen when line->count != 0 because of the initial
> > -  * console open (without a tty) bumping it up to 1.
> > -  */
> > - if((line->tty != NULL) && (line->tty->count == 1))
> > -  line->tty = NULL;
> > - if(line->count == 0)
> > -  line_disable(line, -1);
> > + if (tty->count == 1) {

Hmm, while looking at this again, I think maybe this should be
"tty->count == 0" (it's _behind_ the line->count-- after all ...).
Guess you are writing me because you are trying to pin down a bug,
maybe that one is it ;)

> > +  line_disable(tty, -1);
> > +  tty->driver_data = NULL;
> > + }
> >   up(&line->sem);
> >  }
> Why did you delete this comment? I don't think you fixed the problem, you just 
> worked it around the other way...

No, it's fixed.

> Or better, you removed the initial console open, if it refers to the item #2 
> above... but the code isn't clean...

Yes, the hackish console open is gone, and thus the hack above which
attempts to care about that on close (but never really worked correctly)
is gone as well.

The old code used to use the stdio console device before it actually
registered the device in the tty layer, this is why these hacks used to
be in there.  Now the console device is registered after the stdio
console is initialized, and all the problems associated with that are
gone ;)

Drawback of that is that is that you don't see any messages until the
stdio console is initialized.  That isn't a big problem if the kernel
survives up to this point as the console code will simply print all
messages buffered up so far as soon as the console device is opened.
But if the kernel dies early you don't see the messages.  In that case
the new stderr console will help, which can be enabled with "stderr=1"
in the kernel cmd line (maybe additionally "console=stderr") and which
is simple enougth that it can register very early in the boot process.

HTH,

  Gerd

-- 
#define printk(args...) fprintf(stderr, ## args)


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] Re: Fw: [patch] uml: terminal cleanup

[Blaisorblade]

On Tuesday 08 March 2005 09:25, Gerd Knorr wrote:
> > > -void register_winch(int fd, void *device_data)
> > > +void register_winch(int fd, struct tty_struct *tty)
> > >  {
> > >   int pid, thread, thread_fd;
> > >   int count;

> > > -void line_close(struct line *lines, struct tty_struct *tty)
> > > +void line_close(struct tty_struct *tty, struct file * filp)
> > >  {
> > > - struct line *line;
> > > - int n;
> > > -
> > > - if(tty == NULL) n = 0;
> > > - else n = tty->index;
> > > - line = &lines[n];
> > > + struct line *line = tty->driver_data;
> > >
> > >   down(&line->sem);
> > >   line->count--;
> > > -
> > > - /* I don't like this, but I can't think of anything better.  What's
> > > -  * going on is that the tty is in the process of being closed for
> > > -  * the last time.  Its count hasn't been dropped yet, so it's still
> > > -  * at 1.  This may happen when line->count != 0 because of the
> > > initial -  * console open (without a tty) bumping it up to 1.
> > > -  */
> > > - if((line->tty != NULL) && (line->tty->count == 1))
> > > -  line->tty = NULL;
> > > - if(line->count == 0)
> > > -  line_disable(line, -1);
> > > + if (tty->count == 1) {
>
> Hmm, while looking at this again, I think maybe this should be
> "tty->count == 0" (it's _behind_ the line->count-- after all ...).

> Guess you are writing me because you are trying to pin down a bug,
> maybe that one is it ;)
No, because I got to properly read this just now.
> > > +  line_disable(tty, -1);
> > > +  tty->driver_data = NULL;
> > > + }
> > >   up(&line->sem);
> > >  }
> >
> > Why did you delete this comment? I don't think you fixed the problem, you
> > just worked it around the other way...
>
> No, it's fixed.
Well, if the above gets corrected (tty->count == 0 or 1), I think it's ok.

> Drawback of that is that is that you don't see any messages until the
> stdio console is initialized.  That isn't a big problem if the kernel
> survives up to this point as the console code will simply print all
> messages buffered up so far as soon as the console device is opened.

> But if the kernel dies early you don't see the messages.  In that case
> the new stderr console will help, which can be enabled with "stderr=1"
> in the kernel cmd line (maybe additionally "console=stderr") and which
> is simple enougth that it can register very early in the boot process.

No, this does not happen... the normal situation currently is that the kernel 
skips printing part of the initial output.

Sample output (note ject_hotplug - it's actually a truncated part of a longer 
message)

$ ./OldKernels/vmlinux-2.6.11-rc3
Checking for /proc/mm...found
Checking for the skas3 patch in the host...found
Checking PROT_EXEC mmap in /tmp...OK
ject_hotplug - call_usermodehelper returned -1
io scheduler noop registered
NET: Registered protocol family 2
IP: routing cache hash table of 256 buckets, 4Kbytes
TCP established hash table entries: 2048 (order: 3, 32768 bytes)
TCP bind hash table entries: 2048 (order: 2, 24576 bytes)
TCP: Hash tables configured (established 2048 bind 2048)
Initialized stdio console driver
Console initialized on /dev/tty0
Initializing software serial port version 1
Kernel panic - not syncing: VFS: Unable to mount root fs on 
unknown-block(98,0)
 <6>Stopping all CPUs...done
(stack trace).

$ ./OldKernels/vmlinux-2.6.11-rc3 stderr=1
Checking for /proc/mm...found
Checking for the skas3 patch in the host...found
Checking PROT_EXEC mmap in /tmp...OK
Linux version 2.6.11-rc3 (paolo@zion) (gcc version 3.3.2 (Mandrake Linux 10.0 
3.3.2-6mdk)) #10 SMP Thu Feb 3 23:38:59 CET 2005
Built 1 zonelists
Kernel command line: stderr=1 root=98:0
PID hash table entries: 256 (order: 8, 4096 bytes)
Dentry cache hash table entries: 8192 (order: 3, 32768 bytes)
Inode-cache hash table entries: 4096 (order: 2, 16384 bytes)
Memory: 27896k available
Security Framework v1.0.0 initialized
Mount-cache hash table entries: 512 (order: 0, 4096 bytes)
Checking for host processor cmov support...Yes
Checking for host processor xmm support...No
Checking that ptrace can change system call numbers...OK
Checking syscall emulation patch for ptrace...OK
Checking advanced syscall emulation patch for ptrace...missing
Checking that host ptys support output SIGIO...Yes
Checking that host ptys support SIGIO on close...No, enabling workaround
Checking for /dev/anon on the host...Not available (open failed with errno 2)
Brought up 1 CPUs
NET: Registered protocol family 16
TC classifier action (bugs to netdev@oss.sgi.com cc hadi@cyberus.ca)
mconsole (version 2) initialized on /home/paolo/.uml/GElkTZ/mconsole
ubd: Synchronous mode
audit: initializing netlink socket (disabled)
audit(300.260:0): initialized
Initializing Cryptographic API
io scheduler noop registered
NET: Registered protocol family 2
IP: routing cache hash table of 256 buckets, 4Kbytes
TCP established hash table entries: 2048 (order: 3, 32768 bytes)
TCP bind hash table entries: 2048 (order: 2, 24576 bytes)
TCP: Hash tables configured (established 2048 bind 2048)
Initialized stdio console driver
Initializing software serial port version 1
Kernel panic - not syncing: VFS: Unable to mount root fs on 
unknown-block(98,0)

$ ./vmlinux-2.6.11-rc4-bk-testLock-nofix console=stderr
Checking for /proc/mm...found
Checking for the skas3 patch in the host...found
Checking PROT_EXEC mmap in /tmp...OK

-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] Re: Fw: [patch] uml: terminal cleanup

[Gerd Knorr]

> No, this does not happen... the normal situation currently is that the kernel 
> skips printing part of the initial output.
> 
> $ ./OldKernels/vmlinux-2.6.11-rc3
> Checking for /proc/mm...found
> Checking for the skas3 patch in the host...found
> Checking PROT_EXEC mmap in /tmp...OK
> ject_hotplug - call_usermodehelper returned -1
> io scheduler noop registered
> NET: Registered protocol family 2
> IP: routing cache hash table of 256 buckets, 4Kbytes
> TCP established hash table entries: 2048 (order: 3, 32768 bytes)
> TCP bind hash table entries: 2048 (order: 2, 24576 bytes)
> TCP: Hash tables configured (established 2048 bind 2048)
> Initialized stdio console driver
> Console initialized on /dev/tty0
> Initializing software serial port version 1
> Kernel panic - not syncing: VFS: Unable to mount root fs on 
> unknown-block(98,0)

Ouch.  I've never seen that myself though, works perfectly fine for me.
Could be somehow related to the scrambled compiler logs reported a few
weeks ago, maybe the tty/line buffer handling is broken ...

> $ ./OldKernels/vmlinux-2.6.11-rc3 stderr=1

[ snip ]

as expected ...

> $ ./vmlinux-2.6.11-rc4-bk-testLock-nofix console=stderr
> Checking for /proc/mm...found
> Checking for the skas3 patch in the host...found
> Checking PROT_EXEC mmap in /tmp...OK
> [ no more messages ]

stderr=1 is needed in any case.  console=stderr might be needed
additionally in case you want to sent the kernel messages to _two_
console devices, i.e. something like this (both stderr and the xterm
for tty1):

# linux con=pts con1=xterm stderr=1 console=stderr console=tty1

HTH,

  Gerd

-- 
#define printk(args...) fprintf(stderr, ## args)


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] Re: Fw: [patch] uml: terminal cleanup

[Rob Landley]

On Thursday 10 March 2005 03:13 am, Gerd Knorr wrote:

> Ouch.  I've never seen that myself though, works perfectly fine for me.
> Could be somehow related to the scrambled compiler logs reported a few
> weeks ago, maybe the tty/line buffer handling is broken ...

I've done some more debugging on that, and I can reproduce it at will on any 
machine now.  The problem is that if the process that is on the other end of 
UML's output pipe gets stopped (like the Xterm consuming the UML output), its 
retry logic is borked.

I have a more detailed analysis with debug info on another machine (I'll try 
to dig that up later tonight), but really briefly, find out the PID of the 
xterm you're running in, I.E. the parent process of the shell you're going to 
run it under ("ps" followed by "ps l" should do it).  Then set up a dumb 
little script ala

while /bin/true
do
	kill -STOP $PID
	sleep 1
	kill -CONT $PID
	sleep 1
done

And now run UML and do and extract a tarball under it or something, and watch 
the output go bananas.

The requeue logic handling -EAGAIN requeues the data that's already on the 
buffer, not the new data that should go ON the buffer.  That's why the 
stuttering.  (What happens when the 4k buffer fills up is anybody's guess.  I 
think data just gets dropped.)

Rob


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel