linux-um.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [uml-devel] (no subject)
@ 2003-09-25  6:59 Jeff Chua
  2003-09-25 16:33 ` Adam Heath
  0 siblings, 1 reply; 26+ messages in thread
From: Jeff Chua @ 2003-09-25  6:59 UTC (permalink / raw)
  To: UserModeLinux


Please apply the patch below to host-skas3.patch so that it does not need
to be undone when compiling uml.

Currently, I need to apply host-skas3.patch to /usr/src/linux to compile
the host's vmlinux, and reverse it in order to compile uml linux. With the
patch below, this will prevent gcc from redefining the struct twice.

Thanks,
Jeff


--- linux/include/asm-i386/ptrace.h.org	Thu Sep 25 14:32:18 2003
+++ linux/include/asm-i386/ptrace.h	Thu Sep 25 14:34:49 2003
@@ -51,6 +51,7 @@

 #define PTRACE_SETOPTIONS         21

+#ifndef __arch_um__
 struct ptrace_faultinfo {
 	int is_write;
 	unsigned long addr;
@@ -61,6 +62,7 @@
   	void *ptr;
 	unsigned long bytecount;
 };
+#endif

 #define PTRACE_FAULTINFO 52
 #define PTRACE_SIGPENDING 53



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [uml-devel] (no subject)
  2003-09-25  6:59 Jeff Chua
@ 2003-09-25 16:33 ` Adam Heath
  0 siblings, 0 replies; 26+ messages in thread
From: Adam Heath @ 2003-09-25 16:33 UTC (permalink / raw)
  To: Jeff Chua; +Cc: UserModeLinux

On Thu, 25 Sep 2003, Jeff Chua wrote:

>
> Please apply the patch below to host-skas3.patch so that it does not need
> to be undone when compiling uml.
>
> Currently, I need to apply host-skas3.patch to /usr/src/linux to compile
> the host's vmlinux, and reverse it in order to compile uml linux. With the
> patch below, this will prevent gcc from redefining the struct twice.

A better fix it to modify uml's include, to not redefine the structure.

Maybe a common include, for both files.



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [uml-devel] (no subject)
@ 2003-09-26  1:09 kooper
  2003-10-06 21:22 ` Jeff Dike
  0 siblings, 1 reply; 26+ messages in thread
From: kooper @ 2003-09-26  1:09 UTC (permalink / raw)
  To: user-mode-linux-devel

i knew that in skas mode there is one thread that used as userspace
and switch processes (processes is traced by UML kernel). What i need to
know is how process that is traced by UML is stopped when entering in kernel
mode which is triggered by signal handling in UML kernel. Or process still
running while executing signal handler..
I dont know in which code i could find that...

Thank,
Eddy




-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [uml-devel] (no subject)
@ 2003-10-03 14:51 stian
  2003-10-06 21:25 ` Jeff Dike
  0 siblings, 1 reply; 26+ messages in thread
From: stian @ 2003-10-03 14:51 UTC (permalink / raw)
  To: user-mode-linux-devel

I'd like to share memory between the uml-kernel and a user-space util
(some megabytes). I'd like this to be a file-descriptor that is mmap'ed in
both processes. What is the most nice way to do this?

The memory manager in uml can do this for me as far as I can see (but I'm
not into the internals to hack out the fd and memory maped up in the
kernel side).

Or should I just use malloc() and mmap into this area? Doesn't this fuck
up when uml goes between internal user space and kernel space, remapping
in the /tmp/foo (deleted) files? into the malloc'ed areas?

If so, this might be an issue for the dude who wants to play with gtk/x11
into the kernel then some points of implementations plays with
mmap-buffers etc.



Stian


This mail has been scanned for known viruses on an open, Linux-based mailsystem
developed by http://Knowledge-Network.no and http://Nixia.no.


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [uml-devel] (no subject)
  2003-09-26  1:09 kooper
@ 2003-10-06 21:22 ` Jeff Dike
  0 siblings, 0 replies; 26+ messages in thread
From: Jeff Dike @ 2003-10-06 21:22 UTC (permalink / raw)
  To: kooper; +Cc: user-mode-linux-devel

kooper@inf.its-sby.edu said:
> What i need to know is how process that is traced by UML is stopped
> when entering in kernel mode which is triggered by signal handling in
> UML kernel. Or process still running while executing signal handler..
> I dont know in which code i could find that...

Look at arch/um/kernel/skas/process.c.

				Jeff



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [uml-devel] (no subject)
  2003-10-03 14:51 stian
@ 2003-10-06 21:25 ` Jeff Dike
  0 siblings, 0 replies; 26+ messages in thread
From: Jeff Dike @ 2003-10-06 21:25 UTC (permalink / raw)
  To: stian; +Cc: user-mode-linux-devel

stian@nixia.no said:
> I'd like to share memory between the uml-kernel and a user-space util
> (some megabytes).
> I'd like this to be a file-descriptor that is mmap'ed in both
> processes. What is the most nice way to do this?

Use iomem, which does pretty much what you want.

				Jeff



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [uml-devel] (no subject)
@ 2004-09-20 19:55 Stroesser, Bodo
  2004-09-25 17:34 ` BlaisorBlade
  0 siblings, 1 reply; 26+ messages in thread
From: Stroesser, Bodo @ 2004-09-20 19:55 UTC (permalink / raw)
  To: user-mode-linux-devel; +Cc: BlaisorBlade

[-- Attachment #1: Type: text/plain, Size: 2999 bytes --]

Today I tried testing interrupted systemcalls on UML 2.6.8.1-1 with the
incremental patches up to skas-flush-tlb. My host does not yet support
the new SYSEMU-ptrace.

In skas-mode, the uml system with this kernel doesn't boot! It panics
with the message:

 

Kernel panic: handle_trap - failed to wait at end of syscall, errno = 4,
status = 4479

 

Tracking this down I saw that "errno = 4" from the message is
misleading! It's an old errno, the return value of waitpid() is the pid
of the user-thread. It seems to be stopped by SIGCHLD.

I do not understand exactly what happens, but the signal seems to be
generated while running handle_syscall() called from handle_trap().
Changing back to the sequence of calling waitpid() and handle_syscall()
as it has been before the sysemu-patch removed the problem.

 

Here is my patch (I also changed the panic message to include "err" and
to print status in Hex):

 

Bodo

 

 

--- tmp/linux-2.6.8.1/arch/um/kernel/skas/process.c     2004-09-17
19:20:15.000000000 +0200

+++ linux-2.6.8.1/arch/um/kernel/skas/process.c 2004-09-20
21:31:39.451946258 +0200

@@ -68,25 +68,25 @@

                return;

        }

 

-       handle_syscall(regs);

-       if(use_sysemu)

-               return;

+       if(!use_sysemu) {

+               err = ptrace(PTRACE_POKEUSER, pid, PT_SYSCALL_NR_OFFSET,
__NR_getpid);

+               if(err < 0)

+                       panic("handle_trap - nullifying syscall failed,
errno = %d\n",

+                             errno);

 

-       err = ptrace(PTRACE_POKEUSER, pid, PT_SYSCALL_NR_OFFSET,
__NR_getpid);

-       if(err < 0)

-               panic("handle_trap - nullifying syscall failed, errno =
%d\n",

-                     errno);

+               err = ptrace(PTRACE_SYSCALL, pid, 0, 0);

+               if(err < 0)

+                       panic("handle_trap - continuing to end of
syscall failed, "

+                             "errno = %d\n", errno);

 

-       err = ptrace(PTRACE_SYSCALL, pid, 0, 0);

-       if(err < 0)

-               panic("handle_trap - continuing to end of syscall
failed, "

-                     "errno = %d\n", errno);

-

-       CATCH_EINTR(err = waitpid(pid, &status, WUNTRACED));

-       if((err < 0) || !WIFSTOPPED(status) ||

-          (WSTOPSIG(status) != SIGTRAP))

-               panic("handle_trap - failed to wait at end of syscall, "

-                     "errno = %d, status = %d\n", errno, status);

+               CATCH_EINTR(err = waitpid(pid, &status, WUNTRACED));

+               if((err < 0) || !WIFSTOPPED(status) ||

+                  (WSTOPSIG(status) != SIGTRAP))

+                       panic("handle_trap - failed to wait at end of
syscall, "

+                             "err = %d, errno = %d, status = %x\n",
err, errno, status);

+       }

+

+       handle_syscall(regs);

 }

 

 

 


[-- Attachment #2: Type: text/html, Size: 15299 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [uml-devel] (no subject)
  2004-09-20 19:55 Stroesser, Bodo
@ 2004-09-25 17:34 ` BlaisorBlade
  0 siblings, 0 replies; 26+ messages in thread
From: BlaisorBlade @ 2004-09-25 17:34 UTC (permalink / raw)
  To: user-mode-linux-devel; +Cc: Stroesser, Bodo

On Monday 20 September 2004 21:55, Stroesser, Bodo wrote:
> Today I tried testing interrupted systemcalls on UML 2.6.8.1-1 with the
> incremental patches up to skas-flush-tlb. My host does not yet support
> the new SYSEMU-ptrace.
>
> In skas-mode, the uml system with this kernel doesn't boot! It panics
> with the message:

> Kernel panic: handle_trap - failed to wait at end of syscall, errno = 4,
> status = 4479

> Tracking this down I saw that "errno = 4" from the message is
> misleading!

> It's an old errno, the return value of waitpid() is the pid 
> of the user-thread. It seems to be stopped by SIGCHLD.
Yes, I get some similar failures with the misleading "errno = 4"; I guess it's 
time to fix, maybe, either CATCH_EINTR or just this invocation of it.
However, the problem that UML should not expect the child getting only those 
virtual SIGTRAP (see man 2 ptrace); any other signal in that point should 
IMHO just be ignored. I've not written the patch but it is just adding Yet 
Another Loop, beyond the CATCH_EINTR one (this time no general macro are 
needed).

> I do not understand exactly what happens, but the signal seems to be
> generated while running handle_syscall() called from handle_trap().
> Changing back to the sequence of calling waitpid() and handle_syscall()
> as it has been before the sysemu-patch removed the problem.

> Here is my patch (I also changed the panic message to include "err" and
> to print status in Hex):
This particular part is Ok, but could you, please, avoid trumpling over and 
removing the sysemu code? It's absolutely not needed.
If you enclose the last two ptrace calls in "if (!use_sysemu)" and you move 
the "handle_syscall" after them, you get your fix + no change for sysemu.


> --- tmp/linux-2.6.8.1/arch/um/kernel/skas/process.c     2004-09-17
> 19:20:15.000000000 +0200
>
> +++ linux-2.6.8.1/arch/um/kernel/skas/process.c 2004-09-20
> 21:31:39.451946258 +0200
>
> @@ -68,25 +68,25 @@
>
>                 return;
>
>         }
>
>
>
> -       handle_syscall(regs);
>
A side note: posting HTML messages is bad (on LKML it is enough to be totally 
& completely ignored, I guess); and the above is what the inline patch 
becomes while answering to an HTML message - could you please either send 
plain-text or move the patches to attachments (I strongly prefer the first 
solution)?

Bye
-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729


-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [uml-devel] (no subject)
@ 2004-09-27  9:20 Stroesser, Bodo
  2004-09-29 17:33 ` BlaisorBlade
  0 siblings, 1 reply; 26+ messages in thread
From: Stroesser, Bodo @ 2004-09-27  9:20 UTC (permalink / raw)
  To: user-mode-linux-devel; +Cc: BlaisorBlade

On Saturday 25 September 2004 21:35, BlaisorBlade wrote: 
> ...
> This particular part is Ok, but could you, please, avoid trumpling
over and removing the sysemu code? It's absolutely not needed.
> If you enclose the last two ptrace calls in "if (!use_sysemu)" and you
move the "handle_syscall" after them, you get your fix + no change for
sysemu.
Sorry. I didn't intend to break sysemu! Did I miss something or does it
come from my confusing mail format?
Here is the code of handle_trap() after applying the patch:

---
static void handle_trap(int pid, union uml_pt_regs *regs)
{
	int err, syscall_nr, status;

	syscall_nr = PT_SYSCALL_NR(regs->skas.regs);
	UPT_SYSCALL_NR(regs) = syscall_nr;
	if(syscall_nr < 0){
		relay_signal(SIGTRAP, regs);
		return;
	}

	if(!use_sysemu) {
		err = ptrace(PTRACE_POKEUSER, pid, PT_SYSCALL_NR_OFFSET,
__NR_getpid);
		if(err < 0)
			panic("handle_trap - nullifying syscall failed,
errno = %d\n",
			      errno);

		err = ptrace(PTRACE_SYSCALL, pid, 0, 0);
		if(err < 0)
			panic("handle_trap - continuing to end of
syscall failed, "
			      "errno = %d\n", errno);

		CATCH_EINTR(err = waitpid(pid, &status, WUNTRACED));
		if((err < 0) || !WIFSTOPPED(status) || 
		   (WSTOPSIG(status) != SIGTRAP))
			panic("handle_trap - failed to wait at end of
syscall, "
			      "err = %d, errno = %d, status = %x\n",
err, errno, status);
	}

	handle_syscall(regs);
}

> ...
> A side note: posting HTML messages is bad (on LKML it is enough to be
totally & completely ignored, I guess); and the above is what the inline
patch becomes while answering to an HTML message - could you please
either send plain-text or move the patches to attachments (I strongly
prefer the first solution)?
OK. Thank you for the advice. Unfortunately I'm missing experience in
writing to mailing lists. And I have to do it with our companies Outlook
client...
In the meantime I found a switch to modify the settings to "text only".
Hope it's better now?

Bodo


-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [uml-devel] (no subject)
  2004-09-27  9:20 Stroesser, Bodo
@ 2004-09-29 17:33 ` BlaisorBlade
  0 siblings, 0 replies; 26+ messages in thread
From: BlaisorBlade @ 2004-09-29 17:33 UTC (permalink / raw)
  To: user-mode-linux-devel; +Cc: Stroesser, Bodo, Jeff Dike

On Monday 27 September 2004 11:20, Stroesser, Bodo wrote:
> On Saturday 25 September 2004 21:35, BlaisorBlade wrote:
> > ...
> > This particular part is Ok, but could you, please, avoid trumpling
>
> over and removing the sysemu code? It's absolutely not needed.
>
> > If you enclose the last two ptrace calls in "if (!use_sysemu)" and you
>
> move the "handle_syscall" after them, you get your fix + no change for
> sysemu.
> Sorry. I didn't intend to break sysemu! Did I miss something or does it
> come from my confusing mail format?
> Here is the code of handle_trap() after applying the patch:
Ah, ok. Please accept my excuses, I didn't read well the patch (was I too 
tired?). However ok, it is correct, and the code is written this way in 
2.6.9-rc2, in fact, as with the original sysemu patch - the problem was in 
some little changes done by Jeff in his tree.


> > ...
> > A side note: posting HTML messages is bad (on LKML it is enough to be
>
> totally & completely ignored, I guess); and the above is what the inline
> patch becomes while answering to an HTML message - could you please
> either send plain-text or move the patches to attachments (I strongly
> prefer the first solution)?

> OK. Thank you for the advice. Unfortunately I'm missing experience in
> writing to mailing lists. And I have to do it with our companies Outlook
> client...
> In the meantime I found a switch to modify the settings to "text only".
> Hope it's better now?

Ok, that's perfect!
If you want to get advices on such questions, you may want to take a look at 
the LKML FAQ:

http://www.tux.org/lkml/
(The only important question I remember is "when/why to send the patch 
inline/attached/send an URL from the Web").

And these two documents about patch submission format:

http://linux.yyz.us/patch-format.html
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt
(Note: when you find differences, do whatever you prefer; inserting the kernel 
version is a good idea, like the 1st doc says).

Bye
-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [uml-devel] (no subject)
@ 2005-03-20 22:05 William Stearns
  0 siblings, 0 replies; 26+ messages in thread
From: William Stearns @ 2005-03-20 22:05 UTC (permalink / raw)
  To: ML-uml-user, ML-uml-devel; +Cc: William Stearns, Jeff Dike

Good day, all,
 	Charlyn Dike!  Jeff and Charlyn were married yesterday in Boston 
Mass, USA in a small ceremony near the MIT campus.
 	The two of them will be traveling for a few weeks.  Jeff is 
unlikely to be near a good Internet connection for a while, so we should 
find our own UML answers until he gets back.
 	Jeff and Charlyn, I'd like to wish you the best of luck and 
happiness in your life together.
 	Cheers,
 	- Bill

---------------------------------------------------------------------------
         "There is no beautifier of complexion or form of behavior like
the wish to scatter joy, and not pain, around us."
         -- Ralph Waldo Emerson
--------------------------------------------------------------------------
William Stearns (wstearns@pobox.com).  Mason, Buildkernel, freedups, p0f,
rsync-backup, ssh-keyinstall, dns-check, more at:   http://www.stearns.org
--------------------------------------------------------------------------


-------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [uml-devel] (no subject)
@ 2007-10-01 16:06 dhowells
  0 siblings, 0 replies; 26+ messages in thread
From: dhowells @ 2007-10-01 16:06 UTC (permalink / raw)


From dhowells  Mon Oct  1 14: 11:59 2007
Return-Path: <dhowells@redhat.com>
Received: from localhost.localdomain [127.0.0.1]
	by warthog.procyon.org.uk with IMAP (fetchmail-6.3.7)
	for <dhowells@localhost> (single-drop); Mon, 01 Oct 2007 14:11:59 +0100 (BST)
Received: from pobox.devel.redhat.com ([unix socket])
	 by pobox.devel.redhat.com (Cyrus v2.2.12-Invoca-RPM-2.2.12-8.1.RHEL4) with LMTPA;
	 Mon, 01 Oct 2007 09:11:50 -0400
X-Sieve: CMU Sieve 2.2
Received: from warthog.cambridge.redhat.com (devserv.devel.redhat.com [10.10.36.72])
	by pobox.devel.redhat.com (8.13.1/8.13.1) with ESMTP id l91DBnVD031197;
	Mon, 1 Oct 2007 09:11:49 -0400
Received: from [127.0.0.1] (helo=warthog.procyon.org.uk)
	by warthog.cambridge.redhat.com with esmtp (Exim 4.66 #1 (Red Hat Linux))
	id 1IcL3u-0007i8-3r; Mon, 01 Oct 2007 14:11:50 +0100
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
	Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
	Kingdom.
	Registered in England and Wales under Company Registration No. 3798903
From: David Howells <dhowells@redhat.com>
Subject: [PATCH 29/30] IGET: Stop HPPFS from using iget() and read_inode()
To: hch@infradead.org, viro@ftp.linux.org.uk, torvalds@osdl.org,
    akpm@osdl.org
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
    dhowells@redhat.com
Date: Mon, 01 Oct 2007 14:11:50 +0100
Message-ID: <20071001131150.29339.92417.stgit@warthog.procyon.org.uk>
In-Reply-To: <20071001130921.29339.72876.stgit@warthog.procyon.org.uk>
References: <20071001130921.29339.72876.stgit@warthog.procyon.org.uk>
User-Agent: StGIT/0.13
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Resent-To: user-mode-linux-devel@lists.sourceforge.net
Resent-cc: jdike@karaya.com
Resent-Date: Mon, 01 Oct 2007 17:06:33 +0100
Resent-Message-ID: <19196.1191254793@redhat.com>
Resent-From: David Howells <dhowells@redhat.com>

Stop the HPPFS filesystem from using iget() and read_inode().  Provide an
hppfs_iget(), and call that instead of iget().  hppfs_iget() then uses
iget_locked() directly and returns a proper error code instead of an inode in
the event of an error.

hppfs_fill_sb_common() returns any error incurred when getting the root inode
instead of EINVAL.

Note that the contents of hppfs_kern.c need to be examined:

 (*) The HPPFS inode retains a pointer to the proc dentry it is shadowing, but
     whilst it does appear to retain a reference to it, it doesn't appear to
     destroy the reference if the inode goes away.

 (*) hppfs_iget() should perhaps subsume init_inode() and hppfs_read_inode().

 (*) It would appear that all hppfs inodes are the same inode because iget()
     was being called with inode number 0, which forms the lookup key.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/hppfs/hppfs_kern.c |   27 ++++++++++++++++++++++-----
 1 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/fs/hppfs/hppfs_kern.c b/fs/hppfs/hppfs_kern.c
index affb741..a1e1f0f 100644
--- a/fs/hppfs/hppfs_kern.c
+++ b/fs/hppfs/hppfs_kern.c
@@ -155,6 +155,20 @@ static void hppfs_read_inode(struct inode *ino)
 	ino->i_blocks = proc_ino->i_blocks;
 }
 
+static struct inode *hppfs_iget(struct super_block *sb)
+{
+	struct inode *inode;
+
+	inode = iget_locked(sb, 0);
+	if (!inode)
+		return ERR_PTR(-ENOMEM);
+	if (inode->i_state & I_NEW) {
+		hppfs_read_inode(inode);
+		unlock_new_inode(inode);
+	}
+	return inode;
+}
+
 static struct dentry *hppfs_lookup(struct inode *ino, struct dentry *dentry,
                                   struct nameidata *nd)
 {
@@ -190,9 +204,11 @@ static struct dentry *hppfs_lookup(struct inode *ino, struct dentry *dentry,
 	if(IS_ERR(proc_dentry))
 		return(proc_dentry);
 
-	inode = iget(ino->i_sb, 0);
-	if(inode == NULL)
+	inode = hppfs_iget(ino->i_sb);
+	if (IS_ERR(inode)) {
+		err = PTR_ERR(inode);
 		goto out_dput;
+	}
 
 	err = init_inode(inode, proc_dentry);
 	if(err)
@@ -652,7 +668,6 @@ static void hppfs_destroy_inode(struct inode *inode)
 static const struct super_operations hppfs_sbops = {
 	.alloc_inode	= hppfs_alloc_inode,
 	.destroy_inode	= hppfs_destroy_inode,
-	.read_inode	= hppfs_read_inode,
 	.delete_inode	= hppfs_delete_inode,
 	.statfs		= hppfs_statfs,
 };
@@ -745,9 +760,11 @@ static int hppfs_fill_super(struct super_block *sb, void *d, int silent)
 	sb->s_magic = HPPFS_SUPER_MAGIC;
 	sb->s_op = &hppfs_sbops;
 
-	root_inode = iget(sb, 0);
-	if(root_inode == NULL)
+	root_inode = hppfs_iget(sb);
+	if (IS_ERR(root_inode)) {
+		err = PTR_ERR(root_inode);
 		goto out;
+	}
 
 	err = init_inode(root_inode, proc_sb->s_root);
 	if(err)


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [uml-devel] (no subject)
@ 2007-10-01 16:06 dhowells
  0 siblings, 0 replies; 26+ messages in thread
From: dhowells @ 2007-10-01 16:06 UTC (permalink / raw)


From dhowells  Mon Oct  1 14: 11:57 2007
Return-Path: <dhowells@redhat.com>
Received: from localhost.localdomain [127.0.0.1]
	by warthog.procyon.org.uk with IMAP (fetchmail-6.3.7)
	for <dhowells@localhost> (single-drop); Mon, 01 Oct 2007 14:11:57 +0100 (BST)
Received: from pobox.devel.redhat.com ([unix socket])
	 by pobox.devel.redhat.com (Cyrus v2.2.12-Invoca-RPM-2.2.12-8.1.RHEL4) with LMTPA;
	 Mon, 01 Oct 2007 09:11:45 -0400
X-Sieve: CMU Sieve 2.2
Received: from warthog.cambridge.redhat.com (devserv.devel.redhat.com [10.10.36.72])
	by pobox.devel.redhat.com (8.13.1/8.13.1) with ESMTP id l91DBimG031190;
	Mon, 1 Oct 2007 09:11:45 -0400
Received: from [127.0.0.1] (helo=warthog.procyon.org.uk)
	by warthog.cambridge.redhat.com with esmtp (Exim 4.66 #1 (Red Hat Linux))
	id 1IcL3p-0007hp-1L; Mon, 01 Oct 2007 14:11:45 +0100
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
	Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
	Kingdom.
	Registered in England and Wales under Company Registration No. 3798903
From: David Howells <dhowells@redhat.com>
Subject: [PATCH 28/30] IGET: Stop HOSTFS from using iget() and read_inode()
To: hch@infradead.org, viro@ftp.linux.org.uk, torvalds@osdl.org,
    akpm@osdl.org
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
    dhowells@redhat.com
Date: Mon, 01 Oct 2007 14:11:45 +0100
Message-ID: <20071001131144.29339.30718.stgit@warthog.procyon.org.uk>
In-Reply-To: <20071001130921.29339.72876.stgit@warthog.procyon.org.uk>
References: <20071001130921.29339.72876.stgit@warthog.procyon.org.uk>
User-Agent: StGIT/0.13
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Resent-To: user-mode-linux-devel@lists.sourceforge.net
Resent-cc: jdike@karaya.com
Resent-Date: Mon, 01 Oct 2007 17:06:27 +0100
Resent-Message-ID: <19182.1191254787@redhat.com>
Resent-From: David Howells <dhowells@redhat.com>

Stop the HOSTFS filesystem from using iget() and read_inode().  Provide
hostfs_iget(), and call that instead of iget().  hostfs_iget() then uses
iget_locked() directly and returns a proper error code instead of an inode in
the event of an error.

hostfs_fill_sb_common() returns any error incurred when getting the root inode
instead of EINVAL.

Note that the contents of hostfs_kern.c need to be examined:

 (*) hostfs_iget() should perhaps subsume init_inode() and hostfs_read_inode().

 (*) It would appear that all hostfs inodes are the same inode because iget()
     was being called with inode number 0 - which forms the lookup key.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/hostfs/hostfs_kern.c |   58 ++++++++++++++++++++++++++++++++---------------
 1 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
index c778620..c6a456a 100644
--- a/fs/hostfs/hostfs_kern.c
+++ b/fs/hostfs/hostfs_kern.c
@@ -208,7 +208,7 @@ static char *follow_link(char *link)
 	return ERR_PTR(n);
 }
 
-static int read_inode(struct inode *ino)
+static int hostfs_read_inode(struct inode *ino)
 {
 	char *name;
 	int err = 0;
@@ -238,6 +238,25 @@ static int read_inode(struct inode *ino)
 	return err;
 }
 
+static struct inode *hostfs_iget(struct super_block *sb)
+{
+	struct inode *inode;
+	long ret;
+
+	inode = iget_locked(sb, 0);
+	if (!inode)
+		return ERR_PTR(-ENOMEM);
+	if (inode->i_state & I_NEW) {
+		ret = hostfs_read_inode(inode);
+		if (ret < 0) {
+			iget_failed(inode);
+			return ERR_PTR(ret);
+		}
+		unlock_new_inode(inode);
+	}
+	return inode;
+}
+
 int hostfs_statfs(struct dentry *dentry, struct kstatfs *sf)
 {
 	/* do_statfs uses struct statfs64 internally, but the linux kernel
@@ -305,17 +324,11 @@ static void hostfs_destroy_inode(struct inode *inode)
 	kfree(HOSTFS_I(inode));
 }
 
-static void hostfs_read_inode(struct inode *inode)
-{
-	read_inode(inode);
-}
-
 static const struct super_operations hostfs_sbops = {
 	.alloc_inode	= hostfs_alloc_inode,
 	.drop_inode	= generic_delete_inode,
 	.delete_inode   = hostfs_delete_inode,
 	.destroy_inode	= hostfs_destroy_inode,
-	.read_inode	= hostfs_read_inode,
 	.statfs		= hostfs_statfs,
 };
 
@@ -584,9 +597,11 @@ int hostfs_create(struct inode *dir, struct dentry *dentry, int mode,
 	char *name;
 	int error, fd;
 
-	error = -ENOMEM;
-	inode = iget(dir->i_sb, 0);
-	if(inode == NULL) goto out;
+	inode = hostfs_iget(dir->i_sb);
+	if (IS_ERR(inode)) {
+		error = PTR_ERR(inode);
+		goto out;
+	}
 
 	error = init_inode(inode, dentry);
 	if(error)
@@ -627,10 +642,11 @@ struct dentry *hostfs_lookup(struct inode *ino, struct dentry *dentry,
 	char *name;
 	int err;
 
-	err = -ENOMEM;
-	inode = iget(ino->i_sb, 0);
-	if(inode == NULL)
+	inode = hostfs_iget(ino->i_sb);
+	if (IS_ERR(inode)) {
+		err = PTR_ERR(inode);
 		goto out;
+	}
 
 	err = init_inode(inode, dentry);
 	if(err)
@@ -748,11 +764,13 @@ int hostfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
 {
 	struct inode *inode;
 	char *name;
-	int err = -ENOMEM;
+	int err;
 
-	inode = iget(dir->i_sb, 0);
-	if(inode == NULL)
+	inode = hostfs_iget(dir->i_sb);
+	if (IS_ERR(inode)) {
+		err = PTR_ERR(inode);
 		goto out;
+	}
 
 	err = init_inode(inode, dentry);
 	if(err)
@@ -973,9 +991,11 @@ static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent)
 
 	sprintf(host_root_path, "%s/%s", root_ino, req_root);
 
-	root_inode = iget(sb, 0);
-	if(root_inode == NULL)
+	root_inode = hostfs_iget(sb);
+	if (IS_ERR(root_inode)) {
+		err = PTR_ERR(root_inode);
 		goto out_free;
+	}
 
 	err = init_inode(root_inode, NULL);
 	if(err)
@@ -991,7 +1011,7 @@ static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent)
 	if(sb->s_root == NULL)
 		goto out_put;
 
-	err = read_inode(root_inode);
+	err = hostfs_read_inode(root_inode);
 	if(err){
 		/* No iput in this case because the dput does that for us */
 		dput(sb->s_root);


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [uml-devel] (no subject)
  2009-03-30 18:41 [uml-devel] [patch 1/3] uml: fix compile error from net_device_ops conversion Miklos Szeredi
@ 2009-03-30 18:42 ` Miklos Szeredi
  0 siblings, 0 replies; 26+ messages in thread
From: Miklos Szeredi @ 2009-03-30 18:42 UTC (permalink / raw)
  To: user-mode-linux-devel; +Cc: jdike, akpm, linux-kernel

BCC: miko
Subject: [patch 2/3] uml: fix link error from prefixing of i386 syscalls with ptregs_
References:  <E1LoMQ2-0007bB-By@pomaz-ex.szeredi.hu>
--text follows this line--
From: Miklos Szeredi <mszeredi@suse.cz>

Fix the following link error:

arch/um/sys-i386/built-in.o: In function `sys_call_table':
(.rodata+0x11c): undefined reference to `ptregs_fork'
arch/um/sys-i386/built-in.o: In function `sys_call_table':
(.rodata+0x140): undefined reference to `ptregs_execve'
arch/um/sys-i386/built-in.o: In function `sys_call_table':
(.rodata+0x2cc): undefined reference to `ptregs_iopl'
arch/um/sys-i386/built-in.o: In function `sys_call_table':
(.rodata+0x2d8): undefined reference to `ptregs_vm86old'
arch/um/sys-i386/built-in.o: In function `sys_call_table':
(.rodata+0x2f0): undefined reference to `ptregs_sigreturn'
arch/um/sys-i386/built-in.o: In function `sys_call_table':
(.rodata+0x2f4): undefined reference to `ptregs_clone'
arch/um/sys-i386/built-in.o: In function `sys_call_table':
(.rodata+0x3ac): undefined reference to `ptregs_vm86'
arch/um/sys-i386/built-in.o: In function `sys_call_table':
(.rodata+0x3c8): undefined reference to `ptregs_rt_sigreturn'
arch/um/sys-i386/built-in.o: In function `sys_call_table':
(.rodata+0x3fc): undefined reference to `ptregs_sigaltstack'
arch/um/sys-i386/built-in.o: In function `sys_call_table':
(.rodata+0x40c): undefined reference to `ptregs_vfork'

This was introduced by commit 253f29a4, "x86: pass in pt_regs pointer
for syscalls that need it"

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 arch/um/sys-i386/sys_call_table.S |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: linux-2.6/arch/um/sys-i386/sys_call_table.S
===================================================================
--- linux-2.6.orig/arch/um/sys-i386/sys_call_table.S	2009-03-27 10:57:38.000000000 +0100
+++ linux-2.6/arch/um/sys-i386/sys_call_table.S	2009-03-30 19:53:14.000000000 +0200
@@ -9,6 +9,17 @@
 
 #define old_mmap old_mmap_i386
 
+#define ptregs_fork sys_fork
+#define ptregs_execve sys_execve
+#define ptregs_iopl sys_iopl
+#define ptregs_vm86old sys_vm86old
+#define ptregs_sigreturn sys_sigreturn
+#define ptregs_clone sys_clone
+#define ptregs_vm86 sys_vm86
+#define ptregs_rt_sigreturn sys_rt_sigreturn
+#define ptregs_sigaltstack sys_sigaltstack
+#define ptregs_vfork sys_vfork
+
 .section .rodata,"a"
 
 #include "../../x86/kernel/syscall_table_32.S"

------------------------------------------------------------------------------
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [uml-devel] (no subject)
@ 2012-02-12  0:21 Richard Weinberger
  2012-02-12  0:24 ` [uml-devel] [PATCH] um: Use tty_port Richard Weinberger
                   ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: Richard Weinberger @ 2012-02-12  0:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: user-mode-linux-devel, Richard Weinberger, viro, gregkh, akpm,
	alan

Can you please review this patch?

Thanks,
//richard

---
>>From d8f5e7953def150bcc1e6a39dbbe589f1c68bcbd Mon Sep 17 00:00:00 2001
From: Richard Weinberger <richard@nod.at>
Date: Sun, 12 Feb 2012 01:12:49 +0100
Subject: [PATCH] um: Use tty_port

UML's line driver has to use tty_port.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 arch/um/drivers/line.c          |  212 +++++++++++---------------------------
 arch/um/drivers/line.h          |   13 ++-
 arch/um/drivers/ssl.c           |   16 +++-
 arch/um/drivers/stdio_console.c |   14 ++-
 4 files changed, 94 insertions(+), 161 deletions(-)

diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c
index c1cf220..c789748 100644
--- a/arch/um/drivers/line.c
+++ b/arch/um/drivers/line.c
@@ -19,19 +19,29 @@ static irqreturn_t line_interrupt(int irq, void *data)
 {
 	struct chan *chan = data;
 	struct line *line = chan->line;
+	struct tty_struct *tty;
+
+	if (line) {
+		tty = tty_port_tty_get(&line->port);
+		chan_interrupt(&line->chan_list, &line->task, tty, irq);
+		tty_kref_put(tty);
+	}
 
-	if (line)
-		chan_interrupt(&line->chan_list, &line->task, line->tty, irq);
 	return IRQ_HANDLED;
 }
 
 static void line_timer_cb(struct work_struct *work)
 {
 	struct line *line = container_of(work, struct line, task.work);
+	struct tty_struct *tty;
 
-	if (!line->throttled)
-		chan_interrupt(&line->chan_list, &line->task, line->tty,
+	if (!line->throttled) {
+		tty = tty_port_tty_get(&line->port);
+		chan_interrupt(&line->chan_list, &line->task, tty,
 			       line->driver->read_irq);
+
+		tty_kref_put(tty);
+	}
 }
 
 /*
@@ -228,92 +238,6 @@ void line_set_termios(struct tty_struct *tty, struct ktermios * old)
 	/* nothing */
 }
 
-static const struct {
-	int  cmd;
-	char *level;
-	char *name;
-} tty_ioctls[] = {
-	/* don't print these, they flood the log ... */
-	{ TCGETS,      NULL,       "TCGETS"      },
-	{ TCSETS,      NULL,       "TCSETS"      },
-	{ TCSETSW,     NULL,       "TCSETSW"     },
-	{ TCFLSH,      NULL,       "TCFLSH"      },
-	{ TCSBRK,      NULL,       "TCSBRK"      },
-
-	/* general tty stuff */
-	{ TCSETSF,     KERN_DEBUG, "TCSETSF"     },
-	{ TCGETA,      KERN_DEBUG, "TCGETA"      },
-	{ TIOCMGET,    KERN_DEBUG, "TIOCMGET"    },
-	{ TCSBRKP,     KERN_DEBUG, "TCSBRKP"     },
-	{ TIOCMSET,    KERN_DEBUG, "TIOCMSET"    },
-
-	/* linux-specific ones */
-	{ TIOCLINUX,   KERN_INFO,  "TIOCLINUX"   },
-	{ KDGKBMODE,   KERN_INFO,  "KDGKBMODE"   },
-	{ KDGKBTYPE,   KERN_INFO,  "KDGKBTYPE"   },
-	{ KDSIGACCEPT, KERN_INFO,  "KDSIGACCEPT" },
-};
-
-int line_ioctl(struct tty_struct *tty, unsigned int cmd,
-				unsigned long arg)
-{
-	int ret;
-	int i;
-
-	ret = 0;
-	switch(cmd) {
-#ifdef TIOCGETP
-	case TIOCGETP:
-	case TIOCSETP:
-	case TIOCSETN:
-#endif
-#ifdef TIOCGETC
-	case TIOCGETC:
-	case TIOCSETC:
-#endif
-#ifdef TIOCGLTC
-	case TIOCGLTC:
-	case TIOCSLTC:
-#endif
-	/* Note: these are out of date as we now have TCGETS2 etc but this
-	   whole lot should probably go away */
-	case TCGETS:
-	case TCSETSF:
-	case TCSETSW:
-	case TCSETS:
-	case TCGETA:
-	case TCSETAF:
-	case TCSETAW:
-	case TCSETA:
-	case TCXONC:
-	case TCFLSH:
-	case TIOCOUTQ:
-	case TIOCINQ:
-	case TIOCGLCKTRMIOS:
-	case TIOCSLCKTRMIOS:
-	case TIOCPKT:
-	case TIOCGSOFTCAR:
-	case TIOCSSOFTCAR:
-		return -ENOIOCTLCMD;
-#if 0
-	case TCwhatever:
-		/* do something */
-		break;
-#endif
-	default:
-		for (i = 0; i < ARRAY_SIZE(tty_ioctls); i++)
-			if (cmd == tty_ioctls[i].cmd)
-				break;
-		if (i == ARRAY_SIZE(tty_ioctls)) {
-			printk(KERN_ERR "%s: %s: unknown ioctl: 0x%x\n",
-			       __func__, tty->name, cmd);
-		}
-		ret = -ENOIOCTLCMD;
-		break;
-	}
-	return ret;
-}
-
 void line_throttle(struct tty_struct *tty)
 {
 	struct line *line = tty->driver_data;
@@ -343,7 +267,7 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
 {
 	struct chan *chan = data;
 	struct line *line = chan->line;
-	struct tty_struct *tty = line->tty;
+	struct tty_struct *tty = tty_port_tty_get(&line->port);
 	int err;
 
 	/*
@@ -354,6 +278,9 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
 	spin_lock(&line->lock);
 	err = flush_buffer(line);
 	if (err == 0) {
+		tty_kref_put(tty);
+
+		spin_unlock(&line->lock);
 		return IRQ_NONE;
 	} else if (err < 0) {
 		line->head = line->buffer;
@@ -365,9 +292,12 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
 		return IRQ_NONE;
 
 	tty_wakeup(tty);
+	tty_kref_put(tty);
 	return IRQ_HANDLED;
 }
 
+static const struct tty_port_operations line_port_ops;
+
 int line_setup_irq(int fd, int input, int output, struct line *line, void *data)
 {
 	const struct line_driver *driver = line->driver;
@@ -404,27 +334,27 @@ int line_setup_irq(int fd, int input, int output, struct line *line, void *data)
  * first open or last close.  Otherwise, open and close just return.
  */
 
-int line_open(struct line *lines, struct tty_struct *tty)
+int line_open(struct tty_struct *tty, struct file *filp)
 {
-	struct line *line = &lines[tty->index];
-	int err = -ENODEV;
+	struct line *line = tty->driver_data;
+	return tty_port_open(&line->port, tty, filp);
+}
 
-	spin_lock(&line->count_lock);
-	if (!line->valid)
-		goto out_unlock;
+int line_install(struct tty_driver *driver, struct tty_struct *tty, struct line *line)
+{
+	int ret = tty_init_termios(tty);
 
-	err = 0;
-	if (line->count++)
-		goto out_unlock;
+	if (ret)
+		return ret;
 
-	BUG_ON(tty->driver_data);
+	tty_driver_kref_get(driver);
+	tty->count++;
 	tty->driver_data = line;
-	line->tty = tty;
+	driver->ttys[tty->index] = tty;
 
-	spin_unlock(&line->count_lock);
-	err = enable_chan(line);
-	if (err) /* line_close() will be called by our caller */
-		return err;
+	ret = enable_chan(line);
+	if (ret)
+		return ret;
 
 	INIT_DELAYED_WORK(&line->task, line_timer_cb);
 
@@ -437,48 +367,36 @@ int line_open(struct line *lines, struct tty_struct *tty)
 			 &tty->winsize.ws_col);
 
 	return 0;
-
-out_unlock:
-	spin_unlock(&line->count_lock);
-	return err;
 }
 
 static void unregister_winch(struct tty_struct *tty);
 
-void line_close(struct tty_struct *tty, struct file * filp)
+void line_cleanup(struct tty_struct *tty)
 {
 	struct line *line = tty->driver_data;
 
-	/*
-	 * If line_open fails (and tty->driver_data is never set),
-	 * tty_open will call line_close.  So just return in this case.
-	 */
-	if (line == NULL)
-		return;
-
-	/* We ignore the error anyway! */
-	flush_buffer(line);
-
-	spin_lock(&line->count_lock);
-	BUG_ON(!line->valid);
-
-	if (--line->count)
-		goto out_unlock;
-
-	line->tty = NULL;
-	tty->driver_data = NULL;
-
-	spin_unlock(&line->count_lock);
-
 	if (line->sigio) {
 		unregister_winch(tty);
 		line->sigio = 0;
 	}
 
-	return;
+	tty->driver_data = NULL;
+}
+
+void line_close(struct tty_struct *tty, struct file * filp)
+{
+	struct line *line = tty->driver_data;
+
+	if (!line)
+		return;
+
+	tty_port_close(&line->port, tty, filp);
+}
 
-out_unlock:
-	spin_unlock(&line->count_lock);
+void line_hangup(struct tty_struct *tty)
+{
+	struct line *line = tty->driver_data;
+	tty_port_hangup(&line->port);
 }
 
 void close_lines(struct line *lines, int nlines)
@@ -495,13 +413,6 @@ static int setup_one_line(struct line *lines, int n, char *init, int init_prio,
 	struct line *line = &lines[n];
 	int err = -EINVAL;
 
-	spin_lock(&line->count_lock);
-
-	if (line->count) {
-		*error_out = "Device is already open";
-		goto out;
-	}
-
 	if (line->init_pri <= init_prio) {
 		line->init_pri = init_prio;
 		if (!strcmp(init, "none"))
@@ -512,8 +423,7 @@ static int setup_one_line(struct line *lines, int n, char *init, int init_prio,
 		}
 	}
 	err = 0;
-out:
-	spin_unlock(&line->count_lock);
+
 	return err;
 }
 
@@ -598,6 +508,7 @@ int line_get_config(char *name, struct line *lines, unsigned int num, char *str,
 	struct line *line;
 	char *end;
 	int dev, n = 0;
+	struct tty_struct *tty;
 
 	dev = simple_strtoul(name, &end, 0);
 	if ((*end != '\0') || (end == name)) {
@@ -612,13 +523,15 @@ int line_get_config(char *name, struct line *lines, unsigned int num, char *str,
 
 	line = &lines[dev];
 
-	spin_lock(&line->count_lock);
+	tty = tty_port_tty_get(&line->port);
+
 	if (!line->valid)
 		CONFIG_CHUNK(str, size, n, "none", 1);
-	else if (line->tty == NULL)
+	else if (tty == NULL)
 		CONFIG_CHUNK(str, size, n, line->init_str, 1);
 	else n = chan_config_string(&line->chan_list, str, size, error_out);
-	spin_unlock(&line->count_lock);
+
+	tty_kref_put(tty);
 
 	return n;
 }
@@ -678,8 +591,8 @@ struct tty_driver *register_lines(struct line_driver *line_driver,
 	}
 
 	for(i = 0; i < nlines; i++) {
-		if (!lines[i].valid)
-			tty_unregister_device(driver, i);
+		tty_port_init(&lines[i].port);
+		lines[i].port.ops = &line_port_ops;
 	}
 
 	mconsole_register_dev(&line_driver->mc);
@@ -805,7 +718,6 @@ void register_winch_irq(int fd, int tty_fd, int pid, struct tty_struct *tty,
 				   .pid  	= pid,
 				   .tty 	= tty,
 				   .stack	= stack });
-
 	if (um_request_irq(WINCH_IRQ, fd, IRQ_READ, winch_interrupt,
 			   IRQF_DISABLED | IRQF_SHARED | IRQF_SAMPLE_RANDOM,
 			   "winch", winch) < 0) {
diff --git a/arch/um/drivers/line.h b/arch/um/drivers/line.h
index 63df3ca..54adfc6 100644
--- a/arch/um/drivers/line.h
+++ b/arch/um/drivers/line.h
@@ -31,9 +31,8 @@ struct line_driver {
 };
 
 struct line {
-	struct tty_struct *tty;
-	spinlock_t count_lock;
-	unsigned long count;
+	struct tty_port port;
+
 	int valid;
 
 	char *init_str;
@@ -59,15 +58,17 @@ struct line {
 };
 
 #define LINE_INIT(str, d) \
-	{ .count_lock =	__SPIN_LOCK_UNLOCKED((str).count_lock), \
-	  .init_str =	str,	\
+	{ .init_str =	str,	\
 	  .init_pri =	INIT_STATIC, \
 	  .valid =	1, \
 	  .lock =	__SPIN_LOCK_UNLOCKED((str).lock), \
 	  .driver =	d }
 
 extern void line_close(struct tty_struct *tty, struct file * filp);
-extern int line_open(struct line *lines, struct tty_struct *tty);
+extern int line_open(struct tty_struct *tty, struct file *filp);
+extern int line_install(struct tty_driver *driver, struct tty_struct *tty, struct line *line);
+extern void line_cleanup(struct tty_struct *tty);
+extern void line_hangup(struct tty_struct *tty);
 extern int line_setup(struct line *lines, unsigned int sizeof_lines,
 		      char *init, char **error_out);
 extern int line_write(struct tty_struct *tty, const unsigned char *buf,
diff --git a/arch/um/drivers/ssl.c b/arch/um/drivers/ssl.c
index 9d8c20a..89e4e75 100644
--- a/arch/um/drivers/ssl.c
+++ b/arch/um/drivers/ssl.c
@@ -92,6 +92,7 @@ static int ssl_remove(int n, char **error_out)
 			   error_out);
 }
 
+#if 0
 static int ssl_open(struct tty_struct *tty, struct file *filp)
 {
 	int err = line_open(serial_lines, tty);
@@ -103,7 +104,6 @@ static int ssl_open(struct tty_struct *tty, struct file *filp)
 	return err;
 }
 
-#if 0
 static void ssl_flush_buffer(struct tty_struct *tty)
 {
 	return;
@@ -124,8 +124,16 @@ void ssl_hangup(struct tty_struct *tty)
 }
 #endif
 
+static int ssl_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+	if (tty->index < NR_PORTS)
+		return line_install(driver, tty, &serial_lines[tty->index]);
+	else
+		return -ENODEV;
+}
+
 static const struct tty_operations ssl_ops = {
-	.open 	 		= ssl_open,
+	.open 	 		= line_open,
 	.close 	 		= line_close,
 	.write 	 		= line_write,
 	.put_char 		= line_put_char,
@@ -134,9 +142,11 @@ static const struct tty_operations ssl_ops = {
 	.flush_buffer 		= line_flush_buffer,
 	.flush_chars 		= line_flush_chars,
 	.set_termios 		= line_set_termios,
-	.ioctl 	 		= line_ioctl,
 	.throttle 		= line_throttle,
 	.unthrottle 		= line_unthrottle,
+	.install		= ssl_install,
+	.cleanup		= line_cleanup,
+	.hangup			= line_hangup,
 #if 0
 	.stop 	 		= ssl_stop,
 	.start 	 		= ssl_start,
diff --git a/arch/um/drivers/stdio_console.c b/arch/um/drivers/stdio_console.c
index 088776f..014f3ee 100644
--- a/arch/um/drivers/stdio_console.c
+++ b/arch/um/drivers/stdio_console.c
@@ -97,7 +97,7 @@ static int con_remove(int n, char **error_out)
 
 static int con_open(struct tty_struct *tty, struct file *filp)
 {
-	int err = line_open(vts, tty);
+	int err = line_open(tty, filp);
 	if (err)
 		printk(KERN_ERR "Failed to open console %d, err = %d\n",
 		       tty->index, err);
@@ -105,6 +105,14 @@ static int con_open(struct tty_struct *tty, struct file *filp)
 	return err;
 }
 
+static int con_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+	if (tty->index < MAX_TTYS)
+		return line_install(driver, tty, &vts[tty->index]);
+	else
+		return -ENODEV;
+}
+
 /* Set in an initcall, checked in an exitcall */
 static int con_init_done = 0;
 
@@ -118,9 +126,11 @@ static const struct tty_operations console_ops = {
 	.flush_buffer 		= line_flush_buffer,
 	.flush_chars 		= line_flush_chars,
 	.set_termios 		= line_set_termios,
-	.ioctl 	 		= line_ioctl,
 	.throttle 		= line_throttle,
 	.unthrottle 		= line_unthrottle,
+	.cleanup		= line_cleanup,
+	.install		= con_install,
+	.hangup			= line_hangup,
 };
 
 static void uml_console_write(struct console *console, const char *string,
-- 
1.7.7.3


------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [uml-devel] [PATCH] um: Use tty_port
  2012-02-12  0:21 [uml-devel] (no subject) Richard Weinberger
@ 2012-02-12  0:24 ` Richard Weinberger
  2012-02-12 13:01   ` Jiri Slaby
  2012-02-12  0:25 ` your mail Jesper Juhl
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 26+ messages in thread
From: Richard Weinberger @ 2012-02-12  0:24 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: user-mode-linux-devel, gregkh, linux-kernel, viro, akpm, alan


[-- Attachment #1.1: Type: text/plain, Size: 14570 bytes --]

Whoops, I messed up the subject line.

Sorry!

Am 12.02.2012 01:21, schrieb Richard Weinberger:
> Can you please review this patch?
> 
> Thanks,
> //richard
> 
> ---
> From d8f5e7953def150bcc1e6a39dbbe589f1c68bcbd Mon Sep 17 00:00:00 2001
> From: Richard Weinberger <richard@nod.at>
> Date: Sun, 12 Feb 2012 01:12:49 +0100
> Subject: [PATCH] um: Use tty_port
> 
> UML's line driver has to use tty_port.
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  arch/um/drivers/line.c          |  212 +++++++++++---------------------------
>  arch/um/drivers/line.h          |   13 ++-
>  arch/um/drivers/ssl.c           |   16 +++-
>  arch/um/drivers/stdio_console.c |   14 ++-
>  4 files changed, 94 insertions(+), 161 deletions(-)
> 
> diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c
> index c1cf220..c789748 100644
> --- a/arch/um/drivers/line.c
> +++ b/arch/um/drivers/line.c
> @@ -19,19 +19,29 @@ static irqreturn_t line_interrupt(int irq, void *data)
>  {
>  	struct chan *chan = data;
>  	struct line *line = chan->line;
> +	struct tty_struct *tty;
> +
> +	if (line) {
> +		tty = tty_port_tty_get(&line->port);
> +		chan_interrupt(&line->chan_list, &line->task, tty, irq);
> +		tty_kref_put(tty);
> +	}
>  
> -	if (line)
> -		chan_interrupt(&line->chan_list, &line->task, line->tty, irq);
>  	return IRQ_HANDLED;
>  }
>  
>  static void line_timer_cb(struct work_struct *work)
>  {
>  	struct line *line = container_of(work, struct line, task.work);
> +	struct tty_struct *tty;
>  
> -	if (!line->throttled)
> -		chan_interrupt(&line->chan_list, &line->task, line->tty,
> +	if (!line->throttled) {
> +		tty = tty_port_tty_get(&line->port);
> +		chan_interrupt(&line->chan_list, &line->task, tty,
>  			       line->driver->read_irq);
> +
> +		tty_kref_put(tty);
> +	}
>  }
>  
>  /*
> @@ -228,92 +238,6 @@ void line_set_termios(struct tty_struct *tty, struct ktermios * old)
>  	/* nothing */
>  }
>  
> -static const struct {
> -	int  cmd;
> -	char *level;
> -	char *name;
> -} tty_ioctls[] = {
> -	/* don't print these, they flood the log ... */
> -	{ TCGETS,      NULL,       "TCGETS"      },
> -	{ TCSETS,      NULL,       "TCSETS"      },
> -	{ TCSETSW,     NULL,       "TCSETSW"     },
> -	{ TCFLSH,      NULL,       "TCFLSH"      },
> -	{ TCSBRK,      NULL,       "TCSBRK"      },
> -
> -	/* general tty stuff */
> -	{ TCSETSF,     KERN_DEBUG, "TCSETSF"     },
> -	{ TCGETA,      KERN_DEBUG, "TCGETA"      },
> -	{ TIOCMGET,    KERN_DEBUG, "TIOCMGET"    },
> -	{ TCSBRKP,     KERN_DEBUG, "TCSBRKP"     },
> -	{ TIOCMSET,    KERN_DEBUG, "TIOCMSET"    },
> -
> -	/* linux-specific ones */
> -	{ TIOCLINUX,   KERN_INFO,  "TIOCLINUX"   },
> -	{ KDGKBMODE,   KERN_INFO,  "KDGKBMODE"   },
> -	{ KDGKBTYPE,   KERN_INFO,  "KDGKBTYPE"   },
> -	{ KDSIGACCEPT, KERN_INFO,  "KDSIGACCEPT" },
> -};
> -
> -int line_ioctl(struct tty_struct *tty, unsigned int cmd,
> -				unsigned long arg)
> -{
> -	int ret;
> -	int i;
> -
> -	ret = 0;
> -	switch(cmd) {
> -#ifdef TIOCGETP
> -	case TIOCGETP:
> -	case TIOCSETP:
> -	case TIOCSETN:
> -#endif
> -#ifdef TIOCGETC
> -	case TIOCGETC:
> -	case TIOCSETC:
> -#endif
> -#ifdef TIOCGLTC
> -	case TIOCGLTC:
> -	case TIOCSLTC:
> -#endif
> -	/* Note: these are out of date as we now have TCGETS2 etc but this
> -	   whole lot should probably go away */
> -	case TCGETS:
> -	case TCSETSF:
> -	case TCSETSW:
> -	case TCSETS:
> -	case TCGETA:
> -	case TCSETAF:
> -	case TCSETAW:
> -	case TCSETA:
> -	case TCXONC:
> -	case TCFLSH:
> -	case TIOCOUTQ:
> -	case TIOCINQ:
> -	case TIOCGLCKTRMIOS:
> -	case TIOCSLCKTRMIOS:
> -	case TIOCPKT:
> -	case TIOCGSOFTCAR:
> -	case TIOCSSOFTCAR:
> -		return -ENOIOCTLCMD;
> -#if 0
> -	case TCwhatever:
> -		/* do something */
> -		break;
> -#endif
> -	default:
> -		for (i = 0; i < ARRAY_SIZE(tty_ioctls); i++)
> -			if (cmd == tty_ioctls[i].cmd)
> -				break;
> -		if (i == ARRAY_SIZE(tty_ioctls)) {
> -			printk(KERN_ERR "%s: %s: unknown ioctl: 0x%x\n",
> -			       __func__, tty->name, cmd);
> -		}
> -		ret = -ENOIOCTLCMD;
> -		break;
> -	}
> -	return ret;
> -}
> -
>  void line_throttle(struct tty_struct *tty)
>  {
>  	struct line *line = tty->driver_data;
> @@ -343,7 +267,7 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
>  {
>  	struct chan *chan = data;
>  	struct line *line = chan->line;
> -	struct tty_struct *tty = line->tty;
> +	struct tty_struct *tty = tty_port_tty_get(&line->port);
>  	int err;
>  
>  	/*
> @@ -354,6 +278,9 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
>  	spin_lock(&line->lock);
>  	err = flush_buffer(line);
>  	if (err == 0) {
> +		tty_kref_put(tty);
> +
> +		spin_unlock(&line->lock);
>  		return IRQ_NONE;
>  	} else if (err < 0) {
>  		line->head = line->buffer;
> @@ -365,9 +292,12 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
>  		return IRQ_NONE;
>  
>  	tty_wakeup(tty);
> +	tty_kref_put(tty);
>  	return IRQ_HANDLED;
>  }
>  
> +static const struct tty_port_operations line_port_ops;
> +
>  int line_setup_irq(int fd, int input, int output, struct line *line, void *data)
>  {
>  	const struct line_driver *driver = line->driver;
> @@ -404,27 +334,27 @@ int line_setup_irq(int fd, int input, int output, struct line *line, void *data)
>   * first open or last close.  Otherwise, open and close just return.
>   */
>  
> -int line_open(struct line *lines, struct tty_struct *tty)
> +int line_open(struct tty_struct *tty, struct file *filp)
>  {
> -	struct line *line = &lines[tty->index];
> -	int err = -ENODEV;
> +	struct line *line = tty->driver_data;
> +	return tty_port_open(&line->port, tty, filp);
> +}
>  
> -	spin_lock(&line->count_lock);
> -	if (!line->valid)
> -		goto out_unlock;
> +int line_install(struct tty_driver *driver, struct tty_struct *tty, struct line *line)
> +{
> +	int ret = tty_init_termios(tty);
>  
> -	err = 0;
> -	if (line->count++)
> -		goto out_unlock;
> +	if (ret)
> +		return ret;
>  
> -	BUG_ON(tty->driver_data);
> +	tty_driver_kref_get(driver);
> +	tty->count++;
>  	tty->driver_data = line;
> -	line->tty = tty;
> +	driver->ttys[tty->index] = tty;
>  
> -	spin_unlock(&line->count_lock);
> -	err = enable_chan(line);
> -	if (err) /* line_close() will be called by our caller */
> -		return err;
> +	ret = enable_chan(line);
> +	if (ret)
> +		return ret;
>  
>  	INIT_DELAYED_WORK(&line->task, line_timer_cb);
>  
> @@ -437,48 +367,36 @@ int line_open(struct line *lines, struct tty_struct *tty)
>  			 &tty->winsize.ws_col);
>  
>  	return 0;
> -
> -out_unlock:
> -	spin_unlock(&line->count_lock);
> -	return err;
>  }
>  
>  static void unregister_winch(struct tty_struct *tty);
>  
> -void line_close(struct tty_struct *tty, struct file * filp)
> +void line_cleanup(struct tty_struct *tty)
>  {
>  	struct line *line = tty->driver_data;
>  
> -	/*
> -	 * If line_open fails (and tty->driver_data is never set),
> -	 * tty_open will call line_close.  So just return in this case.
> -	 */
> -	if (line == NULL)
> -		return;
> -
> -	/* We ignore the error anyway! */
> -	flush_buffer(line);
> -
> -	spin_lock(&line->count_lock);
> -	BUG_ON(!line->valid);
> -
> -	if (--line->count)
> -		goto out_unlock;
> -
> -	line->tty = NULL;
> -	tty->driver_data = NULL;
> -
> -	spin_unlock(&line->count_lock);
> -
>  	if (line->sigio) {
>  		unregister_winch(tty);
>  		line->sigio = 0;
>  	}
>  
> -	return;
> +	tty->driver_data = NULL;
> +}
> +
> +void line_close(struct tty_struct *tty, struct file * filp)
> +{
> +	struct line *line = tty->driver_data;
> +
> +	if (!line)
> +		return;
> +
> +	tty_port_close(&line->port, tty, filp);
> +}
>  
> -out_unlock:
> -	spin_unlock(&line->count_lock);
> +void line_hangup(struct tty_struct *tty)
> +{
> +	struct line *line = tty->driver_data;
> +	tty_port_hangup(&line->port);
>  }
>  
>  void close_lines(struct line *lines, int nlines)
> @@ -495,13 +413,6 @@ static int setup_one_line(struct line *lines, int n, char *init, int init_prio,
>  	struct line *line = &lines[n];
>  	int err = -EINVAL;
>  
> -	spin_lock(&line->count_lock);
> -
> -	if (line->count) {
> -		*error_out = "Device is already open";
> -		goto out;
> -	}
> -
>  	if (line->init_pri <= init_prio) {
>  		line->init_pri = init_prio;
>  		if (!strcmp(init, "none"))
> @@ -512,8 +423,7 @@ static int setup_one_line(struct line *lines, int n, char *init, int init_prio,
>  		}
>  	}
>  	err = 0;
> -out:
> -	spin_unlock(&line->count_lock);
> +
>  	return err;
>  }
>  
> @@ -598,6 +508,7 @@ int line_get_config(char *name, struct line *lines, unsigned int num, char *str,
>  	struct line *line;
>  	char *end;
>  	int dev, n = 0;
> +	struct tty_struct *tty;
>  
>  	dev = simple_strtoul(name, &end, 0);
>  	if ((*end != '\0') || (end == name)) {
> @@ -612,13 +523,15 @@ int line_get_config(char *name, struct line *lines, unsigned int num, char *str,
>  
>  	line = &lines[dev];
>  
> -	spin_lock(&line->count_lock);
> +	tty = tty_port_tty_get(&line->port);
> +
>  	if (!line->valid)
>  		CONFIG_CHUNK(str, size, n, "none", 1);
> -	else if (line->tty == NULL)
> +	else if (tty == NULL)
>  		CONFIG_CHUNK(str, size, n, line->init_str, 1);
>  	else n = chan_config_string(&line->chan_list, str, size, error_out);
> -	spin_unlock(&line->count_lock);
> +
> +	tty_kref_put(tty);
>  
>  	return n;
>  }
> @@ -678,8 +591,8 @@ struct tty_driver *register_lines(struct line_driver *line_driver,
>  	}
>  
>  	for(i = 0; i < nlines; i++) {
> -		if (!lines[i].valid)
> -			tty_unregister_device(driver, i);
> +		tty_port_init(&lines[i].port);
> +		lines[i].port.ops = &line_port_ops;
>  	}
>  
>  	mconsole_register_dev(&line_driver->mc);
> @@ -805,7 +718,6 @@ void register_winch_irq(int fd, int tty_fd, int pid, struct tty_struct *tty,
>  				   .pid  	= pid,
>  				   .tty 	= tty,
>  				   .stack	= stack });
> -
>  	if (um_request_irq(WINCH_IRQ, fd, IRQ_READ, winch_interrupt,
>  			   IRQF_DISABLED | IRQF_SHARED | IRQF_SAMPLE_RANDOM,
>  			   "winch", winch) < 0) {
> diff --git a/arch/um/drivers/line.h b/arch/um/drivers/line.h
> index 63df3ca..54adfc6 100644
> --- a/arch/um/drivers/line.h
> +++ b/arch/um/drivers/line.h
> @@ -31,9 +31,8 @@ struct line_driver {
>  };
>  
>  struct line {
> -	struct tty_struct *tty;
> -	spinlock_t count_lock;
> -	unsigned long count;
> +	struct tty_port port;
> +
>  	int valid;
>  
>  	char *init_str;
> @@ -59,15 +58,17 @@ struct line {
>  };
>  
>  #define LINE_INIT(str, d) \
> -	{ .count_lock =	__SPIN_LOCK_UNLOCKED((str).count_lock), \
> -	  .init_str =	str,	\
> +	{ .init_str =	str,	\
>  	  .init_pri =	INIT_STATIC, \
>  	  .valid =	1, \
>  	  .lock =	__SPIN_LOCK_UNLOCKED((str).lock), \
>  	  .driver =	d }
>  
>  extern void line_close(struct tty_struct *tty, struct file * filp);
> -extern int line_open(struct line *lines, struct tty_struct *tty);
> +extern int line_open(struct tty_struct *tty, struct file *filp);
> +extern int line_install(struct tty_driver *driver, struct tty_struct *tty, struct line *line);
> +extern void line_cleanup(struct tty_struct *tty);
> +extern void line_hangup(struct tty_struct *tty);
>  extern int line_setup(struct line *lines, unsigned int sizeof_lines,
>  		      char *init, char **error_out);
>  extern int line_write(struct tty_struct *tty, const unsigned char *buf,
> diff --git a/arch/um/drivers/ssl.c b/arch/um/drivers/ssl.c
> index 9d8c20a..89e4e75 100644
> --- a/arch/um/drivers/ssl.c
> +++ b/arch/um/drivers/ssl.c
> @@ -92,6 +92,7 @@ static int ssl_remove(int n, char **error_out)
>  			   error_out);
>  }
>  
> +#if 0
>  static int ssl_open(struct tty_struct *tty, struct file *filp)
>  {
>  	int err = line_open(serial_lines, tty);
> @@ -103,7 +104,6 @@ static int ssl_open(struct tty_struct *tty, struct file *filp)
>  	return err;
>  }
>  
> -#if 0
>  static void ssl_flush_buffer(struct tty_struct *tty)
>  {
>  	return;
> @@ -124,8 +124,16 @@ void ssl_hangup(struct tty_struct *tty)
>  }
>  #endif
>  
> +static int ssl_install(struct tty_driver *driver, struct tty_struct *tty)
> +{
> +	if (tty->index < NR_PORTS)
> +		return line_install(driver, tty, &serial_lines[tty->index]);
> +	else
> +		return -ENODEV;
> +}
> +
>  static const struct tty_operations ssl_ops = {
> -	.open 	 		= ssl_open,
> +	.open 	 		= line_open,
>  	.close 	 		= line_close,
>  	.write 	 		= line_write,
>  	.put_char 		= line_put_char,
> @@ -134,9 +142,11 @@ static const struct tty_operations ssl_ops = {
>  	.flush_buffer 		= line_flush_buffer,
>  	.flush_chars 		= line_flush_chars,
>  	.set_termios 		= line_set_termios,
> -	.ioctl 	 		= line_ioctl,
>  	.throttle 		= line_throttle,
>  	.unthrottle 		= line_unthrottle,
> +	.install		= ssl_install,
> +	.cleanup		= line_cleanup,
> +	.hangup			= line_hangup,
>  #if 0
>  	.stop 	 		= ssl_stop,
>  	.start 	 		= ssl_start,
> diff --git a/arch/um/drivers/stdio_console.c b/arch/um/drivers/stdio_console.c
> index 088776f..014f3ee 100644
> --- a/arch/um/drivers/stdio_console.c
> +++ b/arch/um/drivers/stdio_console.c
> @@ -97,7 +97,7 @@ static int con_remove(int n, char **error_out)
>  
>  static int con_open(struct tty_struct *tty, struct file *filp)
>  {
> -	int err = line_open(vts, tty);
> +	int err = line_open(tty, filp);
>  	if (err)
>  		printk(KERN_ERR "Failed to open console %d, err = %d\n",
>  		       tty->index, err);
> @@ -105,6 +105,14 @@ static int con_open(struct tty_struct *tty, struct file *filp)
>  	return err;
>  }
>  
> +static int con_install(struct tty_driver *driver, struct tty_struct *tty)
> +{
> +	if (tty->index < MAX_TTYS)
> +		return line_install(driver, tty, &vts[tty->index]);
> +	else
> +		return -ENODEV;
> +}
> +
>  /* Set in an initcall, checked in an exitcall */
>  static int con_init_done = 0;
>  
> @@ -118,9 +126,11 @@ static const struct tty_operations console_ops = {
>  	.flush_buffer 		= line_flush_buffer,
>  	.flush_chars 		= line_flush_chars,
>  	.set_termios 		= line_set_termios,
> -	.ioctl 	 		= line_ioctl,
>  	.throttle 		= line_throttle,
>  	.unthrottle 		= line_unthrottle,
> +	.cleanup		= line_cleanup,
> +	.install		= con_install,
> +	.hangup			= line_hangup,
>  };
>  
>  static void uml_console_write(struct console *console, const char *string,



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 317 bytes --]

------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/

[-- Attachment #3: Type: text/plain, Size: 194 bytes --]

_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: your mail
  2012-02-12  0:21 [uml-devel] (no subject) Richard Weinberger
  2012-02-12  0:24 ` [uml-devel] [PATCH] um: Use tty_port Richard Weinberger
@ 2012-02-12  0:25 ` Jesper Juhl
  2012-02-12  1:02 ` [uml-devel] " Al Viro
  2012-02-12 19:11 ` Al Viro
  3 siblings, 0 replies; 26+ messages in thread
From: Jesper Juhl @ 2012-02-12  0:25 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: linux-kernel, user-mode-linux-devel, viro, akpm, alan, gregkh

On Sun, 12 Feb 2012, Richard Weinberger wrote:

> Can you please review this patch?
> 

A subject on the mail along with a description of the patch would make 
that a great deal easier...

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [uml-devel] your mail
  2012-02-12  0:21 [uml-devel] (no subject) Richard Weinberger
  2012-02-12  0:24 ` [uml-devel] [PATCH] um: Use tty_port Richard Weinberger
  2012-02-12  0:25 ` your mail Jesper Juhl
@ 2012-02-12  1:02 ` Al Viro
  2012-02-12 12:40   ` Jiri Slaby
  2012-02-12 19:11 ` Al Viro
  3 siblings, 1 reply; 26+ messages in thread
From: Al Viro @ 2012-02-12  1:02 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: alan, akpm, linux-kernel, user-mode-linux-devel, gregkh

On Sun, Feb 12, 2012 at 01:21:10AM +0100, Richard Weinberger wrote:

Not a full review by any means, but...

> +++ b/arch/um/drivers/line.c
> @@ -19,19 +19,29 @@ static irqreturn_t line_interrupt(int irq, void *data)
>  {
>  	struct chan *chan = data;
>  	struct line *line = chan->line;
> +	struct tty_struct *tty;
> +
> +	if (line) {
> +		tty = tty_port_tty_get(&line->port);
> +		chan_interrupt(&line->chan_list, &line->task, tty, irq);
> +		tty_kref_put(tty);
> +	}
>  
> -	if (line)
> -		chan_interrupt(&line->chan_list, &line->task, line->tty, irq);
>  	return IRQ_HANDLED;
>  }

Is tty_kref_put() safe in interrupt?  Here it seems to be OK, but in other
callers...  More or less at random: drivers/tty/serial/lantiq.c has it
called from lqasc_rx_int().  It seems to be possible to have it end up
calling ->ops->shutdown() and in this case that'd be lqasc_shutdown().
Which does a bunch of free_irq(), including the ->rx_irq, i.e. the one
we have it called from.  Alan?

> @@ -495,13 +413,6 @@ static int setup_one_line(struct line *lines, int n, char *init, int init_prio,
>  	struct line *line = &lines[n];
>  	int err = -EINVAL;
>  
> -	spin_lock(&line->count_lock);
> -
> -	if (line->count) {
> -		*error_out = "Device is already open";
> -		goto out;
> -	}

... and similar in line_open() - just what happens if you try to reconfigure
an opened one?

> @@ -612,13 +523,15 @@ int line_get_config(char *name, struct line *lines, unsigned int num, char *str,
>  
>  	line = &lines[dev];
>  
> -	spin_lock(&line->count_lock);
> +	tty = tty_port_tty_get(&line->port);
> +
>  	if (!line->valid)
>  		CONFIG_CHUNK(str, size, n, "none", 1);
> -	else if (line->tty == NULL)
> +	else if (tty == NULL)
>  		CONFIG_CHUNK(str, size, n, line->init_str, 1);
>  	else n = chan_config_string(&line->chan_list, str, size, error_out);
> -	spin_unlock(&line->count_lock);
> +
> +	tty_kref_put(tty);

again, where's the exclusion with config changes?

------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: your mail
  2012-02-12  1:02 ` [uml-devel] " Al Viro
@ 2012-02-12 12:40   ` Jiri Slaby
  2012-02-12 19:06     ` Al Viro
  0 siblings, 1 reply; 26+ messages in thread
From: Jiri Slaby @ 2012-02-12 12:40 UTC (permalink / raw)
  To: Al Viro
  Cc: Richard Weinberger, linux-kernel, user-mode-linux-devel, akpm,
	alan, gregkh, Jiri Slaby

On 02/12/2012 02:02 AM, Al Viro wrote:
> On Sun, Feb 12, 2012 at 01:21:10AM +0100, Richard Weinberger wrote:
>> +++ b/arch/um/drivers/line.c
>> @@ -19,19 +19,29 @@ static irqreturn_t line_interrupt(int irq, void *data)
>>  {
>>  	struct chan *chan = data;
>>  	struct line *line = chan->line;
>> +	struct tty_struct *tty;
>> +
>> +	if (line) {
>> +		tty = tty_port_tty_get(&line->port);
>> +		chan_interrupt(&line->chan_list, &line->task, tty, irq);
>> +		tty_kref_put(tty);
>> +	}
>>  
>> -	if (line)
>> -		chan_interrupt(&line->chan_list, &line->task, line->tty, irq);
>>  	return IRQ_HANDLED;
>>  }
> 
> Is tty_kref_put() safe in interrupt?  Here it seems to be OK, but in other
> callers...  More or less at random: drivers/tty/serial/lantiq.c has it
> called from lqasc_rx_int().  It seems to be possible to have it end up
> calling ->ops->shutdown() and in this case that'd be lqasc_shutdown().
> Which does a bunch of free_irq(), including the ->rx_irq, i.e. the one
> we have it called from.  Alan?

I'm not Alan, but will reply anyway. Yes, it is safe (unless the driver
does something tricky). In the driver you mention, this is uart_ops,
called from tty_port_operations' ->shutdown. And that's a different from
tty_operations' ->shutdown.

Yes, there are:
* tty->ops
* tty_port->ops
* uart_port->ops

uart_port->ops->shutdown is supposed to tear down interrupts like in
lantiq.c. It is called from tty_port->ops->shutdown. And that one is
allowed to be called only from user context (tty->ops->close and
tty->ops->hangup).

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] um: Use tty_port
  2012-02-12  0:24 ` [uml-devel] [PATCH] um: Use tty_port Richard Weinberger
@ 2012-02-12 13:01   ` Jiri Slaby
  2012-02-12 13:12     ` Richard Weinberger
  0 siblings, 1 reply; 26+ messages in thread
From: Jiri Slaby @ 2012-02-12 13:01 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: linux-kernel, user-mode-linux-devel, viro, akpm, alan, gregkh,
	Jiri Slaby

On 02/12/2012 01:24 AM, Richard Weinberger wrote:
>> @@ -228,92 +238,6 @@ void line_set_termios(struct tty_struct *tty, struct ktermios * old)
>>  	/* nothing */
>>  }
>>  
>> -static const struct {
>> -	int  cmd;
>> -	char *level;
>> -	char *name;
>> -} tty_ioctls[] = {
>> -	/* don't print these, they flood the log ... */
>> -	{ TCGETS,      NULL,       "TCGETS"      },
>> -	{ TCSETS,      NULL,       "TCSETS"      },
>> -	{ TCSETSW,     NULL,       "TCSETSW"     },
>> -	{ TCFLSH,      NULL,       "TCFLSH"      },
>> -	{ TCSBRK,      NULL,       "TCSBRK"      },
>> -
>> -	/* general tty stuff */
>> -	{ TCSETSF,     KERN_DEBUG, "TCSETSF"     },
>> -	{ TCGETA,      KERN_DEBUG, "TCGETA"      },
>> -	{ TIOCMGET,    KERN_DEBUG, "TIOCMGET"    },
>> -	{ TCSBRKP,     KERN_DEBUG, "TCSBRKP"     },
>> -	{ TIOCMSET,    KERN_DEBUG, "TIOCMSET"    },
>> -
>> -	/* linux-specific ones */
>> -	{ TIOCLINUX,   KERN_INFO,  "TIOCLINUX"   },
>> -	{ KDGKBMODE,   KERN_INFO,  "KDGKBMODE"   },
>> -	{ KDGKBTYPE,   KERN_INFO,  "KDGKBTYPE"   },
>> -	{ KDSIGACCEPT, KERN_INFO,  "KDSIGACCEPT" },
>> -};
>> -
>> -int line_ioctl(struct tty_struct *tty, unsigned int cmd,
>> -				unsigned long arg)
>> -{
>> -	int ret;
>> -	int i;
>> -
>> -	ret = 0;
>> -	switch(cmd) {
>> -#ifdef TIOCGETP
>> -	case TIOCGETP:
>> -	case TIOCSETP:
>> -	case TIOCSETN:
>> -#endif
>> -#ifdef TIOCGETC
>> -	case TIOCGETC:
>> -	case TIOCSETC:
>> -#endif
>> -#ifdef TIOCGLTC
>> -	case TIOCGLTC:
>> -	case TIOCSLTC:
>> -#endif
>> -	/* Note: these are out of date as we now have TCGETS2 etc but this
>> -	   whole lot should probably go away */
>> -	case TCGETS:
>> -	case TCSETSF:
>> -	case TCSETSW:
>> -	case TCSETS:
>> -	case TCGETA:
>> -	case TCSETAF:
>> -	case TCSETAW:
>> -	case TCSETA:
>> -	case TCXONC:
>> -	case TCFLSH:
>> -	case TIOCOUTQ:
>> -	case TIOCINQ:
>> -	case TIOCGLCKTRMIOS:
>> -	case TIOCSLCKTRMIOS:
>> -	case TIOCPKT:
>> -	case TIOCGSOFTCAR:
>> -	case TIOCSSOFTCAR:
>> -		return -ENOIOCTLCMD;
>> -#if 0
>> -	case TCwhatever:
>> -		/* do something */
>> -		break;
>> -#endif
>> -	default:
>> -		for (i = 0; i < ARRAY_SIZE(tty_ioctls); i++)
>> -			if (cmd == tty_ioctls[i].cmd)
>> -				break;
>> -		if (i == ARRAY_SIZE(tty_ioctls)) {
>> -			printk(KERN_ERR "%s: %s: unknown ioctl: 0x%x\n",
>> -			       __func__, tty->name, cmd);
>> -		}
>> -		ret = -ENOIOCTLCMD;
>> -		break;
>> -	}
>> -	return ret;
>> -}
>> -
>>  void line_throttle(struct tty_struct *tty)
>>  {
>>  	struct line *line = tty->driver_data;
>> @@ -343,7 +267,7 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
>>  {
>>  	struct chan *chan = data;
>>  	struct line *line = chan->line;
>> -	struct tty_struct *tty = line->tty;
>> +	struct tty_struct *tty = tty_port_tty_get(&line->port);
>>  	int err;
>>  
>>  	/*
>> @@ -354,6 +278,9 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
>>  	spin_lock(&line->lock);
>>  	err = flush_buffer(line);
>>  	if (err == 0) {
>> +		tty_kref_put(tty);
>> +
>> +		spin_unlock(&line->lock);

This and the ioctl change above fullfils the subject of the patch in no
way. Do this fix and the cleanup above separately, please.

>> @@ -404,27 +334,27 @@ int line_setup_irq(int fd, int input, int output, struct line *line, void *data)
>>   * first open or last close.  Otherwise, open and close just return.
>>   */
>>  
>> -int line_open(struct line *lines, struct tty_struct *tty)
>> +int line_open(struct tty_struct *tty, struct file *filp)
>>  {
>> -	struct line *line = &lines[tty->index];
>> -	int err = -ENODEV;
>> +	struct line *line = tty->driver_data;
>> +	return tty_port_open(&line->port, tty, filp);
>> +}
>>  
>> -	spin_lock(&line->count_lock);
>> -	if (!line->valid)
>> -		goto out_unlock;
>> +int line_install(struct tty_driver *driver, struct tty_struct *tty, struct line *line)

As it stands, it should be tty_port->ops->activate, not
tty->ops->install. Yes, you can still set driver_data and check for
multiple opens in install. Then use also tty_standard_install.

>> +{
>> +	int ret = tty_init_termios(tty);
>>  
>> -	err = 0;
>> -	if (line->count++)
>> -		goto out_unlock;
>> +	if (ret)
>> +		return ret;
>>  
>> -	BUG_ON(tty->driver_data);
>> +	tty_driver_kref_get(driver);
>> +	tty->count++;
>>  	tty->driver_data = line;
>> -	line->tty = tty;
>> +	driver->ttys[tty->index] = tty;
>>  
>> -	spin_unlock(&line->count_lock);
>> -	err = enable_chan(line);
>> -	if (err) /* line_close() will be called by our caller */
>> -		return err;
>> +	ret = enable_chan(line);
>> +	if (ret)
>> +		return ret;
>>  
>>  	INIT_DELAYED_WORK(&line->task, line_timer_cb);
>>  
>> @@ -437,48 +367,36 @@ int line_open(struct line *lines, struct tty_struct *tty)
>>  			 &tty->winsize.ws_col);
>>  
>>  	return 0;
>> -
>> -out_unlock:
>> -	spin_unlock(&line->count_lock);
>> -	return err;
>>  }
...
>> +void line_close(struct tty_struct *tty, struct file * filp)
>> +{
>> +	struct line *line = tty->driver_data;
>> +
>> +	if (!line)
>> +		return;

Unless you set tty->driver_data to NULL somewhere, this can never
happen. If you do, why -- this tends to be racy?

>> +	tty_port_close(&line->port, tty, filp);
>> +}
...
>> --- a/arch/um/drivers/ssl.c
>> +++ b/arch/um/drivers/ssl.c
>> @@ -92,6 +92,7 @@ static int ssl_remove(int n, char **error_out)
>>  			   error_out);
>>  }
>>  
>> +#if 0

Hmm, remove unused code instead.

>>  static int ssl_open(struct tty_struct *tty, struct file *filp)
>>  {
>>  	int err = line_open(serial_lines, tty);
...
>> @@ -124,8 +124,16 @@ void ssl_hangup(struct tty_struct *tty)
>>  }
>>  #endif
>>  
>> +static int ssl_install(struct tty_driver *driver, struct tty_struct *tty)
>> +{
>> +	if (tty->index < NR_PORTS)

Do you register more than NR_PORTS when allocating tty_driver? If not,
this test is always true. But presumably you won't need this hook anyway.

>> +		return line_install(driver, tty, &serial_lines[tty->index]);
>> +	else
>> +		return -ENODEV;
>> +}

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] um: Use tty_port
  2012-02-12 13:01   ` Jiri Slaby
@ 2012-02-12 13:12     ` Richard Weinberger
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Weinberger @ 2012-02-12 13:12 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: linux-kernel, user-mode-linux-devel, viro, akpm, alan, gregkh,
	Jiri Slaby

[-- Attachment #1: Type: text/plain, Size: 3330 bytes --]

Am 12.02.2012 14:01, schrieb Jiri Slaby:
> This and the ioctl change above fullfils the subject of the patch in no
> way. Do this fix and the cleanup above separately, please.

Fair point.

> 
>>> @@ -404,27 +334,27 @@ int line_setup_irq(int fd, int input, int output, struct line *line, void *data)
>>>   * first open or last close.  Otherwise, open and close just return.
>>>   */
>>>  
>>> -int line_open(struct line *lines, struct tty_struct *tty)
>>> +int line_open(struct tty_struct *tty, struct file *filp)
>>>  {
>>> -	struct line *line = &lines[tty->index];
>>> -	int err = -ENODEV;
>>> +	struct line *line = tty->driver_data;
>>> +	return tty_port_open(&line->port, tty, filp);
>>> +}
>>>  
>>> -	spin_lock(&line->count_lock);
>>> -	if (!line->valid)
>>> -		goto out_unlock;
>>> +int line_install(struct tty_driver *driver, struct tty_struct *tty, struct line *line)
> 
> As it stands, it should be tty_port->ops->activate, not
> tty->ops->install. Yes, you can still set driver_data and check for
> multiple opens in install. Then use also tty_standard_install.

Okay.

>>> +{
>>> +	int ret = tty_init_termios(tty);
>>>  
>>> -	err = 0;
>>> -	if (line->count++)
>>> -		goto out_unlock;
>>> +	if (ret)
>>> +		return ret;
>>>  
>>> -	BUG_ON(tty->driver_data);
>>> +	tty_driver_kref_get(driver);
>>> +	tty->count++;
>>>  	tty->driver_data = line;
>>> -	line->tty = tty;
>>> +	driver->ttys[tty->index] = tty;
>>>  
>>> -	spin_unlock(&line->count_lock);
>>> -	err = enable_chan(line);
>>> -	if (err) /* line_close() will be called by our caller */
>>> -		return err;
>>> +	ret = enable_chan(line);
>>> +	if (ret)
>>> +		return ret;
>>>  
>>>  	INIT_DELAYED_WORK(&line->task, line_timer_cb);
>>>  
>>> @@ -437,48 +367,36 @@ int line_open(struct line *lines, struct tty_struct *tty)
>>>  			 &tty->winsize.ws_col);
>>>  
>>>  	return 0;
>>> -
>>> -out_unlock:
>>> -	spin_unlock(&line->count_lock);
>>> -	return err;
>>>  }
> ...
>>> +void line_close(struct tty_struct *tty, struct file * filp)
>>> +{
>>> +	struct line *line = tty->driver_data;
>>> +
>>> +	if (!line)
>>> +		return;
> 
> Unless you set tty->driver_data to NULL somewhere, this can never
> happen. If you do, why -- this tends to be racy?

The old driver set tty->driver_data to NULL.
I guess we can remove it.

>>> +	tty_port_close(&line->port, tty, filp);
>>> +}
> ...
>>> --- a/arch/um/drivers/ssl.c
>>> +++ b/arch/um/drivers/ssl.c
>>> @@ -92,6 +92,7 @@ static int ssl_remove(int n, char **error_out)
>>>  			   error_out);
>>>  }
>>>  
>>> +#if 0
> 
> Hmm, remove unused code instead.

Will do.

>>>  static int ssl_open(struct tty_struct *tty, struct file *filp)
>>>  {
>>>  	int err = line_open(serial_lines, tty);
> ...
>>> @@ -124,8 +124,16 @@ void ssl_hangup(struct tty_struct *tty)
>>>  }
>>>  #endif
>>>  
>>> +static int ssl_install(struct tty_driver *driver, struct tty_struct *tty)
>>> +{
>>> +	if (tty->index < NR_PORTS)
> 
> Do you register more than NR_PORTS when allocating tty_driver? If not,
> this test is always true. But presumably you won't need this hook anyway.

Okay.

>>> +		return line_install(driver, tty, &serial_lines[tty->index]);
>>> +	else
>>> +		return -ENODEV;
>>> +}
> 
> thanks,

Thanks,
//richard


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: your mail
  2012-02-12 12:40   ` Jiri Slaby
@ 2012-02-12 19:06     ` Al Viro
  2012-02-13  9:40       ` Jiri Slaby
  0 siblings, 1 reply; 26+ messages in thread
From: Al Viro @ 2012-02-12 19:06 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Richard Weinberger, linux-kernel, user-mode-linux-devel, akpm,
	alan, gregkh, Jiri Slaby

On Sun, Feb 12, 2012 at 01:40:47PM +0100, Jiri Slaby wrote:
> > Is tty_kref_put() safe in interrupt?  Here it seems to be OK, but in other
> > callers...  More or less at random: drivers/tty/serial/lantiq.c has it
> > called from lqasc_rx_int().  It seems to be possible to have it end up
> > calling ->ops->shutdown() and in this case that'd be lqasc_shutdown().
> > Which does a bunch of free_irq(), including the ->rx_irq, i.e. the one
> > we have it called from.  Alan?
> 
> I'm not Alan, but will reply anyway. Yes, it is safe (unless the driver
> does something tricky). In the driver you mention, this is uart_ops,
> called from tty_port_operations' ->shutdown. And that's a different from
> tty_operations' ->shutdown.
> 
> Yes, there are:
> * tty->ops
> * tty_port->ops
> * uart_port->ops
> 
> uart_port->ops->shutdown is supposed to tear down interrupts like in
> lantiq.c. It is called from tty_port->ops->shutdown. And that one is
> allowed to be called only from user context (tty->ops->close and
> tty->ops->hangup).

Yecchhh...  If I'm reading (and grepping) it right, there are only two
non-default instance of tty_operations ->shutdown() - pty and vt ones.
Lovely...  And while we are at it, vt instance is definitely not safe
from interrupts - calls console_lock().  Not that it was relevant in
this case...

It's probably too late in this case, but I would've called that method
->sync_cleanup().  Assuming I'm not misreading its intent and history...

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: your mail
  2012-02-12  0:21 [uml-devel] (no subject) Richard Weinberger
                   ` (2 preceding siblings ...)
  2012-02-12  1:02 ` [uml-devel] " Al Viro
@ 2012-02-12 19:11 ` Al Viro
  2012-02-13  9:15   ` Jiri Slaby
  3 siblings, 1 reply; 26+ messages in thread
From: Al Viro @ 2012-02-12 19:11 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: linux-kernel, user-mode-linux-devel, akpm, alan, gregkh

On Sun, Feb 12, 2012 at 01:21:10AM +0100, Richard Weinberger wrote:

> @@ -343,7 +267,7 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
>  {
>  	struct chan *chan = data;
>  	struct line *line = chan->line;
> -	struct tty_struct *tty = line->tty;
> +	struct tty_struct *tty = tty_port_tty_get(&line->port);
>  	int err;
>  
>  	/*
> @@ -354,6 +278,9 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
>  	spin_lock(&line->lock);
>  	err = flush_buffer(line);
>  	if (err == 0) {
> +		tty_kref_put(tty);
> +
> +		spin_unlock(&line->lock);
>  		return IRQ_NONE;
>  	} else if (err < 0) {
>  		line->head = line->buffer;
> @@ -365,9 +292,12 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
>  		return IRQ_NONE;
>  
>  	tty_wakeup(tty);
> +	tty_kref_put(tty);
>  	return IRQ_HANDLED;
>  }

That, BTW, smells ugly.  Note that return before the last one has no
tty_kref_put() for a very good reason - it's under if (!tty).  And
just as line->tty, port->tty can become NULL, so tty_port_tty_get()
can, indeed, return NULL here.  Which makes the first tty_kref_put()
oopsable...

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: your mail
  2012-02-12 19:11 ` Al Viro
@ 2012-02-13  9:15   ` Jiri Slaby
  0 siblings, 0 replies; 26+ messages in thread
From: Jiri Slaby @ 2012-02-13  9:15 UTC (permalink / raw)
  To: Al Viro
  Cc: Richard Weinberger, linux-kernel, user-mode-linux-devel, akpm,
	alan, gregkh

On 02/12/2012 08:11 PM, Al Viro wrote:
> On Sun, Feb 12, 2012 at 01:21:10AM +0100, Richard Weinberger wrote:
> 
>> @@ -343,7 +267,7 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
>>  {
>>  	struct chan *chan = data;
>>  	struct line *line = chan->line;
>> -	struct tty_struct *tty = line->tty;
>> +	struct tty_struct *tty = tty_port_tty_get(&line->port);
>>  	int err;
>>  
>>  	/*
>> @@ -354,6 +278,9 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
>>  	spin_lock(&line->lock);
>>  	err = flush_buffer(line);
>>  	if (err == 0) {
>> +		tty_kref_put(tty);
>> +
>> +		spin_unlock(&line->lock);
>>  		return IRQ_NONE;
>>  	} else if (err < 0) {
>>  		line->head = line->buffer;
>> @@ -365,9 +292,12 @@ static irqreturn_t line_write_interrupt(int irq, void *data)
>>  		return IRQ_NONE;
>>  
>>  	tty_wakeup(tty);
>> +	tty_kref_put(tty);
>>  	return IRQ_HANDLED;
>>  }
> 
> That, BTW, smells ugly.  Note that return before the last one has no
> tty_kref_put() for a very good reason - it's under if (!tty).  And
> just as line->tty, port->tty can become NULL, so tty_port_tty_get()
> can, indeed, return NULL here.  Which makes the first tty_kref_put()
> oopsable...

Nope, it is allowed to call tty_kref_put(NULL).

regards,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: your mail
  2012-02-12 19:06     ` Al Viro
@ 2012-02-13  9:40       ` Jiri Slaby
  0 siblings, 0 replies; 26+ messages in thread
From: Jiri Slaby @ 2012-02-13  9:40 UTC (permalink / raw)
  To: Al Viro
  Cc: Jiri Slaby, Richard Weinberger, linux-kernel,
	user-mode-linux-devel, akpm, alan, gregkh

On 02/12/2012 08:06 PM, Al Viro wrote:
> Yecchhh...  If I'm reading (and grepping) it right, there are only two
> non-default instance of tty_operations ->shutdown() - pty and vt ones.
> Lovely...  And while we are at it, vt instance is definitely not safe
> from interrupts - calls console_lock().  Not that it was relevant in
> this case...

Thanks for looking into that. I was too lazy to do that on Sunday.

You're right that it may cause problems. Fortunately vt doesn't refcount
ttys. Hence con_shutdown can be called only from release_tty (close
path) in the user context.

Adding to my TODO list, unless somebody beats me to fix it.

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [uml-devel] (no subject)
@ 2016-01-25 22:23 Richard Weinberger
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Weinberger @ 2016-01-25 22:23 UTC (permalink / raw)
  To: linux-kernel; +Cc: user-mode-linux-devel

Last few months I was very busy and now I have the mess,
UML allmod/yesconfig does not build at all.
This large and boring patch series fixes a lot of drivers which
cannot work/build on UML.
Fixes are grouped by subsystem such that maintainers can review/pickup them.

Thanks,
//richard

[PATCH 01/22] power: reset: Fix dependencies for !HAS_IOMEM archs
[PATCH 02/22] phy: Fix dependencies for !HAS_IOMEM archs
[PATCH 03/22] scsi: Fix dependencies for !HAS_IOMEM and !HAS_DMA
[PATCH 04/22] staging: iio: Fix dependencies for !HAS_IOMEM archs
[PATCH 05/22] hw_random: Fix dependencies for !HAS_IOMEM archs
[PATCH 06/22] iio: adc: Fix dependencies for !HAS_IOMEM archs
[PATCH 07/22] fpga: Fix dependencies for !HAS_IOMEM archs
[PATCH 08/22] hwtracing: Fix dependencies for !HAS_IOMEM archs
[PATCH 09/22] leds: Fix dependencies for !HAS_IOMEM archs
[PATCH 10/22] mailbox: Fix dependencies for !HAS_IOMEM archs
[PATCH 11/22] mtd: Fix dependencies for !HAS_IOMEM archs
[PATCH 12/22] nvmem: Fix dependencies for !HAS_IOMEM archs
[PATCH 13/22] net: Fix dependencies for !HAS_IOMEM archs
[PATCH 14/22] pwm: Fix dependencies for !HAS_IOMEM archs
[PATCH 15/22] watchdog: Fix dependencies for !HAS_IOMEM archs
[PATCH 16/22] iio: imu: Fix dependencies for !HAS_IOMEM archs
[PATCH 17/22] media: Fix dependencies for !HAS_IOMEM archs
[PATCH 18/22] irqchip: Fix dependencies for !HAS_IOMEM archs
[PATCH 19/22] thermal: Fix dependencies for !HAS_IOMEM archs
[PATCH 20/22] clocksource: Fix dependencies for !HAS_IOMEM archs
[PATCH 21/22] mtd: cs553x: Fix dependencies for !HAS_IOMEM archs
[PATCH 22/22] um: Export pm_power_off

------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel


^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2016-01-25 22:24 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-12  0:21 [uml-devel] (no subject) Richard Weinberger
2012-02-12  0:24 ` [uml-devel] [PATCH] um: Use tty_port Richard Weinberger
2012-02-12 13:01   ` Jiri Slaby
2012-02-12 13:12     ` Richard Weinberger
2012-02-12  0:25 ` your mail Jesper Juhl
2012-02-12  1:02 ` [uml-devel] " Al Viro
2012-02-12 12:40   ` Jiri Slaby
2012-02-12 19:06     ` Al Viro
2012-02-13  9:40       ` Jiri Slaby
2012-02-12 19:11 ` Al Viro
2012-02-13  9:15   ` Jiri Slaby
  -- strict thread matches above, loose matches on Subject: below --
2016-01-25 22:23 [uml-devel] (no subject) Richard Weinberger
2009-03-30 18:41 [uml-devel] [patch 1/3] uml: fix compile error from net_device_ops conversion Miklos Szeredi
2009-03-30 18:42 ` [uml-devel] (no subject) Miklos Szeredi
2007-10-01 16:06 dhowells
2007-10-01 16:06 dhowells
2005-03-20 22:05 William Stearns
2004-09-27  9:20 Stroesser, Bodo
2004-09-29 17:33 ` BlaisorBlade
2004-09-20 19:55 Stroesser, Bodo
2004-09-25 17:34 ` BlaisorBlade
2003-10-03 14:51 stian
2003-10-06 21:25 ` Jeff Dike
2003-09-26  1:09 kooper
2003-10-06 21:22 ` Jeff Dike
2003-09-25  6:59 Jeff Chua
2003-09-25 16:33 ` Adam Heath

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).