Re: [PATCH] tty: hvc: wakeup hvc console immediately when needed
On 12. 04. 24, 5:38, li.ha...@zte.com.cn wrote:
From: Li Hao
Cancel the do_wakeup flag in hvc_struct, and change it to immediately
wake up tty when hp->n_outbuf is 0 in hvc_push().
When we receive a key input character, the interrupt handling function
hvc_handle_interrupt() will be executed, and the echo thread
flush_to_ldisc() will be added to the queue.
If the user is currently using tcsetattr(), a hang may occur. tcsetattr()
enters kernel and waits for hp->n_outbuf to become 0 via
tty_wait_until_sent(). If the echo thread finishes executing before
reaching tty_wait_until_sent (for example, put_chars() takes too long),
it will cause while meeting the wakeup condition (hp->do_wakeup = 1),
tty_wait_until_sent() cannot be woken up (missed the tty_wakeup() of
this round's tty_poll). Unless the next key input character comes,
hvc_poll will be executed, and tty_wakeup() will be performed through
the do_wakeup flag.
Signed-off-by: Li Hao
---
drivers/tty/hvc/hvc_console.c | 12 +---
drivers/tty/hvc/hvc_console.h | 1 -
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index cd1f657f7..2fa90d938 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -476,11 +476,13 @@ static void hvc_hangup(struct tty_struct *tty)
static int hvc_push(struct hvc_struct *hp)
{
int n;
+ struct tty_struct *tty;
n = hp->ops->put_chars(hp->vtermno, hp->outbuf, hp->n_outbuf);
+ tty = tty_port_tty_get(>port);
if (n <= 0) {
if (n == 0 || n == -EAGAIN) {
- hp->do_wakeup = 1;
+ tty_wakeup(tty);
What if tty is NULL? Did you intent to use tty_port_tty_wakeup() instead?
thanks,
--
js
suse labs
Re: [PATCH] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
On 08. 04. 24, 7:32, Jiri Slaby wrote: On 08. 04. 24, 7:29, Michael Ellerman wrote: Many maintainers won't drop Cc: tags if they are there in the submitted patch. So I agree with Andy that we should encourage folks not to add them in the first place. But fix the docs first. I am personally not biased to any variant (as in: I don't care where CCs live in a patch). OTOH, as a submitter, it's a major PITA to carry CCs in notes (to have those under the --- line). Esp. when I have patches in a queue for years. How do people handle that? (Like rebases on current kernel.) regards, -- js suse labs
Re: [PATCH] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
On 08. 04. 24, 7:29, Michael Ellerman wrote: Many maintainers won't drop Cc: tags if they are there in the submitted patch. So I agree with Andy that we should encourage folks not to add them in the first place. But fix the docs first. I am personally not biased to any variant (as in: I don't care where CCs live in a patch). regards, -- js suse labs
Re: [PATCH] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
On 04. 04. 24, 0:29, Andy Shevchenko wrote: Cc: Benjamin Herrenschmidt Cc: Michael Ellerman Cc: Nicholas Piggin Cc: Christophe Leroy Cc: "Aneesh Kumar K.V" Cc: "Naveen N. Rao" Cc: linux-m...@lists.linux-m68k.org Second, please move these Cc to be after the '---' line Sorry, but why? -- js suse labs
Re: kexec verbose dumps with 6.8 [was: [PATCH v4 1/7] kexec_file: add kexec_file flag to control debug printing]
Hi,
On 13. 03. 24, 1:48, Baoquan He wrote:
Hi Jiri,
On 03/12/24 at 10:58am, Jiri Slaby wrote:
On 13. 12. 23, 6:57, Baoquan He wrote:
... snip...
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
...
@@ -500,6 +500,13 @@ static inline int crash_hotplug_memory_support(void) {
return 0; }
static inline unsigned int crash_get_elfcorehdr_size(void) { return 0; }
#endif
+extern bool kexec_file_dbg_print;
+
+#define kexec_dprintk(fmt, ...)\
+ printk("%s" fmt, \
+ kexec_file_dbg_print ? KERN_INFO : KERN_DEBUG, \
+ ##__VA_ARGS__)
This means you dump it _always_. Only with different levels.
It dumped always too with pr_debug() before, I just add a switch to
control it's pr_info() or pr_debug().
Not really, see below.
And without any prefix whatsoever, so people see bloat like this in their
log now:
[ +0.01] 1000-0009 (1)
[ +0.02] 7f96d000-7f97efff (3)
[ +0.02] 0080-00807fff (4)
[ +0.01] 0080b000-0080bfff (4)
[ +0.02] 0081-008f (4)
[ +0.01] 7f97f000-7f9fefff (4)
[ +0.01] 7ff0-7fff (4)
[ +0.02] -0fff (2)
On which arch are you seeing this? There should be one line above these
range printing to tell what they are, like:
E820 memmap:
Ah this is there too. It's a lot of output, so I took it out of context,
apparently.
-0009a3ff (1)
0009a400-0009 (2)
000e-000f (2)
0010-6ff83fff (1)
6ff84000-7ac50fff (2)
It should all be prefixed like kdump: or kexec: in any way.
without actually knowing what that is.
There should be nothing logged if that is not asked for and especially if
kexec load went fine, right?
Right. Before this patch, those pr_debug() were already there. You need
enable them to print out like add '#define DEBUG' in *.c file, or enable
the dynamic debugging of the file or function.
I think it's perfectly fine for DEBUG builds to print this out. And many
(all major?) distros use dyndbg, so it used to print nothing by default.
With this patch applied,
you only need specify '-d' when you execute kexec command with
kexec_file load interface, like:
kexec -s -l -d /boot/vmlinuz-.img --initrd xxx.img --reuse-cmdline
Perhaps our (SUSE) tooling passes -d? But I am seeing this every time I
boot.
No, it does not seem so:
load.sh[915]: Starting kdump kernel load; kexec cmdline: /sbin/kexec -p
/var/lib/kdump/kernel --append=" loglevel=7 console=tty0 console=ttyS0
video=1920x1080,1024x768,800x600 oops=panic
lsm=lockdown,capability,integrity,selinux sysrq=yes reset_devices
acpi_no_memhotplug cgroup_disable=memory nokaslr numa=off irqpoll
nr_cpus=1 root=kdump rootflags=bind rd.udev.children-max=8
disable_cpu_apicid=0 panic=1" --initrd=/var/lib/kdump/initrd -a
For kexec_file load, it is not logging if not specifying '-d', unless
you take way to make pr_debug() work in that file.
So is -d detection malfunctioning under some circumstances?
Can this be redesigned, please?
Sure, after making clear what's going on with this, I will try.
Actually what was wrong on the pr_debug()s? Can you simply turn them on from
the kernel when -d is passed to kexec instead of all this?
Joe suggested this during v1 reviewing:
https://lore.kernel.org/all/1e7863ec4e4ab10b84fd0e64f30f8464d2e484a3.ca...@perches.com/T/#u
...
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -52,6 +52,8 @@ atomic_t __kexec_lock = ATOMIC_INIT(0);
/* Flag to indicate we are going to kexec a new kernel */
bool kexec_in_progress = false;
+bool kexec_file_dbg_print;
Ugh, and a global flag for this?
Yeah, kexec_file_dbg_print records if '-d' is specified when 'kexec'
command executed. Anything wrong with the global flag?
Global variables are frowned upon. To cite coding style: unless you
**really** need them. Here, it looks like you do not.
thanks,
--
js
suse labs
kexec verbose dumps with 6.8 [was: [PATCH v4 1/7] kexec_file: add kexec_file flag to control debug printing]
On 13. 12. 23, 6:57, Baoquan He wrote:
When specifying 'kexec -c -d', kexec_load interface will print loading
information, e.g the regions where kernel/initrd/purgatory/cmdline
are put, the memmap passed to 2nd kernel taken as system RAM ranges,
and printing all contents of struct kexec_segment, etc. These are
very helpful for analyzing or positioning what's happening when
kexec/kdump itself failed. The debugging printing for kexec_load
interface is made in user space utility kexec-tools.
Whereas, with kexec_file_load interface, 'kexec -s -d' print nothing.
Because kexec_file code is mostly implemented in kernel space, and the
debugging printing functionality is missed. It's not convenient when
debugging kexec/kdump loading and jumping with kexec_file_load
interface.
Now add KEXEC_FILE_DEBUG to kexec_file flag to control the debugging
message printing. And add global variable kexec_file_dbg_print and
macro kexec_dprintk() to facilitate the printing.
This is a preparation, later kexec_dprintk() will be used to replace the
existing pr_debug(). Once 'kexec -s -d' is specified, it will print out
kexec/kdump loading information. If '-d' is not specified, it regresses
to pr_debug().
Signed-off-by: Baoquan He
...
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
...
@@ -500,6 +500,13 @@ static inline int crash_hotplug_memory_support(void) {
return 0; }
static inline unsigned int crash_get_elfcorehdr_size(void) { return 0; }
#endif
+extern bool kexec_file_dbg_print;
+
+#define kexec_dprintk(fmt, ...)\
+ printk("%s" fmt, \
+ kexec_file_dbg_print ? KERN_INFO : KERN_DEBUG, \
+ ##__VA_ARGS__)
This means you dump it _always_. Only with different levels.
And without any prefix whatsoever, so people see bloat like this in
their log now:
[ +0.01] 1000-0009 (1)
[ +0.02] 7f96d000-7f97efff (3)
[ +0.02] 0080-00807fff (4)
[ +0.01] 0080b000-0080bfff (4)
[ +0.02] 0081-008f (4)
[ +0.01] 7f97f000-7f9fefff (4)
[ +0.01] 7ff0-7fff (4)
[ +0.02] -0fff (2)
without actually knowing what that is.
There should be nothing logged if that is not asked for and especially
if kexec load went fine, right?
Can this be redesigned, please?
Actually what was wrong on the pr_debug()s? Can you simply turn them on
from the kernel when -d is passed to kexec instead of all this?
...
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -52,6 +52,8 @@ atomic_t __kexec_lock = ATOMIC_INIT(0);
/* Flag to indicate we are going to kexec a new kernel */
bool kexec_in_progress = false;
+bool kexec_file_dbg_print;
Ugh, and a global flag for this?
thanks,
--
js
suse labs
Re: [PATCH] tty: hvc-iucv: fix function pointer casts
On 13. 02. 24, 11:17, Arnd Bergmann wrote:
From: Arnd Bergmann
clang warns about explicitly casting between incompatible function
pointers:
drivers/tty/hvc/hvc_iucv.c:1100:23: error: cast from 'void (*)(const void *)'
to 'void (*)(struct device *)' converts to incompatible function type
[-Werror,-Wcast-function-type-strict]
1100 | priv->dev->release = (void (*)(struct device *)) kfree;
| ^
Add a separate function to handle this correctly.
Signed-off-by: Arnd Bergmann
Reviewed-by: Jiri Slaby
diff --git a/drivers/tty/hvc/hvc_iucv.c b/drivers/tty/hvc/hvc_iucv.c
index fdecc0d63731..b1149bc62ca1 100644
--- a/drivers/tty/hvc/hvc_iucv.c
+++ b/drivers/tty/hvc/hvc_iucv.c
@@ -1035,6 +1035,10 @@ static const struct attribute_group
*hvc_iucv_dev_attr_groups[] = {
NULL,
};
+static void hvc_iucv_free(struct device *data)
+{
+ kfree(data);
+}
/**
* hvc_iucv_alloc() - Allocates a new struct hvc_iucv_private instance
@@ -1097,7 +1101,7 @@ static int __init hvc_iucv_alloc(int id, unsigned int
is_console)
priv->dev->bus = _bus;
priv->dev->parent = iucv_root;
priv->dev->groups = hvc_iucv_dev_attr_groups;
- priv->dev->release = (void (*)(struct device *)) kfree;
+ priv->dev->release = hvc_iucv_free;
rc = device_register(priv->dev);
if (rc) {
put_device(priv->dev);
--
js
suse labs
[PATCH 12/27] tty: hvc: convert to u8 and size_t
Switch character types to u8 and sizes to size_t. To conform to
characters/sizes in the rest of the tty layer.
Signed-off-by: Jiri Slaby (SUSE)
Cc: Michael Ellerman
Cc: Nicholas Piggin
Cc: Christophe Leroy
Cc: Amit Shah
Cc: Arnd Bergmann
Cc: Paul Walmsley
Cc: Palmer Dabbelt
Cc: Albert Ou
Cc: linuxppc-dev@lists.ozlabs.org
Cc: virtualizat...@lists.linux.dev
Cc: linux-ri...@lists.infradead.org
---
arch/powerpc/include/asm/hvconsole.h | 4 ++--
arch/powerpc/include/asm/hvsi.h| 18
arch/powerpc/include/asm/opal.h| 8 +---
arch/powerpc/platforms/powernv/opal.c | 14 +++--
arch/powerpc/platforms/pseries/hvconsole.c | 4 ++--
drivers/char/virtio_console.c | 10 -
drivers/tty/hvc/hvc_console.h | 4 ++--
drivers/tty/hvc/hvc_dcc.c | 24 +++---
drivers/tty/hvc/hvc_iucv.c | 18
drivers/tty/hvc/hvc_opal.c | 5 +++--
drivers/tty/hvc/hvc_riscv_sbi.c| 9
drivers/tty/hvc/hvc_rtas.c | 11 +-
drivers/tty/hvc/hvc_udbg.c | 9
drivers/tty/hvc/hvc_vio.c | 18
drivers/tty/hvc/hvc_xen.c | 23 +++--
drivers/tty/hvc/hvsi_lib.c | 20 ++
16 files changed, 107 insertions(+), 92 deletions(-)
diff --git a/arch/powerpc/include/asm/hvconsole.h
b/arch/powerpc/include/asm/hvconsole.h
index ccb2034506f0..d841a97010a0 100644
--- a/arch/powerpc/include/asm/hvconsole.h
+++ b/arch/powerpc/include/asm/hvconsole.h
@@ -21,8 +21,8 @@
* Vio firmware always attempts to fetch MAX_VIO_GET_CHARS chars. The 'count'
* parm is included to conform to put_chars() function pointer template
*/
-extern int hvc_get_chars(uint32_t vtermno, char *buf, int count);
-extern int hvc_put_chars(uint32_t vtermno, const char *buf, int count);
+extern ssize_t hvc_get_chars(uint32_t vtermno, u8 *buf, size_t count);
+extern ssize_t hvc_put_chars(uint32_t vtermno, const u8 *buf, size_t count);
/* Provided by HVC VIO */
void hvc_vio_init_early(void);
diff --git a/arch/powerpc/include/asm/hvsi.h b/arch/powerpc/include/asm/hvsi.h
index 464a7519ed64..9058edcb632b 100644
--- a/arch/powerpc/include/asm/hvsi.h
+++ b/arch/powerpc/include/asm/hvsi.h
@@ -64,7 +64,7 @@ struct hvsi_priv {
unsigned intinbuf_len; /* data in input buffer */
unsigned char inbuf[HVSI_INBUF_SIZE];
unsigned intinbuf_cur; /* Cursor in input buffer */
- unsigned intinbuf_pktlen; /* packet length from cursor */
+ size_t inbuf_pktlen; /* packet length from cursor */
atomic_tseqno; /* packet sequence number */
unsigned intopened:1; /* driver opened */
unsigned intestablished:1; /* protocol established */
@@ -72,24 +72,26 @@ struct hvsi_priv {
unsigned intmctrl_update:1; /* modem control updated */
unsigned short mctrl; /* modem control */
struct tty_struct *tty; /* tty structure */
- int (*get_chars)(uint32_t termno, char *buf, int count);
- int (*put_chars)(uint32_t termno, const char *buf, int count);
+ ssize_t (*get_chars)(uint32_t termno, u8 *buf, size_t count);
+ ssize_t (*put_chars)(uint32_t termno, const u8 *buf, size_t count);
uint32_ttermno;
};
/* hvsi lib functions */
struct hvc_struct;
extern void hvsilib_init(struct hvsi_priv *pv,
-int (*get_chars)(uint32_t termno, char *buf, int
count),
-int (*put_chars)(uint32_t termno, const char *buf,
- int count),
+ssize_t (*get_chars)(uint32_t termno, u8 *buf,
+ size_t count),
+ssize_t (*put_chars)(uint32_t termno, const u8 *buf,
+ size_t count),
int termno, int is_console);
extern int hvsilib_open(struct hvsi_priv *pv, struct hvc_struct *hp);
extern void hvsilib_close(struct hvsi_priv *pv, struct hvc_struct *hp);
extern int hvsilib_read_mctrl(struct hvsi_priv *pv);
extern int hvsilib_write_mctrl(struct hvsi_priv *pv, int dtr);
extern void hvsilib_establish(struct hvsi_priv *pv);
-extern int hvsilib_get_chars(struct hvsi_priv *pv, char *buf, int count);
-extern int hvsilib_put_chars(struct hvsi_priv *pv, const char *buf, int count);
+extern ssize_t hvsilib_get_chars(struct hvsi_priv *pv, u8 *buf, size_t count);
+extern ssize_t hvsilib_put_chars(struct hvsi_priv *pv, const u8 *buf,
+size_t count);
#endif /* _HVSI_H */
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index b66b0c615f4f..af304e6cb486 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc
[PATCH 10/27] tty: ehv_bytechan: convert to u8 and size_t
Switch character types to u8 and sizes to size_t. To conform to
characters/sizes in the rest of the tty layer.
Signed-off-by: Jiri Slaby (SUSE)
Cc: Laurentiu Tudor
Cc: linuxppc-dev@lists.ozlabs.org
---
drivers/tty/ehv_bytechan.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c
index cc9f4338da60..69508d7a4135 100644
--- a/drivers/tty/ehv_bytechan.c
+++ b/drivers/tty/ehv_bytechan.c
@@ -49,7 +49,7 @@ struct ehv_bc_data {
unsigned int tx_irq;
spinlock_t lock;/* lock for transmit buffer */
- unsigned char buf[BUF_SIZE];/* transmit circular buffer */
+ u8 buf[BUF_SIZE]; /* transmit circular buffer */
unsigned int head; /* circular buffer head */
unsigned int tail; /* circular buffer tail */
@@ -138,9 +138,9 @@ static int find_console_handle(void)
static unsigned int local_ev_byte_channel_send(unsigned int handle,
unsigned int *count,
- const char *p)
+ const u8 *p)
{
- char buffer[EV_BYTE_CHANNEL_MAX_BYTES];
+ u8 buffer[EV_BYTE_CHANNEL_MAX_BYTES];
unsigned int c = *count;
/*
@@ -166,7 +166,7 @@ static unsigned int local_ev_byte_channel_send(unsigned int
handle,
* has been sent, or if some error has occurred.
*
*/
-static void byte_channel_spin_send(const char data)
+static void byte_channel_spin_send(const u8 data)
{
int ret, count;
@@ -474,8 +474,7 @@ static ssize_t ehv_bc_tty_write(struct tty_struct *ttys,
const u8 *s,
{
struct ehv_bc_data *bc = ttys->driver_data;
unsigned long flags;
- unsigned int len;
- unsigned int written = 0;
+ size_t len, written = 0;
while (1) {
spin_lock_irqsave(>lock, flags);
--
2.43.0
Re: [PATCH 00/17] tty: small cleanups and fixes
On 23. 11. 23, 21:19, Greg KH wrote: On Tue, Nov 21, 2023 at 10:22:41AM +0100, Jiri Slaby (SUSE) wrote: This is a series to fix/clean up some obvious issues I revealed during u8+size_t conversions (to be posted later). I applied most of these except the last few, as I think you were going to reorder them, right? Yes, great. I will rebase and see/resend what is missing. thanks, -- js suse labs
[PATCH 11/17] tty: hvc_console: use flexible array for outbuf
This means:
* move outbuf to the end of struct hvc_struct and convert from pointer
to flexible array (the structure is smaller now)
* use struct_size() at the allocation site
* align outbuf in the struct instead of ALIGN() at kzalloc()
And apart from the above, use u8 instead of char (which are the same
thanks to -funsigned-char). The former is now preferred over the latter.
It makes the code easier to understand.
Signed-off-by: Jiri Slaby (SUSE)
Cc: linuxppc-dev@lists.ozlabs.org
---
drivers/tty/hvc/hvc_console.c | 4 +---
drivers/tty/hvc/hvc_console.h | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 959fae54ca39..93b613e1f176 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -922,8 +922,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
return ERR_PTR(err);
}
- hp = kzalloc(ALIGN(sizeof(*hp), sizeof(long)) + outbuf_size,
- GFP_KERNEL);
+ hp = kzalloc(struct_size(hp, outbuf, outbuf_size), GFP_KERNEL);
if (!hp)
return ERR_PTR(-ENOMEM);
@@ -931,7 +930,6 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
hp->data = data;
hp->ops = ops;
hp->outbuf_size = outbuf_size;
- hp->outbuf = &((char *)hp)[ALIGN(sizeof(*hp), sizeof(long))];
tty_port_init(>port);
hp->port.ops = _port_ops;
diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
index 9668f821db01..b718714bf399 100644
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -37,7 +37,6 @@ struct hvc_struct {
spinlock_t lock;
int index;
int do_wakeup;
- char *outbuf;
int outbuf_size;
int n_outbuf;
uint32_t vtermno;
@@ -48,6 +47,7 @@ struct hvc_struct {
struct work_struct tty_resize;
struct list_head next;
unsigned long flags;
+ u8 outbuf[] __aligned(sizeof(long));
};
/* implemented by a low level driver */
--
2.42.1
[PATCH 07/17] tty: ehv_bytecha: use memcpy_and_pad() in local_ev_byte_channel_send()
There is a helper for memcpy(buffer)+memset(the_rest). Use it for
simplicity.
And add a comment why we are doing the copy in the first place.
Signed-off-by: Jiri Slaby (SUSE)
Cc: Laurentiu Tudor
Cc: linuxppc-dev@lists.ozlabs.org
---
drivers/tty/ehv_bytechan.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c
index a067628e01c8..cc9f4338da60 100644
--- a/drivers/tty/ehv_bytechan.c
+++ b/drivers/tty/ehv_bytechan.c
@@ -143,9 +143,12 @@ static unsigned int local_ev_byte_channel_send(unsigned
int handle,
char buffer[EV_BYTE_CHANNEL_MAX_BYTES];
unsigned int c = *count;
+ /*
+* ev_byte_channel_send() expects at least EV_BYTE_CHANNEL_MAX_BYTES
+* (16 B) in the buffer. Fake it using a local buffer if needed.
+*/
if (c < sizeof(buffer)) {
- memcpy(buffer, p, c);
- memset([c], 0, sizeof(buffer) - c);
+ memcpy_and_pad(buffer, sizeof(buffer), p, c, 0);
p = buffer;
}
return ev_byte_channel_send(handle, count, p);
--
2.42.1
[PATCH 00/17] tty: small cleanups and fixes
This is a series to fix/clean up some obvious issues I revealed during u8+size_t conversions (to be posted later). Cc: "David S. Miller" Cc: Eric Dumazet Cc: Ivan Kokshaysky Cc: Jakub Kicinski Cc: Jan Kara Cc: Laurentiu Tudor Cc: linux-al...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: Matt Turner Cc: net...@vger.kernel.org Cc: Paolo Abeni Cc: Richard Henderson Jiri Slaby (SUSE) (17): tty: deprecate tty_write_message() tty: remove unneeded mbz from tiocsti() tty: fix tty_operations types in documentation tty: move locking docs out of Returns for functions in tty.h tty: amiserial: return from receive_chars() without goto tty: amiserial: use bool and rename overrun flag in receive_chars() tty: ehv_bytecha: use memcpy_and_pad() in local_ev_byte_channel_send() tty: goldfish: drop unneeded temporary variables tty: hso: don't emit load/unload info to the log tty: hso: don't initialize global serial_table tty: hvc_console: use flexible array for outbuf tty: nozomi: remove unused debugging DUMP() tty: srmcons: use 'buf' directly in srmcons_do_write() tty: srmcons: use 'count' directly in srmcons_do_write() tty: srmcons: make srmcons_do_write() return void tty: srmcons: switch need_cr to bool tty: srmcons: make 'str_cr' const and non-array arch/alpha/kernel/srmcons.c | 29 + drivers/net/usb/hso.c | 11 --- drivers/tty/amiserial.c | 10 -- drivers/tty/ehv_bytechan.c| 7 +-- drivers/tty/goldfish.c| 7 ++- drivers/tty/hvc/hvc_console.c | 4 +--- drivers/tty/hvc/hvc_console.h | 2 +- drivers/tty/nozomi.c | 18 -- drivers/tty/tty_io.c | 8 ++-- include/linux/tty.h | 12 +++- include/linux/tty_driver.h| 5 ++--- 11 files changed, 41 insertions(+), 72 deletions(-) -- 2.42.1
Re: [PATCH v4 4/5] tty: Add SBI debug console support to HVC SBI driver
On 18. 11. 23, 4:38, Anup Patel wrote:
diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
index 31f53fa77e4a..697c981221b5 100644
--- a/drivers/tty/hvc/hvc_riscv_sbi.c
+++ b/drivers/tty/hvc/hvc_riscv_sbi.c
...
-static int __init hvc_sbi_console_init(void)
+static int hvc_sbi_dbcn_tty_get(uint32_t vtermno, char *buf, int count)
{
- hvc_instantiate(0, 0, _sbi_ops);
+ phys_addr_t pa;
+
+ if (is_vmalloc_addr(buf)) {
I wonder, where does this buf come from, so that you have to check for
vmalloc?
+ pa = page_to_phys(vmalloc_to_page(buf)) + offset_in_page(buf);
+ if (PAGE_SIZE < (offset_in_page(buf) + count))
Am I the only one who would prefer:
if (count + offset_in_page(buf) > PAGE_SIZE)
?
+ count = PAGE_SIZE - offset_in_page(buf);
+ } else {
+ pa = __pa(buf);
+ }
+
+ return sbi_debug_console_read(count, pa);
+}
thanks,
--
js
suse labs
Re: [PATCH v2 04/15] tty: Remove now superfluous sentinel element from ctl_table array
On 02. 10. 23, 10:55, Joel Granados via B4 Relay wrote: From: Joel Granados This commit comes at the tail end of a greater effort to remove the empty elements at the end of the ctl_table arrays (sentinels) which will reduce the overall build time size of the kernel and run time memory bloat by ~64 bytes per sentinel (further information Link : https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/) Remove sentinel from tty_table Signed-off-by: Joel Granados Reviewed-by: Jiri Slaby thanks, -- js suse labs
Re: [PATCH 04/15] tty: Remove now superfluous sentinel element from ctl_table array
On 28. 09. 23, 15:21, Joel Granados via B4 Relay wrote:
From: Joel Granados
This commit comes at the tail end of a greater effort to remove the
empty elements at the end of the ctl_table arrays (sentinels) which
will reduce the overall build time size of the kernel and run time
memory bloat by ~64 bytes per sentinel (further information Link :
https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/)
Remove sentinel from tty_table
Signed-off-by: Joel Granados
---
drivers/tty/tty_io.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 8a94e5a43c6d..2f925dc54a20 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3607,8 +3607,7 @@ static struct ctl_table tty_table[] = {
.proc_handler = proc_dointvec,
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
- },
- { }
+ }
Why to remove the comma? One would need to add one when adding a new entry?
thanks,
--
js
suse labs
Re: [PATCH] tty: hvc: remove set but unused variable
On 08. 09. 23, 8:17, Bo Liu wrote: The local variable vdev in hvcs_destruct_port() is set but not used. Remove the variable and related code. Signed-off-by: Bo Liu Reviewed-by: Jiri Slaby -- js suse labs
[PATCH 32/36] tty: hvc: convert counts to size_t
Unify the type of tty_operations::write() counters with the 'count'
parameter. I.e. use size_t for them.
Signed-off-by: Jiri Slaby (SUSE)
Cc: linuxppc-dev@lists.ozlabs.org
---
drivers/tty/hvc/hvc_console.c | 2 +-
drivers/tty/hvc/hvcs.c| 6 +++---
drivers/tty/hvc/hvsi.c| 10 +-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index e93e8072ec86..959fae54ca39 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -500,7 +500,7 @@ static ssize_t hvc_write(struct tty_struct *tty, const u8
*buf, size_t count)
{
struct hvc_struct *hp = tty->driver_data;
unsigned long flags;
- int rsize, written = 0;
+ size_t rsize, written = 0;
/* This write was probably executed during a tty close. */
if (!hp)
diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
index 1de91fa23b04..d29fdfe9d93d 100644
--- a/drivers/tty/hvc/hvcs.c
+++ b/drivers/tty/hvc/hvcs.c
@@ -1263,8 +1263,8 @@ static ssize_t hvcs_write(struct tty_struct *tty, const
u8 *buf, size_t count)
unsigned int unit_address;
const unsigned char *charbuf;
unsigned long flags;
- int total_sent = 0;
- int tosend = 0;
+ size_t total_sent = 0;
+ size_t tosend = 0;
int result = 0;
/*
@@ -1299,7 +1299,7 @@ static ssize_t hvcs_write(struct tty_struct *tty, const
u8 *buf, size_t count)
unit_address = hvcsd->vdev->unit_address;
while (count > 0) {
- tosend = min_t(unsigned, count,
+ tosend = min_t(size_t, count,
(HVCS_BUFF_LEN - hvcsd->chars_in_buffer));
/*
* No more space, this probably means that the last call to
diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index c57bd85aa488..a94068bce76f 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -909,8 +909,8 @@ static ssize_t hvsi_write(struct tty_struct *tty, const u8
*source,
{
struct hvsi_struct *hp = tty->driver_data;
unsigned long flags;
- int total = 0;
- int origcount = count;
+ size_t total = 0;
+ size_t origcount = count;
spin_lock_irqsave(>lock, flags);
@@ -928,7 +928,7 @@ static ssize_t hvsi_write(struct tty_struct *tty, const u8
*source,
* will see there is no room in outbuf and return.
*/
while ((count > 0) && (hvsi_write_room(tty) > 0)) {
- int chunksize = min_t(int, count, hvsi_write_room(tty));
+ size_t chunksize = min_t(size_t, count, hvsi_write_room(tty));
BUG_ON(hp->n_outbuf < 0);
memcpy(hp->outbuf + hp->n_outbuf, source, chunksize);
@@ -952,8 +952,8 @@ static ssize_t hvsi_write(struct tty_struct *tty, const u8
*source,
spin_unlock_irqrestore(>lock, flags);
if (total != origcount)
- pr_debug("%s: wanted %i, only wrote %i\n", __func__, origcount,
- total);
+ pr_debug("%s: wanted %zu, only wrote %zu\n", __func__,
+origcount, total);
return total;
}
--
2.41.0
[PATCH 03/10] tty: hvsi: remove an extra variable from hvsi_write()
'source' is the same as 'buf'. Rename the parameter ('buf') to
'source' and drop the local variable.
Likely, the two were introduced to have a different type. But 'char' and
'unsigned char' are the same in the kernel for a long time.
Signed-off-by: Jiri Slaby (SUSE)
Cc: linuxppc-dev@lists.ozlabs.org
---
drivers/tty/hvc/hvsi.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index a200d01eceed..c1b8a4fd8b1e 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -905,10 +905,9 @@ static unsigned int hvsi_chars_in_buffer(struct tty_struct
*tty)
}
static int hvsi_write(struct tty_struct *tty,
-const unsigned char *buf, int count)
+const unsigned char *source, int count)
{
struct hvsi_struct *hp = tty->driver_data;
- const char *source = buf;
unsigned long flags;
int total = 0;
int origcount = count;
--
2.41.0
Re: [PATCH v4 29/33] x86/mm: try VMA lock-based page fault handling first
Cc Jacob Young (from kernel bugzilla)
On 30. 06. 23, 19:40, Suren Baghdasaryan wrote:
On Fri, Jun 30, 2023 at 1:43 AM Jiri Slaby wrote:
On 30. 06. 23, 10:28, Jiri Slaby wrote:
> 2348
clone3({flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
child_tid=0x7fcaa5882990, parent_tid=0x7fcaa5882990, exit_signal=0,
stack=0x7fcaa5082000, stack_size=0x7ffe00, tls=0x7fcaa58826c0} =>
{parent_tid=[2351]}, 88) = 2351
> 2350 <... clone3 resumed> => {parent_tid=[2372]}, 88) = 2372
> 2351 <... clone3 resumed> => {parent_tid=[2354]}, 88) = 2354
> 2351 <... clone3 resumed> => {parent_tid=[2357]}, 88) = 2357
> 2354 <... clone3 resumed> => {parent_tid=[2355]}, 88) = 2355
> 2355 <... clone3 resumed> => {parent_tid=[2370]}, 88) = 2370
> 2370 mmap(NULL, 262144, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANONYMOUS, -1, 0
> 2370 <... mmap resumed>) = 0x7fca68249000
> 2372 <... clone3 resumed> => {parent_tid=[2384]}, 88) = 2384
> 2384 <... clone3 resumed> => {parent_tid=[2388]}, 88) = 2388
> 2388 <... clone3 resumed> => {parent_tid=[2392]}, 88) = 2392
> 2392 <... clone3 resumed> => {parent_tid=[2395]}, 88) = 2395
> 2395 write(2, "runtime: marked free object in s"..., 36
I.e. IIUC, all are threads (CLONE_VM) and thread 2370 mapped ANON
0x7fca68249000 - 0x7fca6827 and go in thread 2395 thinks for some
reason 0x7fca6824bec8 in that region is "bad".
Thanks for the analysis Jiri.
Is it possible from these logs to identify whether 2370 finished the
mmap operation before 2395 tried to access 0x7fca6824bec8? That access
has to happen only after mmap finishes mapping the region.
Hi,
it's hard to tell, but I assume so.
For now, forget about this go's overly complicated, hard to reproduce
case and concentrate on the very nice reduced testcase in:
https://bugzilla.kernel.org/show_bug.cgi?id=217624
;)
FWIW, I can reproduce using the test case too.
thanks,
--
js
suse labs
Re: [PATCH v4 29/33] x86/mm: try VMA lock-based page fault handling first
On 30. 06. 23, 10:28, Jiri Slaby wrote:
> 2348
clone3({flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID, child_tid=0x7fcaa5882990, parent_tid=0x7fcaa5882990, exit_signal=0, stack=0x7fcaa5082000, stack_size=0x7ffe00, tls=0x7fcaa58826c0} => {parent_tid=[2351]}, 88) = 2351
> 2350 <... clone3 resumed> => {parent_tid=[2372]}, 88) = 2372
> 2351 <... clone3 resumed> => {parent_tid=[2354]}, 88) = 2354
> 2351 <... clone3 resumed> => {parent_tid=[2357]}, 88) = 2357
> 2354 <... clone3 resumed> => {parent_tid=[2355]}, 88) = 2355
> 2355 <... clone3 resumed> => {parent_tid=[2370]}, 88) = 2370
> 2370 mmap(NULL, 262144, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANONYMOUS, -1, 0
> 2370 <... mmap resumed>) = 0x7fca68249000
> 2372 <... clone3 resumed> => {parent_tid=[2384]}, 88) = 2384
> 2384 <... clone3 resumed> => {parent_tid=[2388]}, 88) = 2388
> 2388 <... clone3 resumed> => {parent_tid=[2392]}, 88) = 2392
> 2392 <... clone3 resumed> => {parent_tid=[2395]}, 88) = 2395
> 2395 write(2, "runtime: marked free object in s"..., 36 ...>
I.e. IIUC, all are threads (CLONE_VM) and thread 2370 mapped ANON
0x7fca68249000 - 0x7fca6827 and go in thread 2395 thinks for some
reason 0x7fca6824bec8 in that region is "bad".
As I was noticed, this might be as well be a fail of the go's
inter-thread communication (or alike) too. It might now be only more
exposed with vma-based locks as we can do more parallelism now.
There are older hard to reproduce bugs in go with similar symptoms (we
see this error sometimes now too):
https://github.com/golang/go/issues/15246
Or this 2016 bug is a red herring. Hard to tell...
thanks,
--
js
suse labs
Re: [PATCH v4 29/33] x86/mm: try VMA lock-based page fault handling first
On 30. 06. 23, 8:35, Jiri Slaby wrote:
On 29. 06. 23, 17:30, Suren Baghdasaryan wrote:
On Thu, Jun 29, 2023 at 7:40 AM Jiri Slaby wrote:
Hi,
On 27. 02. 23, 18:36, Suren Baghdasaryan wrote:
Attempt VMA lock-based page fault handling first, and fall back to the
existing mmap_lock-based handling if that fails.
Signed-off-by: Suren Baghdasaryan
---
arch/x86/Kconfig | 1 +
arch/x86/mm/fault.c | 36
2 files changed, 37 insertions(+)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a825bf031f49..df21fba77db1 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -27,6 +27,7 @@ config X86_64
# Options that are inherently 64-bit kernel only:
select ARCH_HAS_GIGANTIC_PAGE
select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
+ select ARCH_SUPPORTS_PER_VMA_LOCK
select ARCH_USE_CMPXCHG_LOCKREF
select HAVE_ARCH_SOFT_DIRTY
select MODULES_USE_ELF_RELA
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index a498ae1fbe66..e4399983c50c 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -19,6 +19,7 @@
#include /*
faulthandler_disabled() */
#include /*
efi_crash_gracefully_on_page_fault()*/
#include
+#include /* find_and_lock_vma() */
#include /* boot_cpu_has,
... */
#include /* dotraplinkage,
... */
@@ -1333,6 +1334,38 @@ void do_user_addr_fault(struct pt_regs *regs,
}
#endif
+#ifdef CONFIG_PER_VMA_LOCK
+ if (!(flags & FAULT_FLAG_USER))
+ goto lock_mmap;
+
+ vma = lock_vma_under_rcu(mm, address);
+ if (!vma)
+ goto lock_mmap;
+
+ if (unlikely(access_error(error_code, vma))) {
+ vma_end_read(vma);
+ goto lock_mmap;
+ }
+ fault = handle_mm_fault(vma, address, flags |
FAULT_FLAG_VMA_LOCK, regs);
+ vma_end_read(vma);
+
+ if (!(fault & VM_FAULT_RETRY)) {
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
+ goto done;
+ }
+ count_vm_vma_lock_event(VMA_LOCK_RETRY);
This is apparently not strong enough as it causes go build failures
like:
[ 409s] strconv
[ 409s] releasep: m=0x579e2000 m->p=0x5781c600 p->m=0x0 p->status=2
[ 409s] fatal error: releasep: invalid p state
[ 409s]
[ 325s] hash/adler32
[ 325s] hash/crc32
[ 325s] cmd/internal/codesign
[ 336s] fatal error: runtime: out of memory
Hi Jiri,
Thanks for reporting! I'm not familiar with go builds. Could you
please explain the error to me or point me to some documentation to
decipher that error?
Sorry, we are on the same boat -- me neither. It only popped up in our
(openSUSE) build system and I only tracked it down by bisection. Let me
know if I can try something (like a patch or gathering some debug info).
FWIW, a failed build log:
https://decibel.fi.muni.cz/~xslaby/n/vma/log.txt
and a strace for it:
https://decibel.fi.muni.cz/~xslaby/n/vma/strace.txt
An excerpt from the log:
[ 55s] runtime: marked free object in span 0x7fca6824bec8,
elemsize=192 freeindex=0 (bad use of unsafe.Pointer? try -d=checkptr)
[ 55s] 0xc0002f2000 alloc marked
[ 55s] 0xc0002f20c0 alloc marked
[ 55s] 0xc0002f2180 alloc marked
[ 55s] 0xc0002f2240 free unmarked
[ 55s] 0xc0002f2300 alloc marked
[ 55s] 0xc0002f23c0 alloc marked
[ 55s] 0xc0002f2480 alloc marked
[ 55s] 0xc0002f2540 alloc marked
[ 55s] 0xc0002f2600 alloc marked
[ 55s] 0xc0002f26c0 alloc marked
[ 55s] 0xc0002f2780 alloc marked
[ 55s] 0xc0002f2840 alloc marked
[ 55s] 0xc0002f2900 alloc marked
[ 55s] 0xc0002f29c0 free unmarked
[ 55s] 0xc0002f2a80 alloc marked
[ 55s] 0xc0002f2b40 alloc marked
[ 55s] 0xc0002f2c00 alloc marked
[ 55s] 0xc0002f2cc0 alloc marked
[ 55s] 0xc0002f2d80 alloc marked
[ 55s] 0xc0002f2e40 alloc marked
[ 55s] 0xc0002f2f00 alloc marked
[ 55s] 0xc0002f2fc0 alloc marked
[ 55s] 0xc0002f3080 alloc marked
[ 55s] 0xc0002f3140 alloc marked
[ 55s] 0xc0002f3200 alloc marked
[ 55s] 0xc0002f32c0 alloc marked
[ 55s] 0xc0002f3380 free unmarked
[ 55s] 0xc0002f3440 free marked zombie
An excerpt from strace:
> 2348
clone3({flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
child_tid=0x7fcaa6a1b990, parent_tid=0x7fcaa6a1b990, exit_signal=0,
stack=0x7fcaa621b000, stack_size=0x7ffe00, tls=0x7fcaa6a1b6c0} =>
{parent_tid=[2350]}, 88) = 2350
> 2348
clone3({flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
child_tid=0x7fcaa5882990, parent_tid=0x7fcaa5882990, exit_signal=0,
stack=0x7fcaa5082000, stack_size=0x7ffe00, tls=0x7fcaa58826c0} =>
{parent_tid=[2351]}, 88) = 2351
> 2350 <... clone3 resumed> => {parent_tid=[2372]}, 88) = 2372
> 2351 <... clone3 resumed> => {paren
Re: [PATCH v4 29/33] x86/mm: try VMA lock-based page fault handling first
On 29. 06. 23, 17:30, Suren Baghdasaryan wrote:
On Thu, Jun 29, 2023 at 7:40 AM Jiri Slaby wrote:
Hi,
On 27. 02. 23, 18:36, Suren Baghdasaryan wrote:
Attempt VMA lock-based page fault handling first, and fall back to the
existing mmap_lock-based handling if that fails.
Signed-off-by: Suren Baghdasaryan
---
arch/x86/Kconfig| 1 +
arch/x86/mm/fault.c | 36
2 files changed, 37 insertions(+)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a825bf031f49..df21fba77db1 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -27,6 +27,7 @@ config X86_64
# Options that are inherently 64-bit kernel only:
select ARCH_HAS_GIGANTIC_PAGE
select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
+ select ARCH_SUPPORTS_PER_VMA_LOCK
select ARCH_USE_CMPXCHG_LOCKREF
select HAVE_ARCH_SOFT_DIRTY
select MODULES_USE_ELF_RELA
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index a498ae1fbe66..e4399983c50c 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -19,6 +19,7 @@
#include /* faulthandler_disabled() */
#include /*
efi_crash_gracefully_on_page_fault()*/
#include
+#include /* find_and_lock_vma() */
#include /* boot_cpu_has, ...*/
#include /* dotraplinkage, ...
*/
@@ -1333,6 +1334,38 @@ void do_user_addr_fault(struct pt_regs *regs,
}
#endif
+#ifdef CONFIG_PER_VMA_LOCK
+ if (!(flags & FAULT_FLAG_USER))
+ goto lock_mmap;
+
+ vma = lock_vma_under_rcu(mm, address);
+ if (!vma)
+ goto lock_mmap;
+
+ if (unlikely(access_error(error_code, vma))) {
+ vma_end_read(vma);
+ goto lock_mmap;
+ }
+ fault = handle_mm_fault(vma, address, flags | FAULT_FLAG_VMA_LOCK, regs);
+ vma_end_read(vma);
+
+ if (!(fault & VM_FAULT_RETRY)) {
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
+ goto done;
+ }
+ count_vm_vma_lock_event(VMA_LOCK_RETRY);
This is apparently not strong enough as it causes go build failures like:
[ 409s] strconv
[ 409s] releasep: m=0x579e2000 m->p=0x5781c600 p->m=0x0 p->status=2
[ 409s] fatal error: releasep: invalid p state
[ 409s]
[ 325s] hash/adler32
[ 325s] hash/crc32
[ 325s] cmd/internal/codesign
[ 336s] fatal error: runtime: out of memory
Hi Jiri,
Thanks for reporting! I'm not familiar with go builds. Could you
please explain the error to me or point me to some documentation to
decipher that error?
Sorry, we are on the same boat -- me neither. It only popped up in our
(openSUSE) build system and I only tracked it down by bisection. Let me
know if I can try something (like a patch or gathering some debug info).
thanks,
--
js
suse labs
Re: [PATCH v4 29/33] x86/mm: try VMA lock-based page fault handling first
Hi,
On 27. 02. 23, 18:36, Suren Baghdasaryan wrote:
Attempt VMA lock-based page fault handling first, and fall back to the
existing mmap_lock-based handling if that fails.
Signed-off-by: Suren Baghdasaryan
---
arch/x86/Kconfig| 1 +
arch/x86/mm/fault.c | 36
2 files changed, 37 insertions(+)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a825bf031f49..df21fba77db1 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -27,6 +27,7 @@ config X86_64
# Options that are inherently 64-bit kernel only:
select ARCH_HAS_GIGANTIC_PAGE
select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
+ select ARCH_SUPPORTS_PER_VMA_LOCK
select ARCH_USE_CMPXCHG_LOCKREF
select HAVE_ARCH_SOFT_DIRTY
select MODULES_USE_ELF_RELA
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index a498ae1fbe66..e4399983c50c 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -19,6 +19,7 @@
#include /* faulthandler_disabled() */
#include /*
efi_crash_gracefully_on_page_fault()*/
#include
+#include /* find_and_lock_vma() */
#include /* boot_cpu_has, ... */
#include /* dotraplinkage, ... */
@@ -1333,6 +1334,38 @@ void do_user_addr_fault(struct pt_regs *regs,
}
#endif
+#ifdef CONFIG_PER_VMA_LOCK
+ if (!(flags & FAULT_FLAG_USER))
+ goto lock_mmap;
+
+ vma = lock_vma_under_rcu(mm, address);
+ if (!vma)
+ goto lock_mmap;
+
+ if (unlikely(access_error(error_code, vma))) {
+ vma_end_read(vma);
+ goto lock_mmap;
+ }
+ fault = handle_mm_fault(vma, address, flags | FAULT_FLAG_VMA_LOCK,
regs);
+ vma_end_read(vma);
+
+ if (!(fault & VM_FAULT_RETRY)) {
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
+ goto done;
+ }
+ count_vm_vma_lock_event(VMA_LOCK_RETRY);
This is apparently not strong enough as it causes go build failures like:
[ 409s] strconv
[ 409s] releasep: m=0x579e2000 m->p=0x5781c600 p->m=0x0 p->status=2
[ 409s] fatal error: releasep: invalid p state
[ 409s]
[ 325s] hash/adler32
[ 325s] hash/crc32
[ 325s] cmd/internal/codesign
[ 336s] fatal error: runtime: out of memory
There are many kinds of similar errors. It happens in 1-3 out of 20
builds only.
If I revert the commit on top of 6.4, they all dismiss. Any idea?
The downstream report:
https://bugzilla.suse.com/show_bug.cgi?id=1212775
+
+ /* Quick path to respond to signals */
+ if (fault_signal_pending(fault, regs)) {
+ if (!user_mode(regs))
+ kernelmode_fixup_or_oops(regs, error_code, address,
+SIGBUS, BUS_ADRERR,
+ARCH_DEFAULT_PKEY);
+ return;
+ }
+lock_mmap:
+#endif /* CONFIG_PER_VMA_LOCK */
+
/*
* Kernel-mode access to the user address space should only occur
* on well-defined single instructions listed in the exception
@@ -1433,6 +1466,9 @@ void do_user_addr_fault(struct pt_regs *regs,
}
mmap_read_unlock(mm);
+#ifdef CONFIG_PER_VMA_LOCK
+done:
+#endif
if (likely(!(fault & VM_FAULT_ERROR)))
return;
thanks,
--
js
suse labs
Re: [PATCH v2 2/2] serial: cpm_uart: Fix a COMPILE_TEST dependency
On 23. 05. 23, 10:59, Herve Codina wrote:
In a COMPILE_TEST configuration, the cpm_uart driver uses symbols from
the cpm_uart_cpm2.c file. This file is compiled only when CONFIG_CPM2 is
set.
Without this dependency, the linker fails with some missing symbols for
COMPILE_TEST configuration that needs SERIAL_CPM without enabling CPM2.
This lead to:
depends on CPM2 || CPM1 || (PPC32 && CPM2 && COMPILE_TEST)
This dependency does not make sense anymore and can be simplified
removing all the COMPILE_TEST part.
Then it's the same as my revert:
https://lore.kernel.org/all/20230518055620.29957-1-jirisl...@kernel.org/
:D
But nevermind.
Signed-off-by: Herve Codina
Reported-by: kernel test robot
Link: https://lore.kernel.org/oe-kbuild-all/202305160221.9xgweobz-...@intel.com/
Fixes: e3e7b13bffae ("serial: allow COMPILE_TEST for some drivers")
---
drivers/tty/serial/Kconfig | 2 +-
drivers/tty/serial/cpm_uart/cpm_uart.h | 2 --
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 625358f44419..de092bc1289e 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -769,7 +769,7 @@ config SERIAL_PMACZILOG_CONSOLE
config SERIAL_CPM
tristate "CPM SCC/SMC serial port support"
- depends on CPM2 || CPM1 || (PPC32 && COMPILE_TEST)
+ depends on CPM2 || CPM1
select SERIAL_CORE
help
This driver supports the SCC and SMC serial ports on Motorola
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart.h
b/drivers/tty/serial/cpm_uart/cpm_uart.h
index 0577618e78c0..46c03ed71c31 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart.h
+++ b/drivers/tty/serial/cpm_uart/cpm_uart.h
@@ -19,8 +19,6 @@ struct gpio_desc;
#include "cpm_uart_cpm2.h"
#elif defined(CONFIG_CPM1)
#include "cpm_uart_cpm1.h"
-#elif defined(CONFIG_COMPILE_TEST)
-#include "cpm_uart_cpm2.h"
#endif
#define SERIAL_CPM_MAJOR 204
--
js
suse labs
Re: [PATCH 2/2] serial: cpm_uart: Fix a COMPILE_TEST dependency
On 22. 05. 23, 10:20, Herve Codina wrote:
In a COMPILE_TEST configuration, the cpm_uart driver uses symbols from
the cpm_uart_cpm2.c file. This file is compiled only when CONFIG_CPM2 is
set.
Without this dependency, the linker fails with some missing symbols for
COMPILE_TEST configuration that needs SERIAL_CPM without enabling CPM2.
Signed-off-by: Herve Codina
Reported-by: kernel test robot
Link: https://lore.kernel.org/oe-kbuild-all/202305160221.9xgweobz-...@intel.com/
Fixes: e3e7b13bffae ("serial: allow COMPILE_TEST for some drivers")
---
drivers/tty/serial/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 625358f44419..68a9d9db9144 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -769,7 +769,7 @@ config SERIAL_PMACZILOG_CONSOLE
config SERIAL_CPM
tristate "CPM SCC/SMC serial port support"
- depends on CPM2 || CPM1 || (PPC32 && COMPILE_TEST)
+ depends on CPM2 || CPM1 || (PPC32 && CPM2 && COMPILE_TEST)
Actually, does this makes sense? I mean, the last part after "||" is now
superfluous and doesn't help anything, right?
select SERIAL_CORE
help
This driver supports the SCC and SMC serial ports on Motorola
--
js
Re: [PATCH 0/2] Fix COMPILE_TEST dependencies for CPM uart, TSA and QMC
On 22. 05. 23, 10:20, Herve Codina wrote: This series fixes issues raised by the kernel test robot https://lore.kernel.org/oe-kbuild-all/202305160221.9xgweobz-...@intel.com/ In COMPILE_TEST configurations, TSA and QMC need CONFIG_CPM to be set in order to compile and CPM uart needs CONFIG_CPM2. Ah, perfect. Greg, please disregard my revert posted at: https://lore.kernel.org/all/20230518055620.29957-1-jirisl...@kernel.org/ and take these instead. Thanks. Best regards, Hervé Herve Codina (2): soc: fsl: cpm1: Fix TSA and QMC dependencies in case of COMPILE_TEST serial: cpm_uart: Fix a COMPILE_TEST dependency drivers/soc/fsl/qe/Kconfig | 4 ++-- drivers/tty/serial/Kconfig | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) -- js
Re: [PATCH v2 11/13] tty/serial: Call ->dtr_rts() parameter active consistently
On 10. 01. 23, 13:02, Ilpo Järvinen wrote: Convert various parameter names for ->dtr_rts() and related functions from onoff, on, and raise to active. Much better. Signed-off-by: Ilpo Järvinen Reviewed-by: Jiri Slaby -- js suse labs
Re: [PATCH net-next] Remove DECnet support from kernel
On 09. 01. 23, 9:14, Jan Engelhardt wrote:
On Monday 2023-01-09 08:04, Jiri Slaby wrote:
On 18. 08. 22, 2:43, Stephen Hemminger wrote:
DECnet is an obsolete network protocol
this breaks userspace. Some projects include linux/dn.h:
https://codesearch.debian.net/search?q=include.*linux%2Fdn.h=0
I found Trinity fails to build:
net/proto-decnet.c:5:10: fatal error: linux/dn.h: No such file or directory
5 | #include
Should we provide the above as empty files?
Not a good idea. There may be configure tests / code that merely checks for
dn.h existence without checking for specific contents/defines. If you provide
empty files, this would fail to build:
#include "config.h"
#ifdef HAVE_LINUX_DN_H
# include
#endif
int main() {
#ifdef HAVE_LINUX_DN_H
socket(AF_DECNET, 0, DNPROTO_NSP); // or whatever
#else
...
#endif
}
So, with my distro hat on, outright removing header files feels like the
slightly lesser of two evils. Given the task to port $arbitrary software
between operating systems, absent header files is something more or less
"regularly" encountered, so one could argue we are "trained" to deal with it.
But missing individual defines is a much deeper dive into the APIs and
software to patch it out.
Right, we used to keep providing also defines and structs in uapi
headers of removed functionality. So that the above socket would
compile, but fail during runtime.
I am not biased to any solution. In fact, I found out trinity was fixed
already. So either path networking takes, it's fine by me. I'm not sure
about the chromium users, though (and I don't care).
thanks,
--
js
suse labs
Re: [PATCH net-next] Remove DECnet support from kernel
On 18. 08. 22, 2:43, Stephen Hemminger wrote: DECnet is an obsolete network protocol that receives more attention from kernel janitors than users. It belongs in computer protocol history museum not in Linux kernel. It has been "Orphaned" in kernel since 2010. The iproute2 support for DECnet was dropped in 5.0 release. The documentation link on Sourceforge says it is abandoned there as well. Leave the UAPI alone to keep userspace programs compiling. This means that there is still an empty neighbour table for AF_DECNET. The table of /proc/sys/net entries was updated to match current directories and reformatted to be alphabetical. Signed-off-by: Stephen Hemminger Acked-by: David Ahern ... include/uapi/linux/dn.h | 149 - include/uapi/linux/netfilter_decnet.h | 72 - Hi, this breaks userspace. Some projects include linux/dn.h: https://codesearch.debian.net/search?q=include.*linux%2Fdn.h=0 I found Trinity fails to build: net/proto-decnet.c:5:10: fatal error: linux/dn.h: No such file or directory 5 | #include Should we provide the above as empty files? thanks, -- js suse labs
Re: [PATCH 07/10] tty: Convert ->dtr_rts() to take bool argument
On 04. 01. 23, 16:15, Ilpo Järvinen wrote:
Convert the raise/on parameter in ->dtr_rts() to bool through the
callchain. The parameter is used like bool. In USB serial, there
remains a few implicit bool -> larger type conversions because some
devices use u8 in their control messages.
Reviewed-by: Jiri Slaby
Signed-off-by: Ilpo Järvinen
---
...
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -378,7 +378,7 @@ static void async_mode(MGSLPC_INFO *info);
static void tx_timeout(struct timer_list *t);
static bool carrier_raised(struct tty_port *port);
-static void dtr_rts(struct tty_port *port, int onoff);
+static void dtr_rts(struct tty_port *port, bool onoff);
Not anything for this patch, but having this dubbed "onoff" instead of
"on" makes it really confusing.
--- a/drivers/mmc/core/sdio_uart.c
+++ b/drivers/mmc/core/sdio_uart.c
@@ -548,14 +548,14 @@ static bool uart_carrier_raised(struct tty_port *tport)
*adjusted during an open, close and hangup.
*/
-static void uart_dtr_rts(struct tty_port *tport, int onoff)
+static void uart_dtr_rts(struct tty_port *tport, bool onoff)
{
struct sdio_uart_port *port =
container_of(tport, struct sdio_uart_port, port);
int ret = sdio_uart_claim_func(port);
if (ret)
return;
- if (onoff == 0)
+ if (!onoff)
sdio_uart_clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
else
sdio_uart_set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
Especially here. What does "!onoff" mean? If it were:
if (on)
sdio_uart_set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
else
sdio_uart_clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
it would be a lot more clear.
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -1459,7 +1459,7 @@ static bool amiga_carrier_raised(struct tty_port *port)
return !(ciab.pra & SER_DCD);
}
-static void amiga_dtr_rts(struct tty_port *port, int raise)
+static void amiga_dtr_rts(struct tty_port *port, bool raise)
Or "raise". That makes sense too and we call it as such in
tty_port_operations:
--- a/include/linux/tty_port.h
+++ b/include/linux/tty_port.h
...
@@ -32,7 +32,7 @@ struct tty_struct;
*/
struct tty_port_operations {
bool (*carrier_raised)(struct tty_port *port);
- void (*dtr_rts)(struct tty_port *port, int raise);
+ void (*dtr_rts)(struct tty_port *port, bool raise);
void (*shutdown)(struct tty_port *port);
int (*activate)(struct tty_port *port, struct tty_struct *tty);
void (*destruct)(struct tty_port *port);
Care to fix that up too?
thanks,
--
js
suse labs
[PATCH 3/3] USB: sisusbvga: use module_usb_driver()
Now, that we only do usb_register() and usb_sisusb_exit() in
module_init() and module_exit() respectivelly, we can simply use
module_usb_driver().
Cc: Michael Ellerman
Cc: Nicholas Piggin
Cc: Christophe Leroy
Cc: Yoshinori Sato
Cc: Rich Felker
Cc: Thomas Winischhofer
Cc: Greg Kroah-Hartman
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Jiri Slaby (SUSE)
---
drivers/usb/misc/sisusbvga/sisusbvga.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/usb/misc/sisusbvga/sisusbvga.c
b/drivers/usb/misc/sisusbvga/sisusbvga.c
index a0d5ba8058f8..654a79fd3231 100644
--- a/drivers/usb/misc/sisusbvga/sisusbvga.c
+++ b/drivers/usb/misc/sisusbvga/sisusbvga.c
@@ -2947,18 +2947,7 @@ static struct usb_driver sisusb_driver = {
.id_table = sisusb_table,
};
-static int __init usb_sisusb_init(void)
-{
- return usb_register(_driver);
-}
-
-static void __exit usb_sisusb_exit(void)
-{
- usb_deregister(_driver);
-}
-
-module_init(usb_sisusb_init);
-module_exit(usb_sisusb_exit);
+module_usb_driver(sisusb_driver);
MODULE_AUTHOR("Thomas Winischhofer ");
MODULE_DESCRIPTION("sisusbvga - Driver for Net2280/SiS315-based USB2VGA
dongles");
--
2.38.1
[PATCH 2/3] USB: sisusbvga: rename sisusb.c to sisusbvga.c
As it's the only source for the sisusbvga module, there is no need for a
2-steps build.
Cc: Michael Ellerman
Cc: Nicholas Piggin
Cc: Christophe Leroy
Cc: Yoshinori Sato
Cc: Rich Felker
Cc: Thomas Winischhofer
Cc: Greg Kroah-Hartman
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Jiri Slaby (SUSE)
---
drivers/usb/misc/sisusbvga/Makefile | 2 --
drivers/usb/misc/sisusbvga/{sisusb.c => sisusbvga.c} | 0
2 files changed, 2 deletions(-)
rename drivers/usb/misc/sisusbvga/{sisusb.c => sisusbvga.c} (100%)
diff --git a/drivers/usb/misc/sisusbvga/Makefile
b/drivers/usb/misc/sisusbvga/Makefile
index 93265de80eb9..28aa1e6ef823 100644
--- a/drivers/usb/misc/sisusbvga/Makefile
+++ b/drivers/usb/misc/sisusbvga/Makefile
@@ -4,5 +4,3 @@
#
obj-$(CONFIG_USB_SISUSBVGA) += sisusbvga.o
-
-sisusbvga-y := sisusb.o
diff --git a/drivers/usb/misc/sisusbvga/sisusb.c
b/drivers/usb/misc/sisusbvga/sisusbvga.c
similarity index 100%
rename from drivers/usb/misc/sisusbvga/sisusb.c
rename to drivers/usb/misc/sisusbvga/sisusbvga.c
--
2.38.1
[PATCH 1/3] USB: sisusbvga: remove console support
It was marked as BROKEN since commit 862ee699fefe (USB: sisusbvga: Make console support depend on BROKEN) 2 years ago. Since noone stepped up to fix it, remove it completely. Cc: Michael Ellerman Cc: Nicholas Piggin Cc: Christophe Leroy Cc: Yoshinori Sato Cc: Rich Felker Cc: Thomas Winischhofer Cc: linuxppc-dev@lists.ozlabs.org Cc: linux...@vger.kernel.org Cc: linux-...@vger.kernel.org Signed-off-by: Jiri Slaby (SUSE) --- arch/powerpc/configs/ppc6xx_defconfig|1 - arch/sh/configs/landisk_defconfig|1 - drivers/usb/misc/sisusbvga/Kconfig | 34 - drivers/usb/misc/sisusbvga/Makefile |1 - drivers/usb/misc/sisusbvga/sisusb.c | 276 +--- drivers/usb/misc/sisusbvga/sisusb.h | 21 - drivers/usb/misc/sisusbvga/sisusb_con.c | 1496 -- drivers/usb/misc/sisusbvga/sisusb_init.c | 955 -- drivers/usb/misc/sisusbvga/sisusb_init.h | 180 --- 9 files changed, 6 insertions(+), 2959 deletions(-) delete mode 100644 drivers/usb/misc/sisusbvga/sisusb_con.c delete mode 100644 drivers/usb/misc/sisusbvga/sisusb_init.c delete mode 100644 drivers/usb/misc/sisusbvga/sisusb_init.h diff --git a/arch/powerpc/configs/ppc6xx_defconfig b/arch/powerpc/configs/ppc6xx_defconfig index 115d40be5481..110258277959 100644 --- a/arch/powerpc/configs/ppc6xx_defconfig +++ b/arch/powerpc/configs/ppc6xx_defconfig @@ -911,7 +911,6 @@ CONFIG_USB_IDMOUSE=m CONFIG_USB_FTDI_ELAN=m CONFIG_USB_APPLEDISPLAY=m CONFIG_USB_SISUSBVGA=m -CONFIG_USB_SISUSBVGA_CON=y CONFIG_USB_LD=m CONFIG_USB_TRANCEVIBRATOR=m CONFIG_USB_IOWARRIOR=m diff --git a/arch/sh/configs/landisk_defconfig b/arch/sh/configs/landisk_defconfig index 492a0a2e0e36..7037320b654a 100644 --- a/arch/sh/configs/landisk_defconfig +++ b/arch/sh/configs/landisk_defconfig @@ -92,7 +92,6 @@ CONFIG_USB_SERIAL_PL2303=m CONFIG_USB_EMI62=m CONFIG_USB_EMI26=m CONFIG_USB_SISUSBVGA=m -CONFIG_USB_SISUSBVGA_CON=y CONFIG_EXT2_FS=y CONFIG_EXT3_FS=y # CONFIG_EXT3_DEFAULTS_TO_ORDERED is not set diff --git a/drivers/usb/misc/sisusbvga/Kconfig b/drivers/usb/misc/sisusbvga/Kconfig index c12cdd015410..42f81c8eaa92 100644 --- a/drivers/usb/misc/sisusbvga/Kconfig +++ b/drivers/usb/misc/sisusbvga/Kconfig @@ -3,7 +3,6 @@ config USB_SISUSBVGA tristate "USB 2.0 SVGA dongle support (Net2280/SiS315)" depends on (USB_MUSB_HDRC || USB_EHCI_HCD) - select FONT_SUPPORT if USB_SISUSBVGA_CON help Say Y here if you intend to attach a USB2VGA dongle based on a Net2280 and a SiS315 chip. @@ -13,36 +12,3 @@ config USB_SISUSBVGA To compile this driver as a module, choose M here; the module will be called sisusbvga. If unsure, say N. - -config USB_SISUSBVGA_CON - bool "Text console and mode switching support" if USB_SISUSBVGA - depends on VT && BROKEN - select FONT_8x16 - help - Say Y here if you want a VGA text console via the USB dongle or - want to support userland applications that utilize the driver's - display mode switching capabilities. - - Note that this console supports VGA/EGA text mode only. - - By default, the console part of the driver will not kick in when - the driver is initialized. If you want the driver to take over - one or more of the consoles, you need to specify the number of - the first and last consoles (starting at 1) as driver parameters. - - For example, if the driver is compiled as a module: - -modprobe sisusbvga first=1 last=5 - - If you use hotplug, add this to your modutils config files with - the "options" keyword, such as eg. - -options sisusbvga first=1 last=5 - - If the driver is compiled into the kernel image, the parameters - must be given in the kernel command like, such as - -sisusbvga.first=1 sisusbvga.last=5 - - - diff --git a/drivers/usb/misc/sisusbvga/Makefile b/drivers/usb/misc/sisusbvga/Makefile index 6551bce68ac5..93265de80eb9 100644 --- a/drivers/usb/misc/sisusbvga/Makefile +++ b/drivers/usb/misc/sisusbvga/Makefile @@ -6,4 +6,3 @@ obj-$(CONFIG_USB_SISUSBVGA) += sisusbvga.o sisusbvga-y := sisusb.o -sisusbvga-$(CONFIG_USB_SISUSBVGA_CON) += sisusb_con.o sisusb_init.o diff --git a/drivers/usb/misc/sisusbvga/sisusb.c b/drivers/usb/misc/sisusbvga/sisusb.c index f08de33d9ff3..a0d5ba8058f8 100644 --- a/drivers/usb/misc/sisusbvga/sisusb.c +++ b/drivers/usb/misc/sisusbvga/sisusb.c @@ -51,25 +51,11 @@ #include #include "sisusb.h" -#include "sisusb_init.h" - -#ifdef CONFIG_USB_SISUSBVGA_CON -#include -#endif #define SISUSB_DONTSYNC /* Forward declarations / clean-up routines */ -#ifdef CONFIG_USB_SISUSBVGA_CON -static int sisusb_first_vc; -static int sisusb_last_vc; -module_param_named(first, sisusb_first_vc, int, 0); -module_param_named(last, sisusb_last_vc, int, 0); -MODULE
Re: [PATCH -next] tty: hvc: make hvc_rtas_dev static
On 19. 10. 22, 8:44, ruanjinjie wrote: The symbol is not used outside of the file, so mark it static. Fixes the following warning: drivers/tty/hvc/hvc_rtas.c:29:19: warning: symbol 'hvc_rtas_dev' was not declared. Should it be static? Reviewed-by: Jiri Slaby Signed-off-by: ruanjinjie --- drivers/tty/hvc/hvc_rtas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/hvc/hvc_rtas.c b/drivers/tty/hvc/hvc_rtas.c index e8b8c645482b..184d325abeed 100644 --- a/drivers/tty/hvc/hvc_rtas.c +++ b/drivers/tty/hvc/hvc_rtas.c @@ -26,7 +26,7 @@ #include "hvc_console.h" #define hvc_rtas_cookie 0x67781e15 -struct hvc_struct *hvc_rtas_dev; +static struct hvc_struct *hvc_rtas_dev; static int rtascons_put_char_token = RTAS_UNKNOWN_SERVICE; static int rtascons_get_char_token = RTAS_UNKNOWN_SERVICE; -- js suse labs
Re: [PATCH] tty: evh_bytechan: Replace NO_IRQ by 0
On 06. 10. 22, 7:20, Christophe Leroy wrote:
NO_IRQ is used to check the return of irq_of_parse_and_map().
On some architecture NO_IRQ is 0, on other architectures it is -1.
irq_of_parse_and_map() returns 0 on error, independent of NO_IRQ.
So use 0 instead of using NO_IRQ.
Signed-off-by: Christophe Leroy
Reviewed-by: Jiri Slaby
---
drivers/tty/ehv_bytechan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c
index 19d32cb6af84..8595483f4697 100644
--- a/drivers/tty/ehv_bytechan.c
+++ b/drivers/tty/ehv_bytechan.c
@@ -118,7 +118,7 @@ static int find_console_handle(void)
return 0;
stdout_irq = irq_of_parse_and_map(np, 0);
- if (stdout_irq == NO_IRQ) {
+ if (!stdout_irq) {
pr_err("ehv-bc: no 'interrupts' property in %pOF node\n", np);
return 0;
}
@@ -696,7 +696,7 @@ static int ehv_bc_tty_probe(struct platform_device *pdev)
bc->rx_irq = irq_of_parse_and_map(np, 0);
bc->tx_irq = irq_of_parse_and_map(np, 1);
- if ((bc->rx_irq == NO_IRQ) || (bc->tx_irq == NO_IRQ)) {
+ if (!bc->rx_irq || !bc->tx_irq) {
dev_err(>dev, "no 'interrupts' property in %pOFn node\n",
np);
ret = -ENODEV;
--
js
suse labs
Re: [PATCH 5/5] tty: hvc: remove HVC_IUCV_MAGIC
On 16. 09. 22, 3:55, наб wrote:
According to Greg, in the context of magic numbers as defined in
magic-number.rst, "the tty layer should not need this and I'll gladly
take patches"
This stretches that definition slightly, since it multiplexes it with
the terminal number as a constant offset, but is equivalent
Ref: https://lore.kernel.org/linux-doc/yymlovoskuchl...@kroah.com/
Signed-off-by: Ahelenia Ziemiańska
Acked-by: Jiri Slaby
---
drivers/tty/hvc/hvc_iucv.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/hvc/hvc_iucv.c b/drivers/tty/hvc/hvc_iucv.c
index 32366caca662..7d49a872de48 100644
--- a/drivers/tty/hvc/hvc_iucv.c
+++ b/drivers/tty/hvc/hvc_iucv.c
@@ -29,7 +29,6 @@
/* General device driver settings */
-#define HVC_IUCV_MAGIC 0xc9e4c3e5
#define MAX_HVC_IUCV_LINESHVC_ALLOC_TTY_ADAPTERS
#define MEMPOOL_MIN_NR(PAGE_SIZE / sizeof(struct
iucv_tty_buffer)/4)
@@ -131,9 +130,9 @@ static struct iucv_handler hvc_iucv_handler = {
*/
static struct hvc_iucv_private *hvc_iucv_get_private(uint32_t num)
{
- if ((num < HVC_IUCV_MAGIC) || (num - HVC_IUCV_MAGIC > hvc_iucv_devices))
+ if (num > hvc_iucv_devices)
return NULL;
- return hvc_iucv_table[num - HVC_IUCV_MAGIC];
+ return hvc_iucv_table[num];
}
/**
@@ -1072,8 +1071,8 @@ static int __init hvc_iucv_alloc(int id, unsigned int
is_console)
priv->is_console = is_console;
/* allocate hvc device */
- priv->hvc = hvc_alloc(HVC_IUCV_MAGIC + id, /*
PAGE_SIZE */
- HVC_IUCV_MAGIC + id, _iucv_ops, 256);
+ priv->hvc = hvc_alloc(id, /* PAGE_SIZE */
+ id, _iucv_ops, 256);
if (IS_ERR(priv->hvc)) {
rc = PTR_ERR(priv->hvc);
goto out_error_hvc;
@@ -1371,7 +1370,7 @@ static int __init hvc_iucv_init(void)
/* register the first terminal device as console
* (must be done before allocating hvc terminal devices) */
- rc = hvc_instantiate(HVC_IUCV_MAGIC, IUCV_HVC_CON_IDX, _iucv_ops);
+ rc = hvc_instantiate(0, IUCV_HVC_CON_IDX, _iucv_ops);
if (rc) {
pr_err("Registering HVC terminal device as "
"Linux console failed\n");
--
js
suse labs
Re: [PATCH] tty: move from strlcpy with unused retval to strscpy
On 18. 08. 22, 23:01, Wolfram Sang wrote:
Follow the advice of the below link and prefer 'strscpy' in this
subsystem. Conversion is 1:1 because the return value is not used.
Generated by a coccinelle script.
Reviewed-by: Jiri Slaby
Link:
https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=v6a6g1ouzcprm...@mail.gmail.com/
Signed-off-by: Wolfram Sang
---
drivers/tty/hvc/hvcs.c | 2 +-
drivers/tty/serial/earlycon.c| 6 +++---
drivers/tty/serial/serial_core.c | 2 +-
drivers/tty/serial/sunsu.c | 6 +++---
drivers/tty/serial/sunzilog.c| 6 +++---
5 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
index 9b7e8246a464..b79ce8d34f11 100644
--- a/drivers/tty/hvc/hvcs.c
+++ b/drivers/tty/hvc/hvcs.c
@@ -839,7 +839,7 @@ static void hvcs_set_pi(struct hvcs_partner_info *pi,
struct hvcs_struct *hvcsd)
hvcsd->p_partition_ID = pi->partition_ID;
/* copy the null-term char too */
- strlcpy(hvcsd->p_location_code, pi->location_code,
+ strscpy(hvcsd->p_location_code, pi->location_code,
sizeof(hvcsd->p_location_code));
}
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 88d08ba1ca83..a5f380584cda 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -67,7 +67,7 @@ static void __init earlycon_init(struct earlycon_device
*device,
if (*s)
earlycon->index = simple_strtoul(s, NULL, 10);
len = s - name;
- strlcpy(earlycon->name, name, min(len + 1, sizeof(earlycon->name)));
+ strscpy(earlycon->name, name, min(len + 1, sizeof(earlycon->name)));
earlycon->data = _console_dev;
}
@@ -123,7 +123,7 @@ static int __init parse_options(struct earlycon_device *device, char *options)
device->baud = simple_strtoul(options, NULL, 0);
length = min(strcspn(options, " ") + 1,
(size_t)(sizeof(device->options)));
- strlcpy(device->options, options, length);
+ strscpy(device->options, options, length);
}
return 0;
@@ -304,7 +304,7 @@ int __init of_setup_earlycon(const struct earlycon_id
*match,
if (options) {
early_console_dev.baud = simple_strtoul(options, NULL, 0);
- strlcpy(early_console_dev.options, options,
+ strscpy(early_console_dev.options, options,
sizeof(early_console_dev.options));
}
earlycon_init(_console_dev, match->name);
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 12c87cd201a7..3561a160cbd5 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2497,7 +2497,7 @@ uart_report_port(struct uart_driver *drv, struct
uart_port *port)
"MMIO 0x%llx", (unsigned long long)port->mapbase);
break;
default:
- strlcpy(address, "*unknown*", sizeof(address));
+ strscpy(address, "*unknown*", sizeof(address));
break;
}
diff --git a/drivers/tty/serial/sunsu.c b/drivers/tty/serial/sunsu.c
index 84d545e5a8c7..d5dcb612804e 100644
--- a/drivers/tty/serial/sunsu.c
+++ b/drivers/tty/serial/sunsu.c
@@ -1217,13 +1217,13 @@ static int sunsu_kbd_ms_init(struct uart_sunsu_port *up)
serio->id.type = SERIO_RS232;
if (up->su_type == SU_PORT_KBD) {
serio->id.proto = SERIO_SUNKBD;
- strlcpy(serio->name, "sukbd", sizeof(serio->name));
+ strscpy(serio->name, "sukbd", sizeof(serio->name));
} else {
serio->id.proto = SERIO_SUN;
serio->id.extra = 1;
- strlcpy(serio->name, "sums", sizeof(serio->name));
+ strscpy(serio->name, "sums", sizeof(serio->name));
}
- strlcpy(serio->phys,
+ strscpy(serio->phys,
(!(up->port.line & 1) ? "su/serio0" : "su/serio1"),
sizeof(serio->phys));
diff --git a/drivers/tty/serial/sunzilog.c b/drivers/tty/serial/sunzilog.c
index c14275d83b0b..c44cf613ff1a 100644
--- a/drivers/tty/serial/sunzilog.c
+++ b/drivers/tty/serial/sunzilog.c
@@ -1307,13 +1307,13 @@ static void sunzilog_register_serio(struct
uart_sunzilog_port *up)
serio->id.type = SERIO_RS232;
if (up->flags & SUNZILOG_FLAG_CONS_KEYB) {
serio->id.proto = SERIO_SUNKBD;
- strlcpy(serio->name, "zskbd", sizeof(serio->name));
+ strscpy(serio->name, "zskbd", sizeof(serio->name));
} else {
serio->id.proto = SERIO_SUN;
Re: [PATCH v2] tty/hvc_opal: simplify if-if to if-else
On 26. 04. 22, 9:10, Wan Jiabing wrote:
Use if and else instead of if(A) and if (!A).
Reviewed-by: Jiri Slaby
Signed-off-by: Wan Jiabing
---
Change log:
v2:
- add braces to the if block.
---
drivers/tty/hvc/hvc_opal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index 84776bc641e6..794c7b18aa06 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -342,9 +342,9 @@ void __init hvc_opal_init_early(void)
* path, so we hard wire it
*/
opal = of_find_node_by_path("/ibm,opal/consoles");
- if (opal)
+ if (opal) {
pr_devel("hvc_opal: Found consoles in new location\n");
- if (!opal) {
+ } else {
opal = of_find_node_by_path("/ibm,opal");
if (opal)
pr_devel("hvc_opal: "
--
js
suse labs
Re: [PATCH] tty/hvc_opal: simplify if-if to if-else
On 24. 04. 22, 11:25, Wan Jiabing wrote:
Use if and else instead of if(A) and if (!A).
Signed-off-by: Wan Jiabing
---
drivers/tty/hvc/hvc_opal.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index 84776bc641e6..2dafa0964c2a 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -344,14 +344,15 @@ void __init hvc_opal_init_early(void)
opal = of_find_node_by_path("/ibm,opal/consoles");
if (opal)
pr_devel("hvc_opal: Found consoles in new location\n");
- if (!opal) {
+ else {
This looks good, except missing braces as noted by Joe.
opal = of_find_node_by_path("/ibm,opal");
if (opal)
pr_devel("hvc_opal: "
"Found consoles in old location\n");
+ else
+ return;
I am not sure this return is more obvious than the previous one. Rather
the opposite, IMO.
}
- if (!opal)
- return;
+
for_each_child_of_node(opal, np) {
if (of_node_name_eq(np, "serial")) {
stdout_node = np;
thanks,
--
js
suse labs
Re: [PATCH] tty: serial: Prepare cleanup of powerpc's asm/prom.h
On 02. 04. 22, 12:20, Christophe Leroy wrote: powerpc's asm/prom.h brings some headers that it doesn't need itself. In order to clean it up, first add missing headers in users of asm/prom.h Signed-off-by: Christophe Leroy Reviewed-by: Jiri Slaby --- drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c | 1 - drivers/tty/serial/mpc52xx_uart.c | 2 ++ drivers/tty/serial/pmac_zilog.c | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c index 6a1cd03bfe39..108af254e8f3 100644 --- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c +++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c @@ -25,7 +25,6 @@ #include #include #include -#include #include #include diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c index 8a6958377764..4ec785e4f9b1 100644 --- a/drivers/tty/serial/mpc52xx_uart.c +++ b/drivers/tty/serial/mpc52xx_uart.c @@ -38,6 +38,8 @@ #include #include #include +#include +#include #include #include diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c index 5d97c201ad88..c903085acb8d 100644 --- a/drivers/tty/serial/pmac_zilog.c +++ b/drivers/tty/serial/pmac_zilog.c @@ -51,7 +51,6 @@ #include #ifdef CONFIG_PPC_PMAC -#include #include #include #include -- js suse labs
Re: [PATCH v2 3/3] vstatus: Display an informational message when the VSTATUS character is pressed or TIOCSTAT ioctl is called.
On 06. 02. 22, 16:48, Walt Drummond wrote: When triggered by pressing the VSTATUS key or calling the TIOCSTAT ioctl, the n_tty line discipline will display a message on the user's tty that provides basic information about the system and an 'interesting' process in the current foreground process group, eg: load: 0.58 cmd: sleep 744474 [sleeping] 0.36r 0.00u 0.00s 0% 772k The status message provides: - System load average - Command name and process id (from the perspective of the session) - Scheduler state - Total wall-clock run time - User space run time - System space run time - Percentage of on-cpu time - Resident set size The message is only displayed when the tty has the VSTATUS character set, the local flags ICANON and IEXTEN are enabled and NOKERNINFO is disabled; it is always displayed when TIOCSTAT is called regardless of tty settings. Signed-off-by: Walt Drummond --- It looks like my comments were addressed. However you did not document the chances since v1 here. IOW, [v2] tag missing here. And please add the CCs I added last time, so that relevant people still can comment. thanks, -- js suse labs
Re: [PATCH v12 1/2] tty: hvc: pass DMA capable memory to put_chars()
On 04. 11. 21, 14:06, Xianting Tian wrote: OTOH, you need c[N_OUTBUF] in the console case (hvc_console_print), but not whole hvc_struct. So cons_hvcs should be an array of structs composed of only the lock and the buffer. It is ok for me. = And I would do it even simpler now. One c[N_OUTBUF] buffer for all consoles and a single lock. And "char c" in struct hvc_struct. No need for the complex logic in hvc_console_print. So you will implement this? No, there is a slight difference between "I would" and "I will" :). I don't have anything to test this hvc change on… thanks, -- js suse labs
Re: [PATCH v12 1/2] tty: hvc: pass DMA capable memory to put_chars()
On 28. 10. 21, 17:09, Xianting Tian wrote:
As well known, hvc backend can register its opertions to hvc backend.
the operations contain put_chars(), get_chars() and so on.
Some hvc backend may do dma in its operations. eg, put_chars() of
virtio-console. But in the code of hvc framework, it may pass DMA
incapable memory to put_chars() under a specific configuration, which
is explained in commit c4baad5029(virtio-console: avoid DMA from stack):
1, c[] is on stack,
hvc_console_print():
char c[N_OUTBUF] __ALIGNED__;
cons_ops[index]->put_chars(vtermnos[index], c, i);
2, ch is on stack,
static void hvc_poll_put_char(,,char ch)
{
struct tty_struct *tty = driver->ttys[0];
struct hvc_struct *hp = tty->driver_data;
int n;
do {
n = hp->ops->put_chars(hp->vtermno, , 1);
} while (n <= 0);
}
Commit c4baad5029 is just the fix to avoid DMA from stack memory, which
is passed to virtio-console by hvc framework in above code. But I think
the fix is aggressive, it directly uses kmemdup() to alloc new buffer
from kmalloc area and do memcpy no matter the memory is in kmalloc area
or not. But most importantly, it should better be fixed in the hvc
framework, by changing it to never pass stack memory to the put_chars()
function in the first place. Otherwise, we still face the same issue if
a new hvc backend using dma added in the furture.
In this patch, add 'char cons_outbuf[]' as part of 'struct hvc_struct',
so hp->cons_outbuf is no longer the stack memory, we can use it in above
cases safely. We also add lock to protect cons_outbuf instead of using
the global lock of hvc.
Introduce array cons_hvcs[] for hvc pointers next to the cons_ops[] and
vtermnos[] arrays. With the array, we can easily find hvc's cons_outbuf
and its lock.
Hi,
this is still overly complicated IMO. As I already noted in:
https://lore.kernel.org/all/5b728c71-a754-b3ef-4ad3-6e33db1b7...@kernel.org/
this:
=
In fact, you need only a single char for the poll case
(hvc_poll_put_char), so hvc_struct needs to contain only c, not an array.
OTOH, you need c[N_OUTBUF] in the console case (hvc_console_print), but
not whole hvc_struct. So cons_hvcs should be an array of structs
composed of only the lock and the buffer.
=
And I would do it even simpler now. One c[N_OUTBUF] buffer for all
consoles and a single lock.
And "char c" in struct hvc_struct.
No need for the complex logic in hvc_console_print.
Introduce array cons_early_outbuf[] to ensure the mechanism of early
console still work normally.
thanks,
--
js
suse labs
Re: [PATCH v11 2/3] tty: hvc: pass DMA capable memory to put_chars()
On 15. 10. 21, 4:46, Xianting Tian wrote:
@@ -151,9 +142,11 @@ static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
static void hvc_console_print(struct console *co, const char *b,
unsigned count)
{
- char c[N_OUTBUF] __ALIGNED__;
+ char *c;
unsigned i = 0, n = 0;
int r, donecr = 0, index = co->index;
+ unsigned long flags;
+ struct hvc_struct *hp;
/* Console access attempt outside of acceptable console range. */
if (index >= MAX_NR_HVC_CONSOLES)
@@ -163,6 +156,13 @@ static void hvc_console_print(struct console *co, const
char *b,
if (vtermnos[index] == -1)
return;
+ hp = cons_hvcs[index];
+ if (!hp)
+ return;
You effectively make the console unusable until someone calls
hvc_alloc() for this device, correct? This doesn't look right. Neither
you describe this change of behaviour in the commit log.
regards,
--
js
suse labs
Re: [PATCH v7 1/2] tty: hvc: pass DMA capable memory to put_chars()
Hi,
On 17. 08. 21, 15:22, Xianting Tian wrote:
As well known, hvc backend can register its opertions to hvc backend.
the opertions contain put_chars(), get_chars() and so on.
"operations". And there too:
Some hvc backend may do dma in its opertions. eg, put_chars() of
virtio-console. But in the code of hvc framework, it may pass DMA
incapable memory to put_chars() under a specific configuration, which
is explained in commit c4baad5029(virtio-console: avoid DMA from stack):
1, c[] is on stack,
hvc_console_print():
char c[N_OUTBUF] __ALIGNED__;
cons_ops[index]->put_chars(vtermnos[index], c, i);
2, ch is on stack,
static void hvc_poll_put_char(,,char ch)
{
struct tty_struct *tty = driver->ttys[0];
struct hvc_struct *hp = tty->driver_data;
int n;
do {
n = hp->ops->put_chars(hp->vtermno, , 1);
} while (n <= 0);
}
Commit c4baad5029 is just the fix to avoid DMA from stack memory, which
is passed to virtio-console by hvc framework in above code. But I think
the fix is aggressive, it directly uses kmemdup() to alloc new buffer
from kmalloc area and do memcpy no matter the memory is in kmalloc area
or not. But most importantly, it should better be fixed in the hvc
framework, by changing it to never pass stack memory to the put_chars()
function in the first place. Otherwise, we still face the same issue if
a new hvc backend using dma added in the furture.
We make 'char c[N_OUTBUF]' part of 'struct hvc_struct', so hp->c is no
longer the stack memory. we can use it in above two cases.
In fact, you need only a single char for the poll case
(hvc_poll_put_char), so hvc_struct needs to contain only c, not an array.
OTOH, you need c[N_OUTBUF] in the console case (hvc_console_print), but
not whole hvc_struct. So cons_hvcs should be an array of structs
composed of only the lock and the buffer.
Hum.
Or maybe rethink and take care of the console case by kmemdup and be
done with that case? What problem do you have with allocating 16 bytes?
It should be quite easy and really fast (lockless) in most cases. On the
contrary, your solution has to take a spinlock to access the global buffer.
Other fix is use L1_CACHE_BYTES as the alignment, use 'sizeof(long)' as
dma alignment is wrong. And use struct_size() to calculate size of
hvc_struct.
This ought to be in separate patches.
thanks,
--
js
suse labs
Re: [PATCH v6 1/2] tty: hvc: pass DMA capable memory to put_chars()
Hi, On 12. 08. 21, 14:26, kernel test robot wrote: drivers/tty/hvc/hvc_console.c:190:26: warning: variable 'hp' is uninitialized when used here [-Wuninitialized] spin_unlock_irqrestore(>c_lock, flags); ^~ drivers/tty/hvc/hvc_console.c:149:23: note: initialize the variable 'hp' to silence this warning struct hvc_struct *hp; ^ = NULL So you clearly didn't test your change as it would crash quite instantly. I wonder, where do you intend to get hp from in the console::print() hook? thanks, -- js suse labs
Re: [PATCH v3 1/2] tty: hvc: pass DMA capable memory to put_chars()
On 05. 08. 21, 9:58, Jiri Slaby wrote: Hi, On 04. 08. 21, 4:54, Xianting Tian wrote: @@ -933,6 +949,16 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data, hp->outbuf_size = outbuf_size; hp->outbuf = &((char *)hp)[ALIGN(sizeof(*hp), sizeof(long))]; This deserves cleanup too. Why is "outbuf" not "char outbuf[0] __ALIGNED__" at the end of the structure? The allocation would be easier (using struct_size()) and this line would be gone completely. + /* + * hvc_con_outbuf is guaranteed to be aligned at least to the + * size(N_OUTBUF) by kmalloc(). + */ + hp->hvc_con_outbuf = kzalloc(N_OUTBUF, GFP_KERNEL); + if (!hp->hvc_con_outbuf) + return ERR_PTR(-ENOMEM); This leaks hp, right? Actually, why don't you make char c[N_OUTBUF] __ALIGNED__; part of struct hvc_struct directly? BTW your 2 patches are still not threaded, that is hard to follow. + + spin_lock_init(>hvc_con_lock); + tty_port_init(>port); hp->port.ops = _port_ops; thanks, -- js suse labs
Re: [PATCH v3 1/2] tty: hvc: pass DMA capable memory to put_chars()
Hi, On 04. 08. 21, 4:54, Xianting Tian wrote: @@ -933,6 +949,16 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data, hp->outbuf_size = outbuf_size; hp->outbuf = &((char *)hp)[ALIGN(sizeof(*hp), sizeof(long))]; + /* +* hvc_con_outbuf is guaranteed to be aligned at least to the +* size(N_OUTBUF) by kmalloc(). +*/ + hp->hvc_con_outbuf = kzalloc(N_OUTBUF, GFP_KERNEL); + if (!hp->hvc_con_outbuf) + return ERR_PTR(-ENOMEM); This leaks hp, right? BTW your 2 patches are still not threaded, that is hard to follow. + + spin_lock_init(>hvc_con_lock); + tty_port_init(>port); hp->port.ops = _port_ops; thanks, -- js suse labs
Re: [PATCH 2/2] virtio-console: remove unnecessary kmemdup()
On 02. 08. 21, 10:32, Xianting Tian wrote:
在 2021/8/2 下午3:25, Jiri Slaby 写道:
Hi,
why is this 2/2? I seem (Lore neither) to find 1/2.
You didn't receive 1/2?
[PATCH 1/2] tty: hvc: pass DMA capable memory to put_chars()
https://lkml.org/lkml/2021/8/1/8 <https://lkml.org/lkml/2021/8/1/8>
Oh, I did, but it's not properly threaded. PLease fix your setup.
On 01. 08. 21, 7:16, Xianting Tian wrote:
hvc framework will never pass stack memory to the put_chars() function,
Am I blind or missing something?
hvc_console_print(...)
{
char c[N_OUTBUF]
...
cons_ops[index]->put_chars(vtermnos[index], c, i);
The same here:
hvc_poll_put_char(..., char ch)
{
...
n = hp->ops->put_chars(hp->vtermno, , 1);
AFAICS both of them *pass* a pointer to stack variable.
yes, I discussed the issue with Arnd before in below thread, you can
get the history, thanks
https://lkml.org/lkml/2021/7/27/494 <https://lkml.org/lkml/2021/7/27/494>
So is this a v2? You should have noted that. And what changed from v1 too.
So the calling of kmemdup() is unnecessary, remove it.
Fixes: c4baad5029 ("virtio-console: avoid DMA from stack")
This patch doesn't "Fix" -- it reverts the commit. You should've CCed
the author too.
yes, we discussed ther issue in above thread, which we CCed the author.
I don't see any input from the author?
Anyway, 1/2 does not even build, so you will send v3 with all the above
fixed, hopefully.
thanks,
--
js
Re: [PATCH 2/2] virtio-console: remove unnecessary kmemdup()
Hi,
why is this 2/2? I seem (Lore neither) to find 1/2.
On 01. 08. 21, 7:16, Xianting Tian wrote:
hvc framework will never pass stack memory to the put_chars() function,
Am I blind or missing something?
hvc_console_print(...)
{
char c[N_OUTBUF]
...
cons_ops[index]->put_chars(vtermnos[index], c, i);
The same here:
hvc_poll_put_char(..., char ch)
{
...
n = hp->ops->put_chars(hp->vtermno, , 1);
AFAICS both of them *pass* a pointer to stack variable.
So the calling of kmemdup() is unnecessary, remove it.
Fixes: c4baad5029 ("virtio-console: avoid DMA from stack")
This patch doesn't "Fix" -- it reverts the commit. You should've CCed
the author too.
Signed-off-by: Xianting Tian
---
drivers/char/virtio_console.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 7eaf303a7..4ed3ffb1d 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1117,8 +1117,6 @@ static int put_chars(u32 vtermno, const char *buf, int
count)
{
struct port *port;
struct scatterlist sg[1];
- void *data;
- int ret;
if (unlikely(early_put_chars))
return early_put_chars(vtermno, buf, count);
@@ -1127,14 +1125,8 @@ static int put_chars(u32 vtermno, const char *buf, int
count)
if (!port)
return -EPIPE;
- data = kmemdup(buf, count, GFP_ATOMIC);
- if (!data)
- return -ENOMEM;
-
- sg_init_one(sg, data, count);
- ret = __send_to_port(port, sg, 1, count, data, false);
- kfree(data);
- return ret;
+ sg_init_one(sg, buf, count);
+ return __send_to_port(port, sg, 1, count, (void *)buf, false);
}
/*
--
js
suse labs
[PATCH 2/8] hvsi: don't panic on tty_register_driver failure
The alloc_tty_driver failure is handled gracefully in hvsi_init. But
tty_register_driver is not. panic is called if that one fails.
So handle the failure of tty_register_driver gracefully too. This will
keep at least the console functional as it was enabled earlier by
console_initcall in hvsi_console_init. Instead of shooting down the
whole system.
This means, we disable interrupts and restore hvsi_wait back to
poll_for_state().
Signed-off-by: Jiri Slaby
Cc: linuxppc-dev@lists.ozlabs.org
---
drivers/tty/hvc/hvsi.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index bfc15279d5bc..f0bc8e780051 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -1038,7 +1038,7 @@ static const struct tty_operations hvsi_ops = {
static int __init hvsi_init(void)
{
- int i;
+ int i, ret;
hvsi_driver = alloc_tty_driver(hvsi_count);
if (!hvsi_driver)
@@ -1069,12 +1069,25 @@ static int __init hvsi_init(void)
}
hvsi_wait = wait_for_state; /* irqs active now */
- if (tty_register_driver(hvsi_driver))
- panic("Couldn't register hvsi console driver\n");
+ ret = tty_register_driver(hvsi_driver);
+ if (ret) {
+ pr_err("Couldn't register hvsi console driver\n");
+ goto err_free_irq;
+ }
printk(KERN_DEBUG "HVSI: registered %i devices\n", hvsi_count);
return 0;
+err_free_irq:
+ hvsi_wait = poll_for_state;
+ for (i = 0; i < hvsi_count; i++) {
+ struct hvsi_struct *hp = _ports[i];
+
+ free_irq(hp->virq, hp);
+ }
+ tty_driver_kref_put(hvsi_driver);
+
+ return ret;
}
device_initcall(hvsi_init);
--
2.32.0
Re: [PATCH v2] xen/hvc: replace BUG_ON() with negative return value
On 07. 07. 21, 12:40, Juergen Gross wrote: And btw, since I've got puzzled by the linuxppc-dev@ in the recipients list, I did look up relevant entries in ./MAINTAINERS. Shouldn't the file be part of "XEN HYPERVISOR INTERFACE"? I wouldn't mind. Greg, Jiri, what do you think? /me concurs. thanks, -- js suse labs
[PATCH 40/44] tty: hvc, drop unneeded forward declarations
Forward declarations make the code larger and rewrites harder. Harder as they are often omitted from global changes. Remove forward declarations which are not really needed, i.e. the definition of the function is before its first use. Signed-off-by: Jiri Slaby Cc: linuxppc-dev@lists.ozlabs.org --- drivers/tty/hvc/hvcs.c | 25 - 1 file changed, 25 deletions(-) diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c index c90848919644..0b89d878a108 100644 --- a/drivers/tty/hvc/hvcs.c +++ b/drivers/tty/hvc/hvcs.c @@ -290,36 +290,11 @@ static LIST_HEAD(hvcs_structs); static DEFINE_SPINLOCK(hvcs_structs_lock); static DEFINE_MUTEX(hvcs_init_mutex); -static void hvcs_unthrottle(struct tty_struct *tty); -static void hvcs_throttle(struct tty_struct *tty); -static irqreturn_t hvcs_handle_interrupt(int irq, void *dev_instance); - -static int hvcs_write(struct tty_struct *tty, - const unsigned char *buf, int count); -static int hvcs_write_room(struct tty_struct *tty); -static int hvcs_chars_in_buffer(struct tty_struct *tty); - -static int hvcs_has_pi(struct hvcs_struct *hvcsd); -static void hvcs_set_pi(struct hvcs_partner_info *pi, - struct hvcs_struct *hvcsd); static int hvcs_get_pi(struct hvcs_struct *hvcsd); static int hvcs_rescan_devices_list(void); -static int hvcs_partner_connect(struct hvcs_struct *hvcsd); static void hvcs_partner_free(struct hvcs_struct *hvcsd); -static int hvcs_enable_device(struct hvcs_struct *hvcsd, - uint32_t unit_address, unsigned int irq, struct vio_dev *dev); - -static int hvcs_open(struct tty_struct *tty, struct file *filp); -static void hvcs_close(struct tty_struct *tty, struct file *filp); -static void hvcs_hangup(struct tty_struct * tty); - -static int hvcs_probe(struct vio_dev *dev, - const struct vio_device_id *id); -static int hvcs_remove(struct vio_dev *dev); -static int __init hvcs_module_init(void); -static void __exit hvcs_module_exit(void); static int hvcs_initialize(void); #define HVCS_SCHED_READ0x0001 -- 2.30.1
Re: [PATCH 3/3] tty: vcc: Drop impossible to hit WARN_ON
On 14. 01. 21, 18:57, Uwe Kleine-König wrote: vcc_get() returns the port that has provided port->index. As the port that is about to be removed isn't removed yet this trivially will find this port. So simplify the call to not assign an identical value to the port pointer and drop the warning that is never hit. Signed-off-by: Uwe Kleine-König Reviewed-by: Jiri Slaby --- drivers/tty/vcc.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c index d9b0dc6deae9..e2d6205f83ce 100644 --- a/drivers/tty/vcc.c +++ b/drivers/tty/vcc.c @@ -692,12 +692,9 @@ static int vcc_remove(struct vio_dev *vdev) tty_vhangup(port->tty); /* Get exclusive reference to VCC, ensures that there are no other -* clients to this port +* clients to this port. This cannot fail. */ - port = vcc_get(port->index, true); - - if (WARN_ON(!port)) - return -ENODEV; + vcc_get(port->index, true); tty_unregister_device(vcc_tty_driver, port->index); -- js
Re: [PATCH 2/3] tty: vcc: Drop unnecessary if block
On 14. 01. 21, 18:57, Uwe Kleine-König wrote:
If vcc_probe() succeeded dev_set_drvdata() is called with a non-NULL
value, and if vcc_probe() failed vcc_remove() isn't called.
So there is no way dev_get_drvdata() can return NULL in vcc_remove() and
the check can just go away.
Signed-off-by: Uwe Kleine-König
Reviewed-by: Jiri Slaby
---
drivers/tty/vcc.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
index 9ffd42e333b8..d9b0dc6deae9 100644
--- a/drivers/tty/vcc.c
+++ b/drivers/tty/vcc.c
@@ -681,9 +681,6 @@ static int vcc_remove(struct vio_dev *vdev)
{
struct vcc_port *port = dev_get_drvdata(>dev);
- if (!port)
- return -ENODEV;
-
del_timer_sync(>rx_timer);
del_timer_sync(>tx_timer);
--
js
Re: [PATCH 1/3] tty: hvcs: Drop unnecessary if block
On 14. 01. 21, 18:57, Uwe Kleine-König wrote: If hvcs_probe() succeeded dev_set_drvdata() is called with a non-NULL value, and if hvcs_probe() failed hvcs_remove() isn't called. So there is no way dev_get_drvdata() can return NULL in hvcs_remove() and the check can just go away. Signed-off-by: Uwe Kleine-König Reviewed-by: Jiri Slaby --- drivers/tty/hvc/hvcs.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c index 509d1042825a..3e0461285c34 100644 --- a/drivers/tty/hvc/hvcs.c +++ b/drivers/tty/hvc/hvcs.c @@ -825,9 +825,6 @@ static int hvcs_remove(struct vio_dev *dev) unsigned long flags; struct tty_struct *tty; - if (!hvcsd) - return -ENODEV; - /* By this time the vty-server won't be getting any more interrupts */ spin_lock_irqsave(>lock, flags); -- js
Re: [PATCH 34/36] tty: serial: pmac_zilog: Make disposable variable __always_unused
On 05. 11. 20, 9:36, Lee Jones wrote:
On Thu, 05 Nov 2020, Jiri Slaby wrote:
On 05. 11. 20, 8:04, Christophe Leroy wrote:
Le 04/11/2020 à 20:35, Lee Jones a écrit :
Fixes the following W=1 kernel build warning(s):
drivers/tty/serial/pmac_zilog.h:365:58: warning: variable
‘garbage’ set but not used [-Wunused-but-set-variable]
Explain how you are fixing this warning.
Setting __always_unused is usually not the good solution for fixing
this warning, but here I guess this is likely the good solution. But it
should be explained why.
There are normally 3 ways to fix this warning;
- Start using/checking the variable/result
- Remove the variable
- Mark it as __{always,maybe}_unused
The later just tells the compiler that not checking the resultant
value is intentional. There are some functions (as Jiri mentions
below) which are marked as '__must_check' which *require* a dummy
(garbage) variable to be used.
Or, why is the "garbage =" needed in the first place? read_zsdata is not
defined with __warn_unused_result__.
I used '__always_used' here for fear of breaking something.
However, if it's safe to remove it, then all the better.
Yes please -- this "garbage" is one of the examples of volatile misuses.
If readb didn't work on volatile pointer, marking the return variable as
volatile wouldn't save it.
And even if it was, would (void)!read_zsdata(port) fix it?
That's hideous. :D
Sure, marking reads as must_check would be insane.
*Much* better to just use '__always_used' in that use-case.
Then using a dummy variable to fool must_check must mean must_check is
used incorrectly, no :)? But there are always exceptions…
thanks,
--
js
suse labs
Re: [PATCH 34/36] tty: serial: pmac_zilog: Make disposable variable __always_unused
On 05. 11. 20, 8:04, Christophe Leroy wrote:
Le 04/11/2020 à 20:35, Lee Jones a écrit :
Fixes the following W=1 kernel build warning(s):
drivers/tty/serial/pmac_zilog.h:365:58: warning: variable ‘garbage’
set but not used [-Wunused-but-set-variable]
Explain how you are fixing this warning.
Setting __always_unused is usually not the good solution for fixing
this warning, but here I guess this is likely the good solution. But it
should be explained why.
Or, why is the "garbage =" needed in the first place? read_zsdata is not
defined with __warn_unused_result__. And even if it was, would
(void)!read_zsdata(port) fix it?
Cc: Greg Kroah-Hartman
Cc: Jiri Slaby
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: linux-ser...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones
---
drivers/tty/serial/pmac_zilog.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/pmac_zilog.h
b/drivers/tty/serial/pmac_zilog.h
index bb874e76810e0..968aec7c1cf82 100644
--- a/drivers/tty/serial/pmac_zilog.h
+++ b/drivers/tty/serial/pmac_zilog.h
@@ -362,7 +362,7 @@ static inline void zssync(struct uart_pmac_port
*port)
/* Misc macros */
#define ZS_CLEARERR(port) (write_zsreg(port, 0, ERR_RES))
-#define ZS_CLEARFIFO(port) do { volatile unsigned char garbage; \
+#define ZS_CLEARFIFO(port) do { volatile unsigned char
__always_unused garbage; \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
thanks,
--
js
Re: [PATCH] tty: hvcs: Don't NULL tty->driver_data until hvcs_cleanup()
On 21. 08. 20, 1:46, Tyrel Datwyler wrote:
> The code currently NULLs tty->driver_data in hvcs_close() with the
> intent of informing the next call to hvcs_open() that device needs to be
> reconfigured. However, when hvcs_cleanup() is called we copy hvcsd from
> tty->driver_data which was previoulsy NULLed by hvcs_close() and our
> call to tty_port_put(>port) doesn't actually do anything since
> >port ends up translating to NULL by chance. This has the side
> effect that when hvcs_remove() is called we have one too many port
> references preventing hvcs_destuct_port() from ever being called. This
> also prevents us from reusing the /dev/hvcsX node in a future
> hvcs_probe() and we can eventually run out of /dev/hvcsX devices.
>
> Fix this by waiting to NULL tty->driver_data in hvcs_cleanup().
Without actually looking into the code, it looks like we need a fix
similar to:
commit 24eb2377f977fe06d84fca558f891f95bc28a449
Author: Jiri Slaby
Date: Tue May 26 16:56:32 2020 +0200
tty: hvc_console, fix crashes on parallel open/close
here too?
> Signed-off-by: Tyrel Datwyler
> ---
> drivers/tty/hvc/hvcs.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
> index 55105ac38f89..509d1042825a 100644
> --- a/drivers/tty/hvc/hvcs.c
> +++ b/drivers/tty/hvc/hvcs.c
> @@ -1216,13 +1216,6 @@ static void hvcs_close(struct tty_struct *tty, struct
> file *filp)
>
> tty_wait_until_sent(tty, HVCS_CLOSE_WAIT);
>
> - /*
> - * This line is important because it tells hvcs_open that this
> - * device needs to be re-configured the next time hvcs_open is
> - * called.
> - */
> - tty->driver_data = NULL;
> -
> free_irq(irq, hvcsd);
> return;
> } else if (hvcsd->port.count < 0) {
> @@ -1237,6 +1230,13 @@ static void hvcs_cleanup(struct tty_struct * tty)
> {
> struct hvcs_struct *hvcsd = tty->driver_data;
>
> + /*
> + * This line is important because it tells hvcs_open that this
> + * device needs to be re-configured the next time hvcs_open is
> + * called.
> + */
> + tty->driver_data = NULL;
> +
> tty_port_put(>port);
> }
>
>
thanks,
--
js
Re: [PATCH] serial: ucc_uart: make qe_uart_set_mctrl() static
On 12. 09. 20, 5:38, Jason Yan wrote:
> This eliminates the following sparse warning:
>
> drivers/tty/serial/ucc_uart.c:286:6: warning: symbol 'qe_uart_set_mctrl'
> was not declared. Should it be static?
>
> Reported-by: Hulk Robot
> Signed-off-by: Jason Yan
Sure:
Acked-by: Jiri Slaby
> ---
> drivers/tty/serial/ucc_uart.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
> index 3c8c662c69e2..d6a8604157ab 100644
> --- a/drivers/tty/serial/ucc_uart.c
> +++ b/drivers/tty/serial/ucc_uart.c
> @@ -283,7 +283,7 @@ static unsigned int qe_uart_tx_empty(struct uart_port
> *port)
> * don't need that support. This function must exist, however, otherwise
> * the kernel will panic.
> */
> -void qe_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +static void qe_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> {
> }
>
>
--
js
Re: [PATCH] tty: hvc: Fix data abort due to race in hvc_open
On 15. 05. 20, 1:22, rana...@codeaurora.org wrote:
> On 2020-05-13 00:04, Greg KH wrote:
>> On Tue, May 12, 2020 at 02:39:50PM -0700, rana...@codeaurora.org wrote:
>>> On 2020-05-12 01:25, Greg KH wrote:
>>> > On Tue, May 12, 2020 at 09:22:15AM +0200, Jiri Slaby wrote:
>>> > > commit bdb498c20040616e94b05c31a0ceb3e134b7e829
>>> > > Author: Jiri Slaby
>>> > > Date: Tue Aug 7 21:48:04 2012 +0200
>>> > >
>>> > > TTY: hvc_console, add tty install
>>> > >
>>> > > added hvc_install but did not move 'tty->driver_data = NULL;' from
>>> > > hvc_open's fail path to hvc_cleanup.
>>> > >
>>> > > IOW hvc_open now NULLs tty->driver_data even for another task which
>>> > > opened the tty earlier. The same holds for
>>> > > "tty_port_tty_set(>port,
>>> > > NULL);" there. And actually "tty_port_put(>port);" is also
>>> > > incorrect
>>> > > for the 2nd task opening the tty.
>>> > >
...
> These are the traces you get when the issue happens:
> [ 154.212291] hvc_install called for pid: 666
> [ 154.216705] hvc_open called for pid: 666
> [ 154.233657] hvc_open: request_irq failed with rc -22.
> [ 154.238934] hvc_open called for pid: 678
> [ 154.243012] Unable to handle kernel NULL pointer dereference at
> virtual address 00c4
> # hvc_install isn't called for pid: 678 as the file wasn't closed yet.
Nice. Does the attached help?
I wonder how comes the tty_port_put in hvc_open does not cause a UAF? I
would say hvc_open fails, tty_port_put is called. It decrements the
reference taken in hvc_install. So far so good.
Now, this should happen IMO:
tty_open
-> hvc_open (fails)
-> tty_port_put
-> tty_release
-> tty_release_struct
-> tty_kref_put
-> queue_release_one_tty
SCHEDULED WORKQUEUE
release_one_tty
-> hvc_cleanup
-> tty_port_put (should die terrible death now)
What am I missing?
thanks,
--
js
suse labs
>From d891cdfcbd3b41eb23ddfc8d9e6cbe038ff8fb72 Mon Sep 17 00:00:00 2001
From: Jiri Slaby
Date: Wed, 20 May 2020 11:29:25 +0200
Subject: [PATCH] hvc_console: fix open
Signed-off-by: Jiri Slaby
---
drivers/tty/hvc/hvc_console.c | 23 ---
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 436cc51c92c3..cdcc64ea2554 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -371,15 +371,14 @@ static int hvc_open(struct tty_struct *tty, struct file * filp)
* tty fields and return the kref reference.
*/
if (rc) {
- tty_port_tty_set(>port, NULL);
- tty->driver_data = NULL;
- tty_port_put(>port);
printk(KERN_ERR "hvc_open: request_irq failed with rc %d.\n", rc);
- } else
+ } else {
/* We are ready... raise DTR/RTS */
if (C_BAUD(tty))
if (hp->ops->dtr_rts)
hp->ops->dtr_rts(hp, 1);
+ tty_port_set_initialized(>port, true);
+ }
/* Force wakeup of the polling thread */
hvc_kick();
@@ -389,22 +388,12 @@ static int hvc_open(struct tty_struct *tty, struct file * filp)
static void hvc_close(struct tty_struct *tty, struct file * filp)
{
- struct hvc_struct *hp;
+ struct hvc_struct *hp = tty->driver_data;
unsigned long flags;
if (tty_hung_up_p(filp))
return;
- /*
- * No driver_data means that this close was issued after a failed
- * hvc_open by the tty layer's release_dev() function and we can just
- * exit cleanly because the kref reference wasn't made.
- */
- if (!tty->driver_data)
- return;
-
- hp = tty->driver_data;
-
spin_lock_irqsave(>port.lock, flags);
if (--hp->port.count == 0) {
@@ -412,6 +401,9 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
/* We are done with the tty pointer now. */
tty_port_tty_set(>port, NULL);
+ if (!tty_port_initialized(>port))
+ return;
+
if (C_HUPCL(tty))
if (hp->ops->dtr_rts)
hp->ops->dtr_rts(hp, 0);
@@ -428,6 +420,7 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
* waking periodically to check chars_in_buffer().
*/
tty_wait_until_sent(tty, HVC_CLOSE_WAIT);
+ tty_port_set_initialized(>port, false);
} else {
if (hp->port.count < 0)
printk(KERN_ERR "hvc_close %X: oops, count is %d\n",
--
2.26.2
Re: [PATCH v2] tty: hvc: Fix data abort due to race in hvc_open
On 20. 05. 20, 8:47, Raghavendra Rao Ananta wrote: > Potentially, hvc_open() can be called in parallel when two tasks calls > open() on /dev/hvcX. In such a scenario, if the hp->ops->notifier_add() > callback in the function fails, where it sets the tty->driver_data to > NULL, the parallel hvc_open() can see this NULL and cause a memory abort. > Hence, do a NULL check at the beginning, before proceeding ahead. > > The issue can be easily reproduced by launching two tasks simultaneously > that does an open() call on /dev/hvcX. > For example: > $ cat /dev/hvc0 & cat /dev/hvc0 & > > Cc: sta...@vger.kernel.org > Signed-off-by: Raghavendra Rao Ananta > --- > drivers/tty/hvc/hvc_console.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c > index 436cc51c92c3..80709f754cc8 100644 > --- a/drivers/tty/hvc/hvc_console.c > +++ b/drivers/tty/hvc/hvc_console.c > @@ -350,6 +350,9 @@ static int hvc_open(struct tty_struct *tty, struct file * > filp) > unsigned long flags; > int rc = 0; > > + if (!hp) > + return -ENODEV; > + This is still not fixing the bug properly. See: https://lore.kernel.org/linuxppc-dev/0f7791f5-0a53-59f6-7277-247a789f3...@suse.cz/ In particular, the paragraph starting "IOW". thanks, -- js suse labs
Re: [PATCH] tty: hvc: Fix data abort due to race in hvc_open
On 11. 05. 20, 9:39, Greg KH wrote:
> On Mon, May 11, 2020 at 12:23:58AM -0700, rana...@codeaurora.org wrote:
>> On 2020-05-09 23:48, Greg KH wrote:
>>> On Sat, May 09, 2020 at 06:30:56PM -0700, rana...@codeaurora.org wrote:
>>>> On 2020-05-06 02:48, Greg KH wrote:
>>>>> On Mon, Apr 27, 2020 at 08:26:01PM -0700, Raghavendra Rao Ananta wrote:
>>>>>> Potentially, hvc_open() can be called in parallel when two tasks calls
>>>>>> open() on /dev/hvcX. In such a scenario, if the
>>>>>> hp->ops->notifier_add()
>>>>>> callback in the function fails, where it sets the tty->driver_data to
>>>>>> NULL, the parallel hvc_open() can see this NULL and cause a memory
>>>>>> abort.
>>>>>> Hence, serialize hvc_open and check if tty->private_data is NULL
>>>>>> before
>>>>>> proceeding ahead.
>>>>>>
>>>>>> The issue can be easily reproduced by launching two tasks
>>>>>> simultaneously
>>>>>> that does nothing but open() and close() on /dev/hvcX.
>>>>>> For example:
>>>>>> $ ./simple_open_close /dev/hvc0 & ./simple_open_close /dev/hvc0 &
>>>>>>
>>>>>> Signed-off-by: Raghavendra Rao Ananta
>>>>>> ---
>>>>>> drivers/tty/hvc/hvc_console.c | 16 ++--
>>>>>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/tty/hvc/hvc_console.c
>>>>>> b/drivers/tty/hvc/hvc_console.c
>>>>>> index 436cc51c92c3..ebe26fe5ac09 100644
>>>>>> --- a/drivers/tty/hvc/hvc_console.c
>>>>>> +++ b/drivers/tty/hvc/hvc_console.c
>>>>>> @@ -75,6 +75,8 @@ static LIST_HEAD(hvc_structs);
>>>>>> */
>>>>>> static DEFINE_MUTEX(hvc_structs_mutex);
>>>>>>
>>>>>> +/* Mutex to serialize hvc_open */
>>>>>> +static DEFINE_MUTEX(hvc_open_mutex);
>>>>>> /*
>>>>>> * This value is used to assign a tty->index value to a hvc_struct
>>>>>> based
>>>>>> * upon order of exposure via hvc_probe(), when we can not match it
>>>>>> to
>>>>>> @@ -346,16 +348,24 @@ static int hvc_install(struct tty_driver
>>>>>> *driver, struct tty_struct *tty)
>>>>>> */
>>>>>> static int hvc_open(struct tty_struct *tty, struct file * filp)
>>>>>> {
>>>>>> -struct hvc_struct *hp = tty->driver_data;
>>>>>> +struct hvc_struct *hp;
>>>>>> unsigned long flags;
>>>>>> int rc = 0;
>>>>>>
>>>>>> +mutex_lock(_open_mutex);
>>>>>> +
>>>>>> +hp = tty->driver_data;
>>>>>> +if (!hp) {
>>>>>> +rc = -EIO;
>>>>>> +goto out;
>>>>>> +}
>>>>>> +
>>>>>> spin_lock_irqsave(>port.lock, flags);
>>>>>> /* Check and then increment for fast path open. */
>>>>>> if (hp->port.count++ > 0) {
>>>>>> spin_unlock_irqrestore(>port.lock, flags);
>>>>>> hvc_kick();
>>>>>> -return 0;
>>>>>> +goto out;
>>>>>> } /* else count == 0 */
>>>>>> spin_unlock_irqrestore(>port.lock, flags);
>>>>>
>>>>> Wait, why isn't this driver just calling tty_port_open() instead of
>>>>> trying to open-code all of this?
>>>>>
>>>>> Keeping a single mutext for open will not protect it from close, it will
>>>>> just slow things down a bit. There should already be a tty lock held by
>>>>> the tty core for open() to keep it from racing things, right?
>>>> The tty lock should have been held, but not likely across
>>>> ->install() and
>>>> ->open() callbacks, thus resulting in a race between hvc_install() and
>>>> hvc_open(),
>>>
>>> How? The tty lock is held in install, and should not conflict with
>>> open(), otherwise, we would be seeing this happen in all tty drivers,
>>> right?
>>>
Re: [PATCH] tty: hvc: remove hvcs_driver_string
On 03. 04. 20, 9:13, Jason Yan wrote: > No users of hvcs_driver_string, remove it. This fixes the following gcc > warning: > > drivers/tty/hvc/hvcs.c:199:19: warning: ‘hvcs_driver_string’ defined but > not used [-Wunused-const-variable=] > static const char hvcs_driver_string[] >^~ > > Reported-by: Hulk Robot > Signed-off-by: Jason Yan Acked-by: Jiri Slaby It fixes a commit from 2011: commit c7704d352d45de47333f2d9f10aead820b49044c Author: Benjamin Herrenschmidt Date: Sun Feb 6 18:26:25 2011 + powerpc/pseries: Reduce HVCS driver insanity ! > --- > drivers/tty/hvc/hvcs.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c > index ee0604cd9c6b..55105ac38f89 100644 > --- a/drivers/tty/hvc/hvcs.c > +++ b/drivers/tty/hvc/hvcs.c > @@ -196,8 +196,6 @@ module_param(hvcs_parm_num_devs, int, 0); > > static const char hvcs_driver_name[] = "hvcs"; > static const char hvcs_device_node[] = "hvcs"; > -static const char hvcs_driver_string[] > - = "IBM hvcs (Hypervisor Virtual Console Server) Driver"; > > /* Status of partner info rescan triggered via sysfs. */ > static int hvcs_rescan_status; > -- js suse labs
Re: [RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)
On 19. 02. 20, 18:50, Krzysztof Kozlowski wrote:
> The ioreadX() helpers have inconsistent interface. On some architectures
> void *__iomem address argument is a pointer to const, on some not.
>
> Implementations of ioreadX() do not modify the memory under the address
> so they can be converted to a "const" version for const-safety and
> consistency among architectures.
>
> Signed-off-by: Krzysztof Kozlowski
> Acked-by: Kalle Valo
> ---
> drivers/net/wireless/ath/ath5k/ahb.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ahb.c
> b/drivers/net/wireless/ath/ath5k/ahb.c
> index 2c9cec8b53d9..8bd01df369fb 100644
> --- a/drivers/net/wireless/ath/ath5k/ahb.c
> +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
>
> if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> /* Enable WMAC AHB arbitration */
> - reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> + reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
While I understand why the parameter of ioread32 should be const, I
don't see a reason for these casts on the users' side. What does it
bring except longer code to read?
thanks,
--
js
Re: [PATCH 01/25] tty: Change return type to void
On 09/05/2018, 03:08 AM, Jaejoong Kim wrote: > > @@ -688,7 +688,7 @@ extern int tty_port_close_start(struct > tty_port *port, > > extern void tty_port_close_end(struct tty_port *port, struct > tty_struct *tty); > > extern void tty_port_close(struct tty_port *port, > > struct tty_struct *tty, struct file > *filp); > > -extern int tty_port_install(struct tty_port *port, struct > tty_driver *driver, > > +extern void tty_port_install(struct tty_port *port, struct > tty_driver *driver, > > struct tty_struct *tty); > > You need to update all the callers in the same patch -- the > kernel must > remain buildable after each patch but you seem to have spread that > update > among a lot of patches.. > > > OK. I will make several patches as one patch. You don't have to. Just reorder the patches as suggested by Sam. (First, make everybody ignore the return value, then change these return types to void.) BTW you can mention in this commit log, that this change is possible after a3123fd0a4a5. That commit made tty_standard_install to always return 0. thanks, -- js suse labs
Re: [PATCH v1] mm: relax deferred struct page requirements
On 08/31/2018, 02:10 PM, Pasha Tatashin wrote: > Thanks Jiri, I am now able to reproduce it with your new config. > > I have tried yesterday to enable sparsemem and deferred_struct_init on > x86_32, and that kernel booted fine, there must be something else in > your config that helps to trigger this problem. I am studying it now. > > [0.051245] Initializing CPU#0 > [0.051682] Initializing HighMem for node 0 (000367fe:0007ffe0) > [0.067499] BUG: unable to handle kernel NULL pointer dereference at > 0028 > [0.068452] *pdpt = *pde = f000ff53f000ff53 > [0.069105] Oops: [#1] PREEMPT SMP PTI > [0.069595] CPU: 0 PID: 0 Comm: swapper Not tainted > 4.19.0-rc1-pae_pt_jiri #1 > [0.070382] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS 1.11.0-20171110_100015-anatol 04/01/2014 > [0.071545] EIP: free_unref_page_prepare.part.70+0x2c/0x50 > [0.072178] Code: 19 e9 ff 89 d1 55 c1 ea 11 c1 e9 07 8b 14 d5 44 52 > fd d6 81 e1 fc 03 00 00 89 e5 56 53 89 cb be 1d 00 00 00 c1 eb 05 83 e1 > 1f <8b> 14 9a 29 ce 89 f1 d3 ea 83 e2 07 89 50 10 b8 01 00 00 00 5b 5e > [0.074296] EAX: f4cfa000 EBX: 000a ECX: 0010 EDX: > [0.075005] ESI: 001d EDI: 0007ffe0 EBP: d6d41ed0 ESP: d6d41ec8 > [0.075714] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00210002 > [0.076508] CR0: 80050033 CR2: 0028 CR3: 16f2 CR4: 000406b0 > [0.077242] DR0: DR1: DR2: DR3: > [0.077934] DR6: fffe0ff0 DR7: 0400 > [0.078380] Call Trace: > [0.078670] free_unref_page+0x3a/0x90 > [0.079136] __free_pages+0x25/0x30 > [0.079533] free_highmem_page+0x1e/0x50 > [0.079978] add_highpages_with_active_regions+0xd1/0x11f > [0.080592] set_highmem_pages_init+0x67/0x7d > [0.081076] mem_init+0x30/0x1fc page_to_pfn(pfn_to_page(pfn)) != pfn with my .config on pfns >= 0x6: [0.157667] add_highpages_with_active_regions: pfn=5fffb pg=f55f9f4c pfn(pg(pfn)=5fffb sec=2 [0.159231] add_highpages_with_active_regions: pfn=5fffc pg=f55f9f70 pfn(pg(pfn)=5fffc sec=2 [0.161020] add_highpages_with_active_regions: pfn=5fffd pg=f55f9f94 pfn(pg(pfn)=5fffd sec=2 [0.163149] add_highpages_with_active_regions: pfn=5fffe pg=f55f9fb8 pfn(pg(pfn)=5fffe sec=2 [0.165204] add_highpages_with_active_regions: pfn=5 pg=f55f9fdc pfn(pg(pfn)=5 sec=2 [0.167216] add_highpages_with_active_regions: pfn=6 pg=f4cfa000 pfn(pg(pfn)=c716a800 sec=3 So add_highpages_with_active_regions passes down page to free_highmem_page and later, free_unref_page does page_to_pfn(page) and __get_pfnblock_flags_mask operates on this modified pfn leading to crash – __pfn_to_section(pfn)->pageblock_flags is NULL! Note that __pfn_to_section(pfn)->pageblock_flags on the original pfn returns a valid bitmap. thanks, -- js suse labs
Re: [PATCH v1] mm: relax deferred struct page requirements
On 08/31/2018, 01:26 PM, Jiri Slaby wrote: > On 08/30/2018, 05:45 PM, Pasha Tatashin wrote: >> Hi Jiri, >> >> I believe this bug is fixed with this change: >> >> d39f8fb4b7776dcb09ec3bf7a321547083078ee3 >> mm: make DEFERRED_STRUCT_PAGE_INIT explicitly depend on SPARSEMEM > > Hi, > > it only shifted. Enabling only SPARSEMEM works fine, enabling also > DEFERRED_STRUCT_PAGE_INIT doesn't even boot – immediately reboots > (config attached). Wow, earlyprintk is up at the moment of crash already: [0.00] Linux version 4.19.0-rc1-pae (jslaby@kunlun) (gcc version 4.8.5 (SUSE Linux)) #4 SMP PREEMPT Fri Aug 31 13:18:33 CEST 2018 [0.00] x86/fpu: x87 FPU will use FXSAVE [0.00] BIOS-provided physical RAM map: [0.00] BIOS-e820: [mem 0x-0x0009fbff] usable [0.00] BIOS-e820: [mem 0x0009fc00-0x0009] reserved [0.00] BIOS-e820: [mem 0x000f-0x000f] reserved [0.00] BIOS-e820: [mem 0x0010-0x7cfd] usable [0.00] BIOS-e820: [mem 0x7cfe-0x7cff] reserved [0.00] BIOS-e820: [mem 0xfeffc000-0xfeff] reserved [0.00] BIOS-e820: [mem 0xfffc-0x] reserved [0.00] bootconsole [earlyser0] enabled [0.00] NX (Execute Disable) protection: active [0.00] SMBIOS 2.8 present. [0.00] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014 [0.00] Hypervisor detected: KVM [0.00] kvm-clock: Using msrs 4b564d01 and 4b564d00 [0.02] kvm-clock: cpu 0, msr 1d12c001, primary cpu clock [0.02] kvm-clock: using sched offset of 1597117996 cycles [0.001395] clocksource: kvm-clock: mask: 0x max_cycles: 0x1cd42e4dffb, max_idle_ns: 881590591483 ns [0.006245] tsc: Detected 2808.000 MHz processor [0.010055] last_pfn = 0x7cfe0 max_arch_pfn = 0x100 [0.011483] x86/PAT: PAT not supported by CPU. [0.012580] x86/PAT: Configuration [0-7]: WB WT UC- UC WB WT UC- UC [0.020644] found SMP MP-table at [mem 0x000f5d20-0x000f5d2f] mapped at [(ptrval)] [0.023528] Scanning 1 areas for low memory corruption [0.025047] ACPI: Early table checksum verification disabled [0.026581] ACPI: RSDP 0x000F5B40 14 (v00 BOCHS ) [0.028031] ACPI: RSDT 0x7CFE157C 30 (v01 BOCHS BXPCRSDT 0001 BXPC 0001) [0.029996] ACPI: FACP 0x7CFE1458 74 (v01 BOCHS BXPCFACP 0001 BXPC 0001) [0.032234] ACPI: DSDT 0x7CFE0040 001418 (v01 BOCHS BXPCDSDT 0001 BXPC 0001) [0.034662] ACPI: FACS 0x7CFE 40 [0.036126] ACPI: APIC 0x7CFE14CC 78 (v01 BOCHS BXPCAPIC 0001 BXPC 0001) [0.038235] ACPI: HPET 0x7CFE1544 38 (v01 BOCHS BXPCHPET 0001 BXPC 0001) [0.040373] No NUMA configuration found [0.041407] Faking a node at [mem 0x-0x7cfd] [0.043306] NODE_DATA(0) allocated [mem 0x367fc000-0x367fcfff] [0.044958] 1127MB HIGHMEM available. [0.045940] 871MB LOWMEM available. [0.046978] mapped low ram: 0 - 367fe000 [0.048200] low ram: 0 - 367fe000 [0.050830] Zone ranges: [0.051625] DMA [mem 0x1000-0x00ff] [0.053295] Normal [mem 0x0100-0x367fdfff] [0.054921] HighMem [mem 0x367fe000-0x7cfd] [0.056408] Movable zone start for each node [0.057452] Early memory node ranges [0.058377] node 0: [mem 0x1000-0x0009efff] [0.059946] node 0: [mem 0x0010-0x7cfd] [0.061825] Reserved but unavailable: 12418 pages [0.061828] Initmem setup node 0 [mem 0x1000-0x7cfd] [0.074252] Using APIC driver default [0.075615] ACPI: PM-Timer IO Port: 0x608 [0.076574] ACPI: LAPIC_NMI (acpi_id[0xff] dfl dfl lint[0x1]) [0.077995] IOAPIC[0]: apic_id 0, version 17, address 0xfec0, GSI 0-23 [0.079610] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl) [0.08] ACPI: INT_SRC_OVR (bus 0 bus_irq 5 global_irq 5 high level) [0.082786] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level) [0.084297] ACPI: INT_SRC_OVR (bus 0 bus_irq 10 global_irq 10 high level) [0.085933] ACPI: INT_SRC_OVR (bus 0 bus_irq 11 global_irq 11 high level) [0.087729] Using ACPI (MADT) for SMP configuration information [0.089119] ACPI: HPET id: 0x8086a201 base: 0xfed0 [0.090351] smpboot: Allowing 1 CPUs, 0 hotplug CPUs [0.091561] PM: Registered nosave memory: [mem 0x-0x0fff] [0.093361] PM: Registered nosave memory: [mem 0x0009f000-0x0009] [0.096382] PM: Registered nosave memory: [mem 0x000a-0x000e] [0.098130] PM: Registered nosave memory: [mem 0x000f0
Re: [PATCH v1] mm: relax deferred struct page requirements
pasha.tatas...@oracle.com -> pavel.tatas...@microsoft.com due to 550 5.1.1 Unknown recipient address. On 08/24/2018, 09:32 AM, Jiri Slaby wrote: > On 06/19/2018, 09:56 PM, Pavel Tatashin wrote: >> On Tue, Jun 19, 2018 at 9:50 AM Pavel Tatashin >> wrote: >>> >>> On Sat, Jun 16, 2018 at 4:04 AM Jiri Slaby wrote: >>>> >>>> On 11/21/2017, 08:24 AM, Michal Hocko wrote: >>>>> On Thu 16-11-17 20:46:01, Pavel Tatashin wrote: >>>>>> There is no need to have ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT, >>>>>> as all the page initialization code is in common code. >>>>>> >>>>>> Also, there is no need to depend on MEMORY_HOTPLUG, as initialization >>>>>> code >>>>>> does not really use hotplug memory functionality. So, we can remove this >>>>>> requirement as well. >>>>>> >>>>>> This patch allows to use deferred struct page initialization on all >>>>>> platforms with memblock allocator. >>>>>> >>>>>> Tested on x86, arm64, and sparc. Also, verified that code compiles on >>>>>> PPC with CONFIG_MEMORY_HOTPLUG disabled. >>>>> >>>>> There is slight risk that we will encounter corner cases on some >>>>> architectures with weird memory layout/topology >>>> >>>> Which x86_32-pae seems to be. Many bad page state errors are emitted >>>> during boot when this patch is applied: >>> >>> Hi Jiri, >>> >>> Thank you for reporting this bug. >>> >>> Because 32-bit systems are limited in the maximum amount of physical >>> memory, they don't need deferred struct pages. So, we can add depends >>> on 64BIT to DEFERRED_STRUCT_PAGE_INIT in mm/Kconfig. >>> >>> However, before we do this, I want to try reproducing this problem and >>> root cause it, as it might expose a general problem that is not 32-bit >>> specific. >> >> Hi Jiri, >> >> Could you please attach your config and full qemu arguments that you >> used to reproduce this bug. > > Hi, > > I seem I never replied. Attaching .config and the qemu cmdline: > $ qemu-kvm -m 2000 -hda /dev/null -kernel bzImage > > "-m 2000" is important to reproduce. > > If I disable CONFIG_DEFERRED_STRUCT_PAGE_INIT (which the patch allowed > to enable), the error goes away, of course. > > thanks, > -- js suse labs
Re: [PATCH v1] mm: relax deferred struct page requirements
On 11/21/2017, 08:24 AM, Michal Hocko wrote: > On Thu 16-11-17 20:46:01, Pavel Tatashin wrote: >> There is no need to have ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT, >> as all the page initialization code is in common code. >> >> Also, there is no need to depend on MEMORY_HOTPLUG, as initialization code >> does not really use hotplug memory functionality. So, we can remove this >> requirement as well. >> >> This patch allows to use deferred struct page initialization on all >> platforms with memblock allocator. >> >> Tested on x86, arm64, and sparc. Also, verified that code compiles on >> PPC with CONFIG_MEMORY_HOTPLUG disabled. > > There is slight risk that we will encounter corner cases on some > architectures with weird memory layout/topology Which x86_32-pae seems to be. Many bad page state errors are emitted during boot when this patch is applied: BUG: Bad page state in process swapper pfn:3c01c page:f566c3f0 count:0 mapcount:1 mapping: index:0x0 flags: 0x0() raw: 0100 0200 raw: page dumped because: nonzero mapcount Modules linked in: CPU: 0 PID: 0 Comm: swapper Tainted: GB 4.17.1-4.gdf028bb-pae #1 openSUSE Tumbleweed (unreleased) Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014 Call Trace: dump_stack+0x7d/0xbd bad_page.cold.111+0x90/0xc7 free_pages_check_bad+0x52/0x70 free_pcppages_bulk+0x37d/0x570 free_unref_page_commit+0x9a/0xc0 free_unref_page+0x6a/0xa0 __free_pages+0x17/0x30 free_highmem_page+0x1e/0x50 add_highpages_with_active_regions+0xd6/0x113 set_highmem_pages_init+0x67/0x7d mem_init+0x23/0x1d9 start_kernel+0x1c2/0x437 i386_start_kernel+0x98/0x9c startup_32_smp+0x164/0x168 free_pages_check_bad expects mapcount == -1, but it is 1 with this patch. Reverting the patch makes the BUGs go away -- the config diff is then: @@ -617,7 +617,7 @@ # CONFIG_PGTABLE_MAPPING is not set # CONFIG_ZSMALLOC_STAT is not set CONFIG_GENERIC_EARLY_IOREMAP=y -CONFIG_DEFERRED_STRUCT_PAGE_INIT=y +CONFIG_ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT=y # CONFIG_IDLE_PAGE_TRACKING is not set CONFIG_FRAME_VECTOR=y # CONFIG_PERCPU_STATS is not set >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -148,7 +148,6 @@ config PPC >> select ARCH_MIGHT_HAVE_PC_PARPORT >> select ARCH_MIGHT_HAVE_PC_SERIO >> select ARCH_SUPPORTS_ATOMIC_RMW >> -select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT >> select ARCH_USE_BUILTIN_BSWAP >> select ARCH_USE_CMPXCHG_LOCKREF if PPC64 >> select ARCH_WANT_IPC_PARSE_VERSION >> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig >> index 863a62a6de3c..525c2e3df6f5 100644 >> --- a/arch/s390/Kconfig >> +++ b/arch/s390/Kconfig >> @@ -108,7 +108,6 @@ config S390 >> select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE >> select ARCH_SAVE_PAGE_KEYS if HIBERNATION >> select ARCH_SUPPORTS_ATOMIC_RMW >> -select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT >> select ARCH_SUPPORTS_NUMA_BALANCING >> select ARCH_USE_BUILTIN_BSWAP >> select ARCH_USE_CMPXCHG_LOCKREF >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >> index df3276d6bfe3..00a5446de394 100644 >> --- a/arch/x86/Kconfig >> +++ b/arch/x86/Kconfig >> @@ -69,7 +69,6 @@ config X86 >> select ARCH_MIGHT_HAVE_PC_PARPORT >> select ARCH_MIGHT_HAVE_PC_SERIO >> select ARCH_SUPPORTS_ATOMIC_RMW >> -select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT >> select ARCH_SUPPORTS_NUMA_BALANCING if X86_64 >> select ARCH_USE_BUILTIN_BSWAP >> select ARCH_USE_QUEUED_RWLOCKS >> diff --git a/mm/Kconfig b/mm/Kconfig >> index 9c4b80c2..c6bd0309ce7a 100644 >> --- a/mm/Kconfig >> +++ b/mm/Kconfig >> @@ -639,15 +639,10 @@ config MAX_STACK_SIZE_MB >> >>A sane initial value is 80 MB. >> >> -# For architectures that support deferred memory initialisation >> -config ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT >> -bool >> - >> config DEFERRED_STRUCT_PAGE_INIT >> bool "Defer initialisation of struct pages to kthreads" >> default n >> -depends on ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT >> -depends on NO_BOOTMEM && MEMORY_HOTPLUG >> +depends on NO_BOOTMEM >> depends on !FLATMEM >> help >>Ordinarily all struct pages are initialised during early boot in a thanks, -- js suse labs
Re: [PATCH 4.9 27/33] futex: Remove duplicated code and fix undefined behaviour
On 05/18/2018, 10:16 AM, Greg Kroah-Hartman wrote:
> 4.9-stable review patch. If anyone has any objections, please let me know.
>
> --
>
> From: Jiri Slaby <jsl...@suse.cz>
>
> commit 30d6e0a4190d37740e9447e4e4815f06992dd8c3 upstream.
...
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -1458,6 +1458,45 @@ out:
> return ret;
> }
>
> +static int futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
> +{
> + unsigned int op = (encoded_op & 0x7000) >> 28;
> + unsigned int cmp =(encoded_op & 0x0f00) >> 24;
> + int oparg = sign_extend32((encoded_op & 0x00fff000) >> 12, 12);
> + int cmparg = sign_extend32(encoded_op & 0x0fff, 12);
12 is wrong here – wherever you apply this, you need also a follow-up fix:
commit d70ef22892ed6c066e51e118b225923c9b74af34
Author: Jiri Slaby <jsl...@suse.cz>
Date: Thu Nov 30 15:35:44 2017 +0100
futex: futex_wake_op, fix sign_extend32 sign bits
thanks,
--
js
suse labs
[PATCH v2 1/1] futex: remove duplicated code and fix UB
There is code duplicated over all architecture's headers for
futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
and comparison of the result.
Remove this duplication and leave up to the arches only the needed
assembly which is now in arch_futex_atomic_op_inuser.
This effectively distributes the Will Deacon's arm64 fix for undefined
behaviour reported by UBSAN to all architectures. The fix was done in
commit 5f16a046f8e1 (arm64: futex: Fix undefined behaviour with
FUTEX_OP_OPARG_SHIFT usage). Look there for an example dump.
And as suggested by Thomas, check for negative oparg too, because it was
also reported to cause undefined behaviour report.
Note that s390 removed access_ok check in d12a29703 ("s390/uaccess:
remove pointless access_ok() checks") as access_ok there returns true.
We introduce it back to the helper for the sake of simplicity (it gets
optimized away anyway).
[v2]
- check also for negative values
- wait for Will's fix to be in upstream
Signed-off-by: Jiri Slaby <jsl...@suse.cz>
Cc: Richard Henderson <r...@twiddle.net>
Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru>
Cc: Matt Turner <matts...@gmail.com>
Cc: Vineet Gupta <vgu...@synopsys.com>
Acked-by: Russell King <rmk+ker...@armlinux.org.uk>
Cc: Catalin Marinas <catalin.mari...@arm.com>
Cc: Will Deacon <will.dea...@arm.com>
Reviewed-by: Darren Hart (VMware) <dvh...@infradead.org>
Cc: Richard Kuo <r...@codeaurora.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Fenghua Yu <fenghua...@intel.com>
Cc: Michal Simek <mon...@monstr.eu>
Cc: Ralf Baechle <r...@linux-mips.org>
Cc: Jonas Bonn <jo...@southpole.se>
Cc: Stefan Kristiansson <stefan.kristians...@saunalahti.fi>
Cc: Stafford Horne <sho...@gmail.com>
Cc: "James E.J. Bottomley" <j...@parisc-linux.org>
Cc: Helge Deller <del...@gmx.de>
Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
Cc: Paul Mackerras <pau...@samba.org>
Acked-by: Michael Ellerman <m...@ellerman.id.au> (powerpc)
Cc: Martin Schwidefsky <schwidef...@de.ibm.com>
Acked-by: Heiko Carstens <heiko.carst...@de.ibm.com> [s390]
Cc: Yoshinori Sato <ys...@users.sourceforge.jp>
Cc: Rich Felker <dal...@libc.org>
Cc: "David S. Miller" <da...@davemloft.net>
Acked-by: Chris Metcalf <cmetc...@mellanox.com> [for tile]
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: Chris Zankel <ch...@zankel.net>
Cc: Max Filippov <jcmvb...@gmail.com>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: <x...@kernel.org>
Cc: <linux-al...@vger.kernel.org>
Cc: <linux-ker...@vger.kernel.org>
Cc: <linux-snps-...@lists.infradead.org>
Cc: <linux-arm-ker...@lists.infradead.org>
Cc: <linux-hexa...@vger.kernel.org>
Cc: <linux-i...@vger.kernel.org>
Cc: <linux-m...@linux-mips.org>
Cc: <openr...@lists.librecores.org>
Cc: <linux-par...@vger.kernel.org>
Cc: <linuxppc-dev@lists.ozlabs.org>
Cc: <linux-s...@vger.kernel.org>
Cc: <linux...@vger.kernel.org>
Cc: <sparcli...@vger.kernel.org>
Cc: <linux-xte...@linux-xtensa.org>
Cc: <linux-a...@vger.kernel.org>
---
arch/alpha/include/asm/futex.h | 26 ---
arch/arc/include/asm/futex.h| 40 -
arch/arm/include/asm/futex.h| 26 +++
arch/arm64/include/asm/futex.h | 26 +++
arch/frv/include/asm/futex.h| 3 ++-
arch/frv/kernel/futex.c | 27 +++-
arch/hexagon/include/asm/futex.h| 38 +++-
arch/ia64/include/asm/futex.h | 25 +++
arch/microblaze/include/asm/futex.h | 38 +++-
arch/mips/include/asm/futex.h | 25 +++
arch/openrisc/include/asm/futex.h | 39 +++--
arch/parisc/include/asm/futex.h | 26 +++
arch/powerpc/include/asm/futex.h| 26 ---
arch/s390/include/asm/futex.h | 23 -
arch/sh/include/asm/futex.h | 26 +++
arch/sparc/include/asm/futex_64.h | 26 ---
arch/tile/include/asm/futex.h | 40 -
arch/x86/include/asm/futex.h| 40 -
arch/xtensa/include/asm/futex.h | 27
include/asm-generic/futex.h | 50 +++--
kernel/futex.c | 39 +
21 files changed, 130 insertions(+), 506 deletions(-)
diff --git a/arch/alpha/include/asm/futex.h b/arch/alpha/include/asm/futex.h
index fb01dfb760c2..05a70edd57b6 100644
--- a/arch/alpha/include/asm/futex.h
+++ b/arch/alpha/include/asm/futex.h
@@ -25,18 +25,10 @@
Re: [PATCH 1/1] futex: remove duplicated code and fix UB
On 06/23/2017, 09:51 AM, Thomas Gleixner wrote:
> On Wed, 21 Jun 2017, Jiri Slaby wrote:
>> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
>> index f32b42e8725d..5bb2fd4674e7 100644
>> --- a/arch/arm64/include/asm/futex.h
>> +++ b/arch/arm64/include/asm/futex.h
>> @@ -48,20 +48,10 @@ do {
>> \
>> } while (0)
>>
>> static inline int
>> -futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
>
> That unsigned int seems to be a change from the arm64 tree in next. It's
> not upstream and it'll cause a (easy to resolve) conflict.
Ugh, I thought the arm64 is in upstream already. Note that this patch
just takes what is in this arm64 fix and makes it effective for all
architectures. So I will wait with v2 until it merges upstream.
So, Will, will you incorporate Thomas' comments into your arm64 fix?
...
> Yes, we probably can't change that anymore, but at least we should make it
> very explicit and add a comment to that effect.
Something like this or do you want a comment yet?
unsigned int op = (encoded_op & 0x7000) >> 28;
unsigned int cmp =(encoded_op & 0x0f00) >> 24;
int oparg = sign_extend32((encoded_op & 0x00fff000) >> 12, 12);
int cmparg = sign_extend32(encoded_op & 0x0fff, 12);
thanks,
--
js
suse labs
[PATCH 1/1] futex: remove duplicated code and fix UB
There is code duplicated over all architecture's headers for
futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
and comparison of the result.
Remove this duplication and leave up to the arches only the needed
assembly which is now in arch_futex_atomic_op_inuser.
This effectively distributes the Will Deacon's arm64 fix for undefined
behaviour reported by UBSAN to all architectures. The fix was done in
commit 5f16a046f8e1 (arm64: futex: Fix undefined behaviour with
FUTEX_OP_OPARG_SHIFT usage). Look there for an example dump.
Note that s390 removed access_ok check in d12a29703 ("s390/uaccess:
remove pointless access_ok() checks") as access_ok there returns true.
We introduce it back to the helper for the sake of simplicity (it gets
optimized away anyway).
Signed-off-by: Jiri Slaby <jsl...@suse.cz>
Cc: Richard Henderson <r...@twiddle.net>
Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru>
Cc: Matt Turner <matts...@gmail.com>
Cc: Vineet Gupta <vgu...@synopsys.com>
Acked-by: Russell King <rmk+ker...@armlinux.org.uk>
Cc: Catalin Marinas <catalin.mari...@arm.com>
Cc: Will Deacon <will.dea...@arm.com>
Cc: Richard Kuo <r...@codeaurora.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Fenghua Yu <fenghua...@intel.com>
Cc: Michal Simek <mon...@monstr.eu>
Cc: Ralf Baechle <r...@linux-mips.org>
Cc: Jonas Bonn <jo...@southpole.se>
Cc: Stefan Kristiansson <stefan.kristians...@saunalahti.fi>
Cc: Stafford Horne <sho...@gmail.com>
Cc: "James E.J. Bottomley" <j...@parisc-linux.org>
Cc: Helge Deller <del...@gmx.de>
Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
Cc: Paul Mackerras <pau...@samba.org>
Acked-by: Michael Ellerman <m...@ellerman.id.au> (powerpc)
Cc: Martin Schwidefsky <schwidef...@de.ibm.com>
Acked-by: Heiko Carstens <heiko.carst...@de.ibm.com> [s390]
Cc: Yoshinori Sato <ys...@users.sourceforge.jp>
Cc: Rich Felker <dal...@libc.org>
Cc: "David S. Miller" <da...@davemloft.net>
Acked-by: Chris Metcalf <cmetc...@mellanox.com> [for tile]
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: Chris Zankel <ch...@zankel.net>
Cc: Max Filippov <jcmvb...@gmail.com>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: <x...@kernel.org>
Cc: <linux-al...@vger.kernel.org>
Cc: <linux-ker...@vger.kernel.org>
Cc: <linux-snps-...@lists.infradead.org>
Cc: <linux-arm-ker...@lists.infradead.org>
Cc: <linux-hexa...@vger.kernel.org>
Cc: <linux-i...@vger.kernel.org>
Cc: <linux-m...@linux-mips.org>
Cc: <openr...@lists.librecores.org>
Cc: <linux-par...@vger.kernel.org>
Cc: <linuxppc-dev@lists.ozlabs.org>
Cc: <linux-s...@vger.kernel.org>
Cc: <linux...@vger.kernel.org>
Cc: <sparcli...@vger.kernel.org>
Cc: <linux-xte...@linux-xtensa.org>
Cc: <linux-a...@vger.kernel.org>
---
arch/alpha/include/asm/futex.h | 26 ---
arch/arc/include/asm/futex.h| 40 -
arch/arm/include/asm/futex.h| 26 +++
arch/arm64/include/asm/futex.h | 26 +++
arch/frv/include/asm/futex.h| 3 ++-
arch/frv/kernel/futex.c | 27 +++-
arch/hexagon/include/asm/futex.h| 38 +++-
arch/ia64/include/asm/futex.h | 25 +++
arch/microblaze/include/asm/futex.h | 38 +++-
arch/mips/include/asm/futex.h | 25 +++
arch/openrisc/include/asm/futex.h | 39 +++--
arch/parisc/include/asm/futex.h | 26 +++
arch/powerpc/include/asm/futex.h| 26 ---
arch/s390/include/asm/futex.h | 23 -
arch/sh/include/asm/futex.h | 26 +++
arch/sparc/include/asm/futex_64.h | 26 ---
arch/tile/include/asm/futex.h | 40 -
arch/x86/include/asm/futex.h| 40 -
arch/xtensa/include/asm/futex.h | 27
include/asm-generic/futex.h | 50 +++--
kernel/futex.c | 36 ++
21 files changed, 127 insertions(+), 506 deletions(-)
diff --git a/arch/alpha/include/asm/futex.h b/arch/alpha/include/asm/futex.h
index fb01dfb760c2..05a70edd57b6 100644
--- a/arch/alpha/include/asm/futex.h
+++ b/arch/alpha/include/asm/futex.h
@@ -25,18 +25,10 @@
: "r" (uaddr), "r"(oparg) \
: "memory")
-static inline int futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuse
Re: [PATCH 1/1] futex: remove duplicated code
On 05/15/2017, 03:16 PM, Will Deacon wrote:
> Whilst I think this is a good idea, the code in question actually results
> in undefined behaviour per the C spec and is reported by UBSAN.
Hi, yes, I know -- this patch was the 1st from the series of 3 which I
sent a long time ago to fix that up too. But I remember your patch, so I
sent only this one this time.
> See my
> patch fixing arm64 here (which I'd forgotten about):
>
> https://www.spinics.net/lists/linux-arch/msg38564.html
>
> But, as stated in the thread above, I think we should go a step further
> and remove FUTEX_OP_{OR,ANDN,XOR,OPARG_SHIFT} altogether. They don't
> appear to be used by userspace, and this whole thing is a total mess.
>
> Any thoughts?
Ok, I am all for that. I think the only question is who is going to do
the work and submit it :)? Do I understand correctly to eliminate all
these functions and the path into the kernel? But won't this break API
-- are there really no users of this interface?
thanks,
--
js
suse labs
[PATCH 1/1] futex: remove duplicated code
There is code duplicated over all architecture's headers for
futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
and comparison of the result.
Remove this duplication and leave up to the arches only the needed
assembly which is now in arch_futex_atomic_op_inuser.
Note that s390 removed access_ok check in d12a29703 ("s390/uaccess:
remove pointless access_ok() checks") as access_ok there returns true.
We introduce it back to the helper for the sake of simplicity (it gets
optimized away anyway).
Signed-off-by: Jiri Slaby <jsl...@suse.cz>
Cc: Richard Henderson <r...@twiddle.net>
Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru>
Cc: Matt Turner <matts...@gmail.com>
Cc: Vineet Gupta <vgu...@synopsys.com>
Acked-by: Russell King <rmk+ker...@armlinux.org.uk>
Cc: Catalin Marinas <catalin.mari...@arm.com>
Cc: Will Deacon <will.dea...@arm.com>
Cc: Richard Kuo <r...@codeaurora.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Fenghua Yu <fenghua...@intel.com>
Cc: Michal Simek <mon...@monstr.eu>
Cc: Ralf Baechle <r...@linux-mips.org>
Cc: Jonas Bonn <jo...@southpole.se>
Cc: Stefan Kristiansson <stefan.kristians...@saunalahti.fi>
Cc: Stafford Horne <sho...@gmail.com>
Cc: "James E.J. Bottomley" <j...@parisc-linux.org>
Cc: Helge Deller <del...@gmx.de>
Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
Cc: Paul Mackerras <pau...@samba.org>
Acked-by: Michael Ellerman <m...@ellerman.id.au> (powerpc)
Cc: Martin Schwidefsky <schwidef...@de.ibm.com>
Acked-by: Heiko Carstens <heiko.carst...@de.ibm.com> [s390]
Cc: Yoshinori Sato <ys...@users.sourceforge.jp>
Cc: Rich Felker <dal...@libc.org>
Cc: "David S. Miller" <da...@davemloft.net>
Acked-by: Chris Metcalf <cmetc...@mellanox.com> [for tile]
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: Chris Zankel <ch...@zankel.net>
Cc: Max Filippov <jcmvb...@gmail.com>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: <x...@kernel.org>
Cc: <linux-al...@vger.kernel.org>
Cc: <linux-ker...@vger.kernel.org>
Cc: <linux-snps-...@lists.infradead.org>
Cc: <linux-arm-ker...@lists.infradead.org>
Cc: <linux-hexa...@vger.kernel.org>
Cc: <linux-i...@vger.kernel.org>
Cc: <linux-m...@linux-mips.org>
Cc: <openr...@lists.librecores.org>
Cc: <linux-par...@vger.kernel.org>
Cc: <linuxppc-dev@lists.ozlabs.org>
Cc: <linux-s...@vger.kernel.org>
Cc: <linux...@vger.kernel.org>
Cc: <sparcli...@vger.kernel.org>
Cc: <linux-xte...@linux-xtensa.org>
Cc: <linux-a...@vger.kernel.org>
---
arch/alpha/include/asm/futex.h | 26 ---
arch/arc/include/asm/futex.h| 40 -
arch/arm/include/asm/futex.h| 26 +++
arch/arm64/include/asm/futex.h | 26 +++
arch/frv/include/asm/futex.h| 3 ++-
arch/frv/kernel/futex.c | 27 +++-
arch/hexagon/include/asm/futex.h| 38 +++-
arch/ia64/include/asm/futex.h | 25 +++
arch/microblaze/include/asm/futex.h | 38 +++-
arch/mips/include/asm/futex.h | 25 +++
arch/openrisc/include/asm/futex.h | 39 +++--
arch/parisc/include/asm/futex.h | 26 +++
arch/powerpc/include/asm/futex.h| 26 ---
arch/s390/include/asm/futex.h | 23 -
arch/sh/include/asm/futex.h | 26 +++
arch/sparc/include/asm/futex_64.h | 26 ---
arch/tile/include/asm/futex.h | 40 -
arch/x86/include/asm/futex.h| 40 -
arch/xtensa/include/asm/futex.h | 27
include/asm-generic/futex.h | 50 +++--
kernel/futex.c | 36 ++
21 files changed, 127 insertions(+), 506 deletions(-)
diff --git a/arch/alpha/include/asm/futex.h b/arch/alpha/include/asm/futex.h
index fb01dfb760c2..05a70edd57b6 100644
--- a/arch/alpha/include/asm/futex.h
+++ b/arch/alpha/include/asm/futex.h
@@ -25,18 +25,10 @@
: "r" (uaddr), "r"(oparg) \
: "memory")
-static inline int futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
+ u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op <<
[PATCH 1/3] futex: remove duplicated code
There is code duplicated over all architecture's headers for
futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
and comparison of the result.
Remove this duplication and leave up to the arches only the needed
assembly which is now in arch_futex_atomic_op_inuser.
Note that s390 removed access_ok check in d12a29703 ("s390/uaccess:
remove pointless access_ok() checks") as access_ok there returns true.
We introduce it back to the helper for the sake of simplicity (it gets
optimized away anyway).
Signed-off-by: Jiri Slaby <jsl...@suse.cz>
Cc: Richard Henderson <r...@twiddle.net>
Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru>
Cc: Matt Turner <matts...@gmail.com>
Cc: Vineet Gupta <vgu...@synopsys.com>
Cc: Russell King <li...@armlinux.org.uk>
Cc: Catalin Marinas <catalin.mari...@arm.com>
Cc: Will Deacon <will.dea...@arm.com>
Cc: Richard Kuo <r...@codeaurora.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Fenghua Yu <fenghua...@intel.com>
Cc: Michal Simek <mon...@monstr.eu>
Cc: Ralf Baechle <r...@linux-mips.org>
Cc: Jonas Bonn <jo...@southpole.se>
Cc: Stefan Kristiansson <stefan.kristians...@saunalahti.fi>
Cc: Stafford Horne <sho...@gmail.com>
Cc: "James E.J. Bottomley" <j...@parisc-linux.org>
Cc: Helge Deller <del...@gmx.de>
Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Michael Ellerman <m...@ellerman.id.au>
Cc: Martin Schwidefsky <schwidef...@de.ibm.com>
Cc: Heiko Carstens <heiko.carst...@de.ibm.com>
Cc: Yoshinori Sato <ys...@users.sourceforge.jp>
Cc: Rich Felker <dal...@libc.org>
Cc: "David S. Miller" <da...@davemloft.net>
Cc: Chris Metcalf <cmetc...@mellanox.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: Chris Zankel <ch...@zankel.net>
Cc: Max Filippov <jcmvb...@gmail.com>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: <x...@kernel.org>
Cc: <linux-al...@vger.kernel.org>
Cc: <linux-ker...@vger.kernel.org>
Cc: <linux-snps-...@lists.infradead.org>
Cc: <linux-arm-ker...@lists.infradead.org>
Cc: <linux-hexa...@vger.kernel.org>
Cc: <linux-i...@vger.kernel.org>
Cc: <linux-m...@linux-mips.org>
Cc: <openr...@lists.librecores.org>
Cc: <linux-par...@vger.kernel.org>
Cc: <linuxppc-dev@lists.ozlabs.org>
Cc: <linux-s...@vger.kernel.org>
Cc: <linux...@vger.kernel.org>
Cc: <sparcli...@vger.kernel.org>
Cc: <linux-xte...@linux-xtensa.org>
Cc: <linux-a...@vger.kernel.org>
---
arch/alpha/include/asm/futex.h | 26 ---
arch/arc/include/asm/futex.h| 40 -
arch/arm/include/asm/futex.h| 26 +++
arch/arm64/include/asm/futex.h | 26 +++
arch/frv/include/asm/futex.h| 3 ++-
arch/frv/kernel/futex.c | 27 +++-
arch/hexagon/include/asm/futex.h| 38 +++-
arch/ia64/include/asm/futex.h | 25 +++
arch/microblaze/include/asm/futex.h | 38 +++-
arch/mips/include/asm/futex.h | 25 +++
arch/openrisc/include/asm/futex.h | 39 +++--
arch/parisc/include/asm/futex.h | 26 +++
arch/powerpc/include/asm/futex.h| 26 ---
arch/s390/include/asm/futex.h | 23 -
arch/sh/include/asm/futex.h | 26 +++
arch/sparc/include/asm/futex_64.h | 26 ---
arch/tile/include/asm/futex.h | 40 -
arch/x86/include/asm/futex.h| 40 -
arch/xtensa/include/asm/futex.h | 27
include/asm-generic/futex.h | 50 +++--
kernel/futex.c | 36 ++
21 files changed, 127 insertions(+), 506 deletions(-)
diff --git a/arch/alpha/include/asm/futex.h b/arch/alpha/include/asm/futex.h
index f939794363ac..56474690e685 100644
--- a/arch/alpha/include/asm/futex.h
+++ b/arch/alpha/include/asm/futex.h
@@ -29,18 +29,10 @@
: "r" (uaddr), "r"(oparg) \
: "memory")
-static inline int futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
+ u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op &
[PATCH 3.12 143/235] powerpc: Fix build warning on 32-bit PPC
From: Larry Finger <larry.fin...@lwfinger.net>
3.12-stable review patch. If anyone has any objections, please let me know.
===
commit 8ae679c4bc2ea2d16d92620da8e3e9332fa4039f upstream.
I am getting the following warning when I build kernel 4.9-git on my
PowerBook G4 with a 32-bit PPC processor:
AS arch/powerpc/kernel/misc_32.o
arch/powerpc/kernel/misc_32.S:299:7: warning: "CONFIG_FSL_BOOKE" is not
defined [-Wundef]
This problem is evident after commit 989cea5c14be ("kbuild: prevent
lib-ksyms.o rebuilds"); however, this change in kbuild only exposes an
error that has been in the code since 2005 when this source file was
created. That was with commit 9994a33865f4 ("powerpc: Introduce
entry_{32,64}.S, misc_{32,64}.S, systbl.S").
The offending line does not make a lot of sense. This error does not
seem to cause any errors in the executable, thus I am not recommending
that it be applied to any stable versions.
Thanks to Nicholas Piggin for suggesting this solution.
Fixes: 9994a33865f4 ("powerpc: Introduce entry_{32,64}.S, misc_{32,64}.S,
systbl.S")
Signed-off-by: Larry Finger <larry.fin...@lwfinger.net>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Michael Ellerman <m...@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Linus Torvalds <torva...@linux-foundation.org>
Signed-off-by: Jiri Slaby <jsl...@suse.cz>
---
arch/powerpc/kernel/misc_32.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index ace34137a501..e23298f065df 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -313,7 +313,7 @@ _GLOBAL(flush_instruction_cache)
lis r3, KERNELBASE@h
iccci 0,r3
#endif
-#elif CONFIG_FSL_BOOKE
+#elif defined(CONFIG_FSL_BOOKE)
BEGIN_FTR_SECTION
mfspr r3,SPRN_L1CSR0
ori r3,r3,L1CSR0_CFI|L1CSR0_CLFC
--
2.11.0
[patch added to 3.12-stable] powerpc: Fix build warning on 32-bit PPC
From: Larry Finger <larry.fin...@lwfinger.net>
This patch has been added to the 3.12 stable tree. If you have any
objections, please let us know.
===
commit 8ae679c4bc2ea2d16d92620da8e3e9332fa4039f upstream.
I am getting the following warning when I build kernel 4.9-git on my
PowerBook G4 with a 32-bit PPC processor:
AS arch/powerpc/kernel/misc_32.o
arch/powerpc/kernel/misc_32.S:299:7: warning: "CONFIG_FSL_BOOKE" is not
defined [-Wundef]
This problem is evident after commit 989cea5c14be ("kbuild: prevent
lib-ksyms.o rebuilds"); however, this change in kbuild only exposes an
error that has been in the code since 2005 when this source file was
created. That was with commit 9994a33865f4 ("powerpc: Introduce
entry_{32,64}.S, misc_{32,64}.S, systbl.S").
The offending line does not make a lot of sense. This error does not
seem to cause any errors in the executable, thus I am not recommending
that it be applied to any stable versions.
Thanks to Nicholas Piggin for suggesting this solution.
Fixes: 9994a33865f4 ("powerpc: Introduce entry_{32,64}.S, misc_{32,64}.S,
systbl.S")
Signed-off-by: Larry Finger <larry.fin...@lwfinger.net>
Cc: Nicholas Piggin <npig...@gmail.com>
Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Michael Ellerman <m...@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Linus Torvalds <torva...@linux-foundation.org>
Signed-off-by: Jiri Slaby <jsl...@suse.cz>
---
arch/powerpc/kernel/misc_32.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index ace34137a501..e23298f065df 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -313,7 +313,7 @@ _GLOBAL(flush_instruction_cache)
lis r3, KERNELBASE@h
iccci 0,r3
#endif
-#elif CONFIG_FSL_BOOKE
+#elif defined(CONFIG_FSL_BOOKE)
BEGIN_FTR_SECTION
mfspr r3,SPRN_L1CSR0
ori r3,r3,L1CSR0_CFI|L1CSR0_CLFC
--
2.11.0
[PATCH v2] TTY: serial, handle platform_get_irq retval properly
platform_get_irq can fail, so we should handle negative value when
returned.
[v2]
platform_get_irq can actually return zero on some platforms. So do not
remove checks for irq == 0 there.
Signed-off-by: Jiri Slaby <jsl...@suse.cz>
Cc: Russell King <li...@arm.linux.org.uk>
Cc: "Uwe Kleine-König" <ker...@pengutronix.de>
Cc: Russell King <li...@armlinux.org.uk>
Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Michael Ellerman <m...@ellerman.id.au>
Cc: Laxman Dewangan <ldewan...@nvidia.com>
Cc: Stephen Warren <swar...@wwwdotorg.org>
Cc: Thierry Reding <thierry.red...@gmail.com>
Cc: Alexandre Courbot <gnu...@gmail.com>
Cc: linux-ser...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-te...@vger.kernel.org
---
drivers/tty/serial/amba-pl011.c | 8 +++-
drivers/tty/serial/fsl_lpuart.c | 8 +++-
drivers/tty/serial/pmac_zilog.c | 2 +-
drivers/tty/serial/serial-tegra.c | 7 ++-
4 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index a2aa655f56c4..c70bb41800f1 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2553,11 +2553,17 @@ static int sbsa_uart_probe(struct platform_device *pdev)
if (!uap)
return -ENOMEM;
+ ret = platform_get_irq(pdev, 0);
+ if (ret < 0) {
+ dev_err(>dev, "cannot obtain irq\n");
+ return ret;
+ }
+ uap->port.irq = ret;
+
uap->reg_offset = vendor_sbsa.reg_offset;
uap->vendor = _sbsa;
uap->fifosize = 32;
uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
- uap->port.irq = platform_get_irq(pdev, 0);
uap->port.ops = _uart_pops;
uap->fixed_baud = baudrate;
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 3d790033744e..7f95f782a485 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1830,7 +1830,13 @@ static int lpuart_probe(struct platform_device *pdev)
sport->port.dev = >dev;
sport->port.type = PORT_LPUART;
sport->port.iotype = UPIO_MEM;
- sport->port.irq = platform_get_irq(pdev, 0);
+ ret = platform_get_irq(pdev, 0);
+ if (ret < 0) {
+ dev_err(>dev, "cannot obtain irq\n");
+ return ret;
+ }
+ sport->port.irq = ret;
+
if (sport->lpuart32)
sport->port.ops = _pops;
else
diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index e156e39d620c..b24b0556f5a8 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -1720,7 +1720,7 @@ static int __init pmz_init_port(struct uart_pmac_port
*uap)
r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
irq = platform_get_irq(uap->pdev, 0);
- if (!r_ports || !irq)
+ if (!r_ports || irq <= 0)
return -ENODEV;
uap->port.mapbase = r_ports->start;
diff --git a/drivers/tty/serial/serial-tegra.c
b/drivers/tty/serial/serial-tegra.c
index bee1e5867426..4c4674b51db9 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -1310,7 +1310,12 @@ static int tegra_uart_probe(struct platform_device *pdev)
}
u->iotype = UPIO_MEM32;
- u->irq = platform_get_irq(pdev, 0);
+ ret = platform_get_irq(pdev, 0);
+ if (ret < 0) {
+ dev_err(>dev, "Couldn't get IRQ\n");
+ return ret;
+ }
+ u->irq = ret;
u->regshift = 2;
ret = uart_add_one_port(_uart_driver, u);
if (ret < 0) {
--
2.8.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 2/4] exit_thread: remove empty bodies
Define HAVE_EXIT_THREAD for archs which want to do something in exit_thread. For others, let's define exit_thread as an empty inline. This is a cleanup before we change the prototype of exit_thread to accept a task parameter. Signed-off-by: Jiri Slaby <jsl...@suse.cz> Cc: Richard Henderson <r...@twiddle.net> Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru> Cc: Matt Turner <matts...@gmail.com> Cc: Vineet Gupta <vgu...@synopsys.com> Cc: Russell King <li...@arm.linux.org.uk> Cc: Catalin Marinas <catalin.mari...@arm.com> Cc: Will Deacon <will.dea...@arm.com> Cc: Haavard Skinnemoen <hskinnem...@gmail.com> Cc: Hans-Christian Egtvedt <egtv...@samfundet.no> Cc: Steven Miao <real...@gmail.com> Cc: Mark Salter <msal...@redhat.com> Cc: Aurelien Jacquiot <a-jacqu...@ti.com> Cc: Mikael Starvik <star...@axis.com> Cc: Jesper Nilsson <jesper.nils...@axis.com> Cc: Yoshinori Sato <ys...@users.sourceforge.jp> Cc: Richard Kuo <r...@codeaurora.org> Cc: Geert Uytterhoeven <ge...@linux-m68k.org> Cc: James Hogan <james.ho...@imgtec.com> Cc: Michal Simek <mon...@monstr.eu> Cc: Ralf Baechle <r...@linux-mips.org> Cc: David Howells <dhowe...@redhat.com> Cc: Koichi Yasutake <yasutake.koi...@jp.panasonic.com> Cc: Ley Foon Tan <lf...@altera.com> Cc: Jonas Bonn <jo...@southpole.se> Cc: "James E.J. Bottomley" <j...@parisc-linux.org> Cc: Helge Deller <del...@gmx.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Paul Mackerras <pau...@samba.org> Cc: Michael Ellerman <m...@ellerman.id.au> Cc: Martin Schwidefsky <schwidef...@de.ibm.com> Cc: Heiko Carstens <heiko.carst...@de.ibm.com> Cc: Chen Liqin <liqin.li...@gmail.com> Cc: Lennox Wu <lennox...@gmail.com> Cc: Rich Felker <dal...@libc.org> Cc: "David S. Miller" <da...@davemloft.net> Cc: Chris Metcalf <cmetc...@mellanox.com> Cc: Jeff Dike <jd...@addtoit.com> Cc: Richard Weinberger <rich...@nod.at> Cc: Guan Xuetao <g...@mprc.pku.edu.cn> Cc: Thomas Gleixner <t...@linutronix.de> Cc: Ingo Molnar <mi...@redhat.com> Cc: "H. Peter Anvin" <h...@zytor.com> Cc: x...@kernel.org Cc: Tony Luck <tony.l...@intel.com> Cc: Fenghua Yu <fenghua...@intel.com> Cc: Chris Zankel <ch...@zankel.net> Cc: Max Filippov <jcmvb...@gmail.com> Cc: Peter Zijlstra <pet...@infradead.org> Cc: linux-ker...@vger.kernel.org Cc: linux-al...@vger.kernel.org Cc: linux-snps-...@lists.infradead.org Cc: linux-arm-ker...@lists.infradead.org Cc: adi-buildroot-de...@lists.sourceforge.net Cc: linux-c6x-...@linux-c6x.org Cc: linux-cris-ker...@axis.com Cc: linux-i...@vger.kernel.org Cc: uclinux-h8-de...@lists.sourceforge.jp Cc: linux-hexa...@vger.kernel.org Cc: linux-m...@lists.linux-m68k.org Cc: linux-me...@vger.kernel.org Cc: linux-m...@linux-mips.org Cc: linux-am33-l...@redhat.com Cc: nios2-...@lists.rocketboards.org Cc: linux-par...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-s...@vger.kernel.org Cc: linux...@vger.kernel.org Cc: sparcli...@vger.kernel.org Cc: user-mode-linux-de...@lists.sourceforge.net Cc: user-mode-linux-u...@lists.sourceforge.net Cc: linux-xte...@linux-xtensa.org --- arch/Kconfig| 5 + arch/alpha/kernel/process.c | 8 arch/arc/kernel/process.c | 7 --- arch/arm/Kconfig| 1 + arch/arm64/kernel/process.c | 7 --- arch/avr32/Kconfig | 1 + arch/blackfin/include/asm/processor.h | 7 --- arch/c6x/kernel/process.c | 4 arch/cris/Kconfig | 1 + arch/cris/arch-v10/kernel/process.c | 9 - arch/frv/include/asm/processor.h| 7 --- arch/h8300/include/asm/processor.h | 7 --- arch/hexagon/kernel/process.c | 7 --- arch/ia64/Kconfig | 1 + arch/m32r/kernel/process.c | 9 - arch/m68k/include/asm/processor.h | 7 --- arch/metag/Kconfig | 1 + arch/metag/include/asm/processor.h | 2 -- arch/microblaze/include/asm/processor.h | 10 -- arch/mips/include/asm/processor.h | 4 arch/mn10300/Kconfig| 1 + arch/nios2/include/asm/processor.h | 5 - arch/openrisc/include/asm/processor.h | 9 - arch/parisc/kernel/process.c| 7 --- arch/powerpc/kernel/process.c | 4 arch/s390/Kconfig | 1 + arch/score/kernel/process.c | 2 -- arch/sh/Kconfig | 1 + arch/sh/kernel/process_32.c | 7 --- arch/sparc/Kconfig | 1 + arch/tile/Kconfig | 1 + arch/um/kernel/process.c
Re: [PATCH 3/4] exit_thread: accept a task parameter to be exited
On 03/24/2016, 02:03 PM, Peter Zijlstra wrote:
> On Thu, Mar 24, 2016 at 01:58:13PM +0100, Jiri Slaby wrote:
>> void
>> -exit_thread(void)
>> +exit_thread(struct task_struct *me)
>> {
>> }
>
> task_struct arguments are called: tsk, task, p
> 'me' seems very wrong, as that could only mean 'current', and its
> clearly not that.
Ah, OK. I will wait for more feedback and will fix this.
thanks,
--
js
suse labs
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/4] exit_thread: accept a task parameter to be exited
We need to call exit_thread from copy_process in a fail path. So make it accept task_struct as a parameter. Signed-off-by: Jiri Slaby <jsl...@suse.cz> Cc: Richard Henderson <r...@twiddle.net> Cc: Ivan Kokshaysky <i...@jurassic.park.msu.ru> Cc: Matt Turner <matts...@gmail.com> Cc: Vineet Gupta <vgu...@synopsys.com> Cc: Russell King <li...@arm.linux.org.uk> Cc: Catalin Marinas <catalin.mari...@arm.com> Cc: Will Deacon <will.dea...@arm.com> Cc: Haavard Skinnemoen <hskinnem...@gmail.com> Cc: Hans-Christian Egtvedt <egtv...@samfundet.no> Cc: Steven Miao <real...@gmail.com> Cc: Mark Salter <msal...@redhat.com> Cc: Aurelien Jacquiot <a-jacqu...@ti.com> Cc: Mikael Starvik <star...@axis.com> Cc: Jesper Nilsson <jesper.nils...@axis.com> Cc: Yoshinori Sato <ys...@users.sourceforge.jp> Cc: Richard Kuo <r...@codeaurora.org> Cc: Tony Luck <tony.l...@intel.com> Cc: Fenghua Yu <fenghua...@intel.com> Cc: Geert Uytterhoeven <ge...@linux-m68k.org> Cc: James Hogan <james.ho...@imgtec.com> Cc: Michal Simek <mon...@monstr.eu> Cc: Ralf Baechle <r...@linux-mips.org> Cc: David Howells <dhowe...@redhat.com> Cc: Koichi Yasutake <yasutake.koi...@jp.panasonic.com> Cc: Ley Foon Tan <lf...@altera.com> Cc: Jonas Bonn <jo...@southpole.se> Cc: "James E.J. Bottomley" <j...@parisc-linux.org> Cc: Helge Deller <del...@gmx.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Paul Mackerras <pau...@samba.org> Cc: Michael Ellerman <m...@ellerman.id.au> Cc: Martin Schwidefsky <schwidef...@de.ibm.com> Cc: Heiko Carstens <heiko.carst...@de.ibm.com> Cc: Chen Liqin <liqin.li...@gmail.com> Cc: Lennox Wu <lennox...@gmail.com> Cc: Rich Felker <dal...@libc.org> Cc: "David S. Miller" <da...@davemloft.net> Cc: Chris Metcalf <cmetc...@mellanox.com> Cc: Jeff Dike <jd...@addtoit.com> Cc: Richard Weinberger <rich...@nod.at> Cc: Guan Xuetao <g...@mprc.pku.edu.cn> Cc: Thomas Gleixner <t...@linutronix.de> Cc: Ingo Molnar <mi...@redhat.com> Cc: "H. Peter Anvin" <h...@zytor.com> Cc: x...@kernel.org Cc: Chris Zankel <ch...@zankel.net> Cc: Max Filippov <jcmvb...@gmail.com> Cc: Peter Zijlstra <pet...@infradead.org> Cc: linux-al...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-snps-...@lists.infradead.org Cc: linux-arm-ker...@lists.infradead.org Cc: adi-buildroot-de...@lists.sourceforge.net Cc: linux-c6x-...@linux-c6x.org Cc: linux-cris-ker...@axis.com Cc: uclinux-h8-de...@lists.sourceforge.jp Cc: linux-hexa...@vger.kernel.org Cc: linux-i...@vger.kernel.org Cc: linux-m...@lists.linux-m68k.org Cc: linux-me...@vger.kernel.org Cc: linux-m...@linux-mips.org Cc: linux-am33-l...@redhat.com Cc: nios2-...@lists.rocketboards.org Cc: linux-par...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-s...@vger.kernel.org Cc: linux...@vger.kernel.org Cc: sparcli...@vger.kernel.org Cc: user-mode-linux-de...@lists.sourceforge.net Cc: user-mode-linux-u...@lists.sourceforge.net Cc: linux-xte...@linux-xtensa.org --- arch/alpha/kernel/process.c | 2 +- arch/arc/kernel/process.c | 2 +- arch/arm/kernel/process.c | 4 ++-- arch/arm64/kernel/process.c | 2 +- arch/avr32/kernel/process.c | 4 ++-- arch/blackfin/include/asm/processor.h | 2 +- arch/c6x/kernel/process.c | 2 +- arch/cris/arch-v10/kernel/process.c | 2 +- arch/cris/arch-v32/kernel/process.c | 4 ++-- arch/frv/include/asm/processor.h| 2 +- arch/h8300/include/asm/processor.h | 2 +- arch/hexagon/kernel/process.c | 2 +- arch/ia64/kernel/perfmon.c | 4 ++-- arch/ia64/kernel/process.c | 14 +++--- arch/m32r/kernel/process.c | 4 ++-- arch/m68k/include/asm/processor.h | 2 +- arch/metag/include/asm/processor.h | 2 +- arch/metag/kernel/process.c | 6 +++--- arch/microblaze/include/asm/processor.h | 4 ++-- arch/mips/kernel/process.c | 2 +- arch/mn10300/kernel/process.c | 4 ++-- arch/nios2/include/asm/processor.h | 2 +- arch/openrisc/include/asm/processor.h | 2 +- arch/parisc/kernel/process.c| 2 +- arch/powerpc/kernel/process.c | 2 +- arch/s390/kernel/process.c | 4 ++-- arch/score/kernel/process.c | 2 +- arch/sh/kernel/process_32.c | 2 +- arch/sh/kernel/process_64.c | 4 ++-- arch/sparc/kernel/process_32.c | 12 ++-- arch/sparc/kernel/process_64.c | 4 ++-- arch/tile/kernel/process.c | 4 ++-- arch/um/kernel/process.c| 2 +- arch/unicore32/kernel/process.c | 2 +- arch/x86/kernel/process.c | 3
Re: stable: Please include commit bb344ca5b90 (powerpc/mpc85xx: Add ranges to etsec2 nodes)
On 03/26/2015, 10:14 PM, Scott Wood wrote: Commit bb344ca5b90df6 (powerpc/mpc85xx: Add ranges to etsec2 nodes) fixes a bug that was exposed by commit 746c9e9f92dd (of/base: Fix PowerPC address parsing hack). The latter commit was applied to stable trees, so the former should be as well. Applied to 3.12. Thanks. -- js suse labs ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3.12 013/176] powerpc/powernv: Switch off MMU before entering nap/sleep/rvwinkle mode
From: Paul Mackerras pau...@samba.org 3.12-stable review patch. If anyone has any objections, please let me know. === commit 8117ac6a6c2fa0f847ff6a21a1f32c8d2c8501d0 upstream. Currently, when going idle, we set the flag indicating that we are in nap mode (paca-kvm_hstate.hwthread_state) and then execute the nap (or sleep or rvwinkle) instruction, all with the MMU on. This is bad for two reasons: (a) the architecture specifies that those instructions must be executed with the MMU off, and in fact with only the SF, HV, ME and possibly RI bits set, and (b) this introduces a race, because as soon as we set the flag, another thread can switch the MMU to a guest context. If the race is lost, this thread will typically start looping on relocation-on ISIs at 0xc...4400. This fixes it by setting the MSR as required by the architecture before setting the flag or executing the nap/sleep/rvwinkle instruction. [ shre...@linux.vnet.ibm.com: Edited to handle LE ] Signed-off-by: Paul Mackerras pau...@samba.org Signed-off-by: Shreyas B. Prabhu shre...@linux.vnet.ibm.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Michael Ellerman m...@ellerman.id.au Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Michael Ellerman m...@ellerman.id.au Signed-off-by: Jiri Slaby jsl...@suse.cz --- arch/powerpc/include/asm/reg.h| 1 + arch/powerpc/kernel/idle_power7.S | 16 2 files changed, 17 insertions(+) diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index cb9c1740cee0..390e09872b77 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -116,6 +116,7 @@ /* Server variant */ #define MSR_ (MSR_ME | MSR_RI | MSR_IR | MSR_DR | MSR_ISF |MSR_HV) +#define MSR_IDLE (MSR_ME | MSR_SF | MSR_HV) #define MSR_KERNEL (MSR_ | MSR_64BIT) #define MSR_USER32 (MSR_ | MSR_PR | MSR_EE) #define MSR_USER64 (MSR_USER32 | MSR_64BIT) diff --git a/arch/powerpc/kernel/idle_power7.S b/arch/powerpc/kernel/idle_power7.S index e11863f4e595..df930727f73b 100644 --- a/arch/powerpc/kernel/idle_power7.S +++ b/arch/powerpc/kernel/idle_power7.S @@ -84,6 +84,22 @@ _GLOBAL(power7_nap) std r9,_MSR(r1) std r1,PACAR1(r13) + /* +* Go to real mode to do the nap, as required by the architecture. +* Also, we need to be in real mode before setting hwthread_state, +* because as soon as we do that, another thread can switch +* the MMU context to the guest. +*/ + LOAD_REG_IMMEDIATE(r5, MSR_IDLE) + li r6, MSR_RI + andcr6, r9, r6 + LOAD_REG_ADDR(r7, power7_enter_nap_mode) + mtmsrd r6, 1 /* clear RI before setting SRR0/1 */ + mtspr SPRN_SRR0, r7 + mtspr SPRN_SRR1, r5 + rfid + +power7_enter_nap_mode: #ifdef CONFIG_KVM_BOOK3S_64_HV /* Tell KVM we're napping */ li r4,KVM_HWTHREAD_IN_NAP -- 2.2.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[patch added to the 3.12 stable tree] powerpc/powernv: Switch off MMU before entering nap/sleep/rvwinkle mode
From: Paul Mackerras pau...@samba.org This patch has been added to the 3.12 stable tree. If you have any objections, please let us know. === commit 8117ac6a6c2fa0f847ff6a21a1f32c8d2c8501d0 upstream. Currently, when going idle, we set the flag indicating that we are in nap mode (paca-kvm_hstate.hwthread_state) and then execute the nap (or sleep or rvwinkle) instruction, all with the MMU on. This is bad for two reasons: (a) the architecture specifies that those instructions must be executed with the MMU off, and in fact with only the SF, HV, ME and possibly RI bits set, and (b) this introduces a race, because as soon as we set the flag, another thread can switch the MMU to a guest context. If the race is lost, this thread will typically start looping on relocation-on ISIs at 0xc...4400. This fixes it by setting the MSR as required by the architecture before setting the flag or executing the nap/sleep/rvwinkle instruction. [ shre...@linux.vnet.ibm.com: Edited to handle LE ] Signed-off-by: Paul Mackerras pau...@samba.org Signed-off-by: Shreyas B. Prabhu shre...@linux.vnet.ibm.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Michael Ellerman m...@ellerman.id.au Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Michael Ellerman m...@ellerman.id.au Signed-off-by: Jiri Slaby jsl...@suse.cz --- arch/powerpc/include/asm/reg.h| 1 + arch/powerpc/kernel/idle_power7.S | 16 2 files changed, 17 insertions(+) diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index cb9c1740cee0..390e09872b77 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -116,6 +116,7 @@ /* Server variant */ #define MSR_ (MSR_ME | MSR_RI | MSR_IR | MSR_DR | MSR_ISF |MSR_HV) +#define MSR_IDLE (MSR_ME | MSR_SF | MSR_HV) #define MSR_KERNEL (MSR_ | MSR_64BIT) #define MSR_USER32 (MSR_ | MSR_PR | MSR_EE) #define MSR_USER64 (MSR_USER32 | MSR_64BIT) diff --git a/arch/powerpc/kernel/idle_power7.S b/arch/powerpc/kernel/idle_power7.S index e11863f4e595..df930727f73b 100644 --- a/arch/powerpc/kernel/idle_power7.S +++ b/arch/powerpc/kernel/idle_power7.S @@ -84,6 +84,22 @@ _GLOBAL(power7_nap) std r9,_MSR(r1) std r1,PACAR1(r13) + /* +* Go to real mode to do the nap, as required by the architecture. +* Also, we need to be in real mode before setting hwthread_state, +* because as soon as we do that, another thread can switch +* the MMU context to the guest. +*/ + LOAD_REG_IMMEDIATE(r5, MSR_IDLE) + li r6, MSR_RI + andcr6, r9, r6 + LOAD_REG_ADDR(r7, power7_enter_nap_mode) + mtmsrd r6, 1 /* clear RI before setting SRR0/1 */ + mtspr SPRN_SRR0, r7 + mtspr SPRN_SRR1, r5 + rfid + +power7_enter_nap_mode: #ifdef CONFIG_KVM_BOOK3S_64_HV /* Tell KVM we're napping */ li r4,KVM_HWTHREAD_IN_NAP -- 2.2.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3.12 71/94] locking/mutex: Disable optimistic spinning on some architectures
From: Peter Zijlstra pet...@infradead.org 3.12-stable review patch. If anyone has any objections, please let me know. === commit 4badad352a6bb202ec68afa7a574c0bb961e5ebc upstream. The optimistic spin code assumes regular stores and cmpxchg() play nice; this is found to not be true for at least: parisc, sparc32, tile32, metag-lock1, arc-!llsc and hexagon. There is further wreckage, but this in particular seemed easy to trigger, so blacklist this. Opt in for known good archs. Signed-off-by: Peter Zijlstra pet...@infradead.org Reported-by: Mikulas Patocka mpato...@redhat.com Cc: David Miller da...@davemloft.net Cc: Chris Metcalf cmetc...@tilera.com Cc: James Bottomley james.bottom...@hansenpartnership.com Cc: Vineet Gupta vgu...@synopsys.com Cc: Jason Low jason.l...@hp.com Cc: Waiman Long waiman.l...@hp.com Cc: James E.J. Bottomley j...@parisc-linux.org Cc: Paul McKenney paul...@linux.vnet.ibm.com Cc: John David Anglin dave.ang...@bell.net Cc: James Hogan james.ho...@imgtec.com Cc: Linus Torvalds torva...@linux-foundation.org Cc: Davidlohr Bueso davidl...@hp.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Catalin Marinas catalin.mari...@arm.com Cc: Russell King li...@arm.linux.org.uk Cc: Will Deacon will.dea...@arm.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-ker...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: sparcli...@vger.kernel.org Link: http://lkml.kernel.org/r/20140606175316.gv13...@laptop.programming.kicks-ass.net Signed-off-by: Ingo Molnar mi...@kernel.org Signed-off-by: Jiri Slaby jsl...@suse.cz --- arch/arm/Kconfig | 1 + arch/arm64/Kconfig | 1 + arch/powerpc/Kconfig | 1 + arch/sparc/Kconfig | 1 + arch/x86/Kconfig | 1 + kernel/Kconfig.locks | 5 - 6 files changed, 9 insertions(+), 1 deletion(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index e47fcd1e9645..99e1ce978cf9 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -5,6 +5,7 @@ config ARM select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAVE_CUSTOM_GPIO_H + select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_WANT_IPC_PARSE_VERSION select BUILDTIME_EXTABLE_SORT if MMU select CLONE_BACKWARDS diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index c04454876bcb..fe70eaea0e28 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1,6 +1,7 @@ config ARM64 def_bool y select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE + select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_WANT_OPTIONAL_GPIOLIB select ARCH_WANT_COMPAT_IPC_PARSE_VERSION select ARCH_WANT_FRAME_POINTERS diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index d5d026b6d237..2e0ddfadc0b9 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -138,6 +138,7 @@ config PPC select OLD_SIGSUSPEND select OLD_SIGACTION if PPC32 select HAVE_DEBUG_STACKOVERFLOW + select ARCH_SUPPORTS_ATOMIC_RMW config EARLY_PRINTK bool diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig index 4e5683877b93..d60f34dbae89 100644 --- a/arch/sparc/Kconfig +++ b/arch/sparc/Kconfig @@ -75,6 +75,7 @@ config SPARC64 select ARCH_HAVE_NMI_SAFE_CMPXCHG select HAVE_C_RECORDMCOUNT select NO_BOOTMEM + select ARCH_SUPPORTS_ATOMIC_RMW config ARCH_DEFCONFIG string diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index eb2dfa61eabe..9dc1a24d41b8 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -123,6 +123,7 @@ config X86 select COMPAT_OLD_SIGACTION if IA32_EMULATION select RTC_LIB select HAVE_DEBUG_STACKOVERFLOW + select ARCH_SUPPORTS_ATOMIC_RMW config INSTRUCTION_DECODER def_bool y diff --git a/kernel/Kconfig.locks b/kernel/Kconfig.locks index d2b32ac27a39..ecee67a00f5f 100644 --- a/kernel/Kconfig.locks +++ b/kernel/Kconfig.locks @@ -220,6 +220,9 @@ config INLINE_WRITE_UNLOCK_IRQRESTORE endif +config ARCH_SUPPORTS_ATOMIC_RMW + bool + config MUTEX_SPIN_ON_OWNER def_bool y - depends on SMP !DEBUG_MUTEXES + depends on SMP !DEBUG_MUTEXES ARCH_SUPPORTS_ATOMIC_RMW -- 2.0.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[patch added to the 3.12 stable tree] locking/mutex: Disable optimistic spinning on some architectures
From: Peter Zijlstra pet...@infradead.org This patch has been added to the 3.12 stable tree. If you have any objections, please let us know. === commit 4badad352a6bb202ec68afa7a574c0bb961e5ebc upstream. The optimistic spin code assumes regular stores and cmpxchg() play nice; this is found to not be true for at least: parisc, sparc32, tile32, metag-lock1, arc-!llsc and hexagon. There is further wreckage, but this in particular seemed easy to trigger, so blacklist this. Opt in for known good archs. Signed-off-by: Peter Zijlstra pet...@infradead.org Reported-by: Mikulas Patocka mpato...@redhat.com Cc: David Miller da...@davemloft.net Cc: Chris Metcalf cmetc...@tilera.com Cc: James Bottomley james.bottom...@hansenpartnership.com Cc: Vineet Gupta vgu...@synopsys.com Cc: Jason Low jason.l...@hp.com Cc: Waiman Long waiman.l...@hp.com Cc: James E.J. Bottomley j...@parisc-linux.org Cc: Paul McKenney paul...@linux.vnet.ibm.com Cc: John David Anglin dave.ang...@bell.net Cc: James Hogan james.ho...@imgtec.com Cc: Linus Torvalds torva...@linux-foundation.org Cc: Davidlohr Bueso davidl...@hp.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Catalin Marinas catalin.mari...@arm.com Cc: Russell King li...@arm.linux.org.uk Cc: Will Deacon will.dea...@arm.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-ker...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: sparcli...@vger.kernel.org Link: http://lkml.kernel.org/r/20140606175316.gv13...@laptop.programming.kicks-ass.net Signed-off-by: Ingo Molnar mi...@kernel.org Signed-off-by: Jiri Slaby jsl...@suse.cz --- arch/arm/Kconfig | 1 + arch/arm64/Kconfig | 1 + arch/powerpc/Kconfig | 1 + arch/sparc/Kconfig | 1 + arch/x86/Kconfig | 1 + kernel/Kconfig.locks | 5 - 6 files changed, 9 insertions(+), 1 deletion(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index e47fcd1e9645..99e1ce978cf9 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -5,6 +5,7 @@ config ARM select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAVE_CUSTOM_GPIO_H + select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_WANT_IPC_PARSE_VERSION select BUILDTIME_EXTABLE_SORT if MMU select CLONE_BACKWARDS diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index c04454876bcb..fe70eaea0e28 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1,6 +1,7 @@ config ARM64 def_bool y select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE + select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_WANT_OPTIONAL_GPIOLIB select ARCH_WANT_COMPAT_IPC_PARSE_VERSION select ARCH_WANT_FRAME_POINTERS diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index d5d026b6d237..2e0ddfadc0b9 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -138,6 +138,7 @@ config PPC select OLD_SIGSUSPEND select OLD_SIGACTION if PPC32 select HAVE_DEBUG_STACKOVERFLOW + select ARCH_SUPPORTS_ATOMIC_RMW config EARLY_PRINTK bool diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig index 4e5683877b93..d60f34dbae89 100644 --- a/arch/sparc/Kconfig +++ b/arch/sparc/Kconfig @@ -75,6 +75,7 @@ config SPARC64 select ARCH_HAVE_NMI_SAFE_CMPXCHG select HAVE_C_RECORDMCOUNT select NO_BOOTMEM + select ARCH_SUPPORTS_ATOMIC_RMW config ARCH_DEFCONFIG string diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index eb2dfa61eabe..9dc1a24d41b8 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -123,6 +123,7 @@ config X86 select COMPAT_OLD_SIGACTION if IA32_EMULATION select RTC_LIB select HAVE_DEBUG_STACKOVERFLOW + select ARCH_SUPPORTS_ATOMIC_RMW config INSTRUCTION_DECODER def_bool y diff --git a/kernel/Kconfig.locks b/kernel/Kconfig.locks index d2b32ac27a39..ecee67a00f5f 100644 --- a/kernel/Kconfig.locks +++ b/kernel/Kconfig.locks @@ -220,6 +220,9 @@ config INLINE_WRITE_UNLOCK_IRQRESTORE endif +config ARCH_SUPPORTS_ATOMIC_RMW + bool + config MUTEX_SPIN_ON_OWNER def_bool y - depends on SMP !DEBUG_MUTEXES + depends on SMP !DEBUG_MUTEXES ARCH_SUPPORTS_ATOMIC_RMW -- 2.0.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/2] ftrace: PPC, fix obsolete comment
CONFIG_MCOUNT is not defined anymore, the corresponding #ifdef there is CONFIG_FUNCTION_TRACER. Signed-off-by: Jiri Slaby jsl...@suse.cz Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Paul Mackerras pau...@samba.org Cc: linuxppc-dev@lists.ozlabs.org Cc: Steven Rostedt rost...@goodmis.org Cc: Frederic Weisbecker fweis...@gmail.com Cc: Ingo Molnar mi...@redhat.com --- arch/powerpc/kernel/entry_32.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S index 22b45a4955cd..f1f652a8f395 100644 --- a/arch/powerpc/kernel/entry_32.S +++ b/arch/powerpc/kernel/entry_32.S @@ -1457,4 +1457,4 @@ _GLOBAL(return_to_handler) blr #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ -#endif /* CONFIG_MCOUNT */ +#endif /* CONFIG_FUNCTION_TRACER */ -- 1.9.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] drivers/tty/hvc: using strlcpy instead of strncpy
On 03/05/2013 02:58 AM, Chen Gang wrote: 于 2013年02月28日 21:47, Jiri Slaby 写道: when strlen(pi-location_code[0]) == HVCS_CLC_LENGTH + 2 It cannot, pi-location_code is defined as char[HVCS_CLC_LENGTH + 1]. really, it is, I did not notice it. but I still prefer to modify it, but the patch should be changed such as: subject: beautify code: deleting useless judging code. comments: src buf len and dest buf len are the same, strcpy is better. contents: using strcpy instead of strncpy, and delete judging code. is it ok ? Yeah. BTW: sorry for my reply is too late, and did not notify it, originally before. I have to do some urgent things, during these days. my father had a serious heart disease, and is in hospital. No problem, these drivers are not so critical. Neither these code paths in them. Take care of your relatives first. -- js suse labs ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] drivers/tty/hvc: using strlcpy instead of strncpy
On 02/26/2013 04:43 AM, Chen Gang wrote:
when strlen pi-location_code is larger than HVCS_CLC_LENGTH + 1,
original implementation can not let hvcsd-p_location_code NUL terminated.
so need fix it (also can simplify the code)
It should never be larger because the +1 is exactly for NUL. But it is a
cleanup, so why not...
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/tty/hvc/hvcs.c |9 ++---
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
index 1956593..81e939e 100644
--- a/drivers/tty/hvc/hvcs.c
+++ b/drivers/tty/hvc/hvcs.c
@@ -881,17 +881,12 @@ static struct vio_driver hvcs_vio_driver = {
/* Only called from hvcs_get_pi please */
static void hvcs_set_pi(struct hvcs_partner_info *pi, struct hvcs_struct
*hvcsd)
{
- int clclength;
-
hvcsd-p_unit_address = pi-unit_address;
hvcsd-p_partition_ID = pi-partition_ID;
- clclength = strlen(pi-location_code[0]);
- if (clclength HVCS_CLC_LENGTH)
- clclength = HVCS_CLC_LENGTH;
/* copy the null-term char too */
- strncpy(hvcsd-p_location_code[0],
- pi-location_code[0], clclength + 1);
+ strlcpy(hvcsd-p_location_code[0],
+ pi-location_code[0], sizeof(hvcsd-p_location_code));
}
/*
--
js
suse labs
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] drivers/tty/hvc: using strlcpy instead of strncpy
On 02/28/2013 12:15 PM, Chen Gang wrote: 于 2013年02月28日 19:13, Chen Gang 写道: 于 2013年02月28日 18:41, Jiri Slaby 写道: On 02/26/2013 04:43 AM, Chen Gang wrote: when strlen pi-location_code is larger than HVCS_CLC_LENGTH + 1, original implementation can not let hvcsd-p_location_code NUL terminated. so need fix it (also can simplify the code) It should never be larger because the +1 is exactly for NUL. But it is a cleanup, so why not... when strlen(pi-location_code[0]) == HVCS_CLC_LENGTH + 2 It cannot, pi-location_code is defined as char[HVCS_CLC_LENGTH + 1]. -- js suse labs ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] TTY: hvc_console, fix port reference count going to zero prematurely
On 11/14/2012 09:15 AM, Paul Mackerras wrote:
Commit bdb498c20040 TTY: hvc_console, add tty install took the port
refcounting out of hvc_open()/hvc_close(), but failed to remove the
kref_put() and tty_kref_put() calls in hvc_hangup() that were there to
remove the extra references that hvc_open() had taken.
The result was that doing a vhangup() when the current terminal was
a hvc_console, then closing the current terminal, would end up calling
destroy_hvc_struct() and making the port disappear entirely. This
meant that Fedora 17 systems would boot up but then not display the
login prompt on the console, and attempts to open /dev/hvc0 would
give a No such device error.
This fixes it by removing the extra kref_put() and tty_kref_put() calls.
Oh yeah. Thanks.
Acked-by: Jiri Slaby jsl...@suse.cz
Signed-off-by: Paul Mackerras pau...@samba.org
Cc: sta...@vger.kernel.org
---
drivers/tty/hvc/hvc_console.c |7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index a5dec1c..13ee53b 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -424,7 +424,6 @@ static void hvc_hangup(struct tty_struct *tty)
{
struct hvc_struct *hp = tty-driver_data;
unsigned long flags;
- int temp_open_count;
if (!hp)
return;
@@ -444,7 +443,6 @@ static void hvc_hangup(struct tty_struct *tty)
return;
}
- temp_open_count = hp-port.count;
hp-port.count = 0;
spin_unlock_irqrestore(hp-port.lock, flags);
tty_port_tty_set(hp-port, NULL);
@@ -453,11 +451,6 @@ static void hvc_hangup(struct tty_struct *tty)
if (hp-ops-notifier_hangup)
hp-ops-notifier_hangup(hp, hp-data);
-
- while(temp_open_count) {
- --temp_open_count;
- tty_port_put(hp-port);
- }
}
/*
--
js
suse labs
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 39/41] TTY: hvc_console, add tty install
This has two outcomes:
* we give the TTY layer a tty_port
* we do not find the info structure every time open is called on that
tty
Since we take a reference to a port in -install, we need also
-cleanup to drop that reference.
Signed-off-by: Jiri Slaby jsl...@suse.cz
Cc: linuxppc-dev@lists.ozlabs.org
---
drivers/tty/hvc/hvc_console.c | 31 +--
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 2d691eb..7f80f15 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -299,20 +299,33 @@ static void hvc_unthrottle(struct tty_struct *tty)
hvc_kick();
}
+static int hvc_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+ struct hvc_struct *hp;
+ int rc;
+
+ /* Auto increments kref reference if found. */
+ if (!(hp = hvc_get_by_index(tty-index)))
+ return -ENODEV;
+
+ tty-driver_data = hp;
+
+ rc = tty_port_install(hp-port, driver, tty);
+ if (rc)
+ tty_port_put(hp-port);
+ return rc;
+}
+
/*
* The TTY interface won't be used until after the vio layer has exposed the
vty
* adapter to the kernel.
*/
static int hvc_open(struct tty_struct *tty, struct file * filp)
{
- struct hvc_struct *hp;
+ struct hvc_struct *hp = tty-driver_data;
unsigned long flags;
int rc = 0;
- /* Auto increments kref reference if found. */
- if (!(hp = hvc_get_by_index(tty-index)))
- return -ENODEV;
-
spin_lock_irqsave(hp-port.lock, flags);
/* Check and then increment for fast path open. */
if (hp-port.count++ 0) {
@@ -322,7 +335,6 @@ static int hvc_open(struct tty_struct *tty, struct file *
filp)
} /* else count == 0 */
spin_unlock_irqrestore(hp-port.lock, flags);
- tty-driver_data = hp;
tty_port_tty_set(hp-port, tty);
if (hp-ops-notifier_add)
@@ -389,6 +401,11 @@ static void hvc_close(struct tty_struct *tty, struct file
* filp)
hp-vtermno, hp-port.count);
spin_unlock_irqrestore(hp-port.lock, flags);
}
+}
+
+static void hvc_cleanup(struct tty_struct *tty)
+{
+ struct hvc_struct *hp = tty-driver_data;
tty_port_put(hp-port);
}
@@ -792,8 +809,10 @@ static void hvc_poll_put_char(struct tty_driver *driver,
int line, char ch)
#endif
static const struct tty_operations hvc_ops = {
+ .install = hvc_install,
.open = hvc_open,
.close = hvc_close,
+ .cleanup = hvc_cleanup,
.write = hvc_write,
.hangup = hvc_hangup,
.unthrottle = hvc_unthrottle,
--
1.7.10.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 41/41] TTY: hvcs, add tty install
This has two outcomes:
* we give the TTY layer a tty_port
* we do not find the info structure every time open is called on that
tty
From now on, we only increase the reference count in -install (and
decrease in -cleanup).
Signed-off-by: Jiri Slaby jsl...@suse.cz
Cc: linuxppc-dev@lists.ozlabs.org
---
drivers/tty/hvc/hvcs.c | 52 ++--
1 file changed, 33 insertions(+), 19 deletions(-)
diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
index 6f5c3be..cab5c7a 100644
--- a/drivers/tty/hvc/hvcs.c
+++ b/drivers/tty/hvc/hvcs.c
@@ -1102,11 +1102,7 @@ static struct hvcs_struct *hvcs_get_by_index(int index)
return NULL;
}
-/*
- * This is invoked via the tty_open interface when a user app connects to the
- * /dev node.
- */
-static int hvcs_open(struct tty_struct *tty, struct file *filp)
+static int hvcs_install(struct tty_driver *driver, struct tty_struct *tty)
{
struct hvcs_struct *hvcsd;
struct vio_dev *vdev;
@@ -1114,9 +1110,6 @@ static int hvcs_open(struct tty_struct *tty, struct file
*filp)
unsigned int irq;
int retval;
- if (tty-driver_data)
- goto fast_open;
-
/*
* Is there a vty-server that shares the same index?
* This function increments the kref index.
@@ -1139,7 +1132,7 @@ static int hvcs_open(struct tty_struct *tty, struct file
*filp)
}
}
- hvcsd-port.count = 1;
+ hvcsd-port.count = 0;
hvcsd-port.tty = tty;
tty-driver_data = hvcsd;
@@ -1166,28 +1159,42 @@ static int hvcs_open(struct tty_struct *tty, struct
file *filp)
goto err_put;
}
- goto open_success;
+ retval = tty_port_install(hvcsd-port, driver, tty);
+ if (retval)
+ goto err_irq;
-fast_open:
- hvcsd = tty-driver_data;
+ return 0;
+err_irq:
+ spin_lock_irqsave(hvcsd-lock, flags);
+ vio_disable_interrupts(hvcsd-vdev);
+ spin_unlock_irqrestore(hvcsd-lock, flags);
+ free_irq(irq, hvcsd);
+err_put:
+ tty_port_put(hvcsd-port);
+
+ return retval;
+}
+
+/*
+ * This is invoked via the tty_open interface when a user app connects to the
+ * /dev node.
+ */
+static int hvcs_open(struct tty_struct *tty, struct file *filp)
+{
+ struct hvcs_struct *hvcsd = tty-driver_data;
+ unsigned long flags;
spin_lock_irqsave(hvcsd-lock, flags);
- tty_port_get(hvcsd-port);
hvcsd-port.count++;
hvcsd-todo_mask |= HVCS_SCHED_READ;
spin_unlock_irqrestore(hvcsd-lock, flags);
-open_success:
hvcs_kick();
printk(KERN_INFO HVCS: vty-server@%X connection opened.\n,
hvcsd-vdev-unit_address );
return 0;
-err_put:
- tty_port_put(hvcsd-port);
-
- return retval;
}
static void hvcs_close(struct tty_struct *tty, struct file *filp)
@@ -1238,7 +1245,6 @@ static void hvcs_close(struct tty_struct *tty, struct
file *filp)
tty-driver_data = NULL;
free_irq(irq, hvcsd);
- tty_port_put(hvcsd-port);
return;
} else if (hvcsd-port.count 0) {
printk(KERN_ERR HVCS: vty-server@%X open_count: %d
@@ -1247,6 +1253,12 @@ static void hvcs_close(struct tty_struct *tty, struct
file *filp)
}
spin_unlock_irqrestore(hvcsd-lock, flags);
+}
+
+static void hvcs_cleanup(struct tty_struct * tty)
+{
+ struct hvcs_struct *hvcsd = tty-driver_data;
+
tty_port_put(hvcsd-port);
}
@@ -1433,8 +1445,10 @@ static int hvcs_chars_in_buffer(struct tty_struct *tty)
}
static const struct tty_operations hvcs_ops = {
+ .install = hvcs_install,
.open = hvcs_open,
.close = hvcs_close,
+ .cleanup = hvcs_cleanup,
.hangup = hvcs_hangup,
.write = hvcs_write,
.write_room = hvcs_write_room,
--
1.7.10.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: linux-next: boot failures with next-20120411
On 04/13/2012 04:30 AM, Michael Neuling wrote:
Stephen Rothwell s...@canb.auug.org.au wrote:
Hi all,
Some (not all) of my PowerPC boot tests have failed like this after
getting into user mode (this one was just after udev started, but others
are after other processes getting going):
Unable to handle kernel paging request for data at address 0xc003f9d550
Faulting instruction address: 0xc01b7f40
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=32 NUMA pSeries
Modules linked in: ehea
NIP: c01b7f40 LR: c01b7f14 CTR: c00e04f0
REGS: c003f68bf6b0 TRAP: 0300 Not tainted (3.4.0-rc2-autokern1)
MSR: 8280b032 SF,VEC,VSX,EE,FP,ME,IR,DR,RI CR: 24422424 XER:
2001
SOFTE: 1
CFAR: 562c
DAR: 00c003f9d550, DSISR: 4000
TASK = c003f8818000[3192] 'kdump' THREAD: c003f68bc000 CPU: 5
GPR00: c003f68bf930 c0ce1d40 c003fe00ec00
GPR04: 02d0 0038 c003f8f935e8 c0e55280
GPR08: 0011 c0bcb280 c0bcb1e8 0028a000
GPR12: 24422424 cf33bc80 0fffdd90a770 00081000
GPR16: c003f846c000 0de4f7a0 fde4f7a0
GPR20: c003f8365408 c003f8365480 c003f8e5d110
GPR24: 0100 c003f8365400 c01e5424 02d0
GPR28: 0800 00c003f9d550 c0c5b718 c003fe00ec00
NIP [c01b7f40] .__kmalloc+0x70/0x230
LR [c01b7f14] .__kmalloc+0x44/0x230
Call Trace:
[c003f68bf930] [c003f68bf9b0] 0xc003f68bf9b0 (unreliable)
[c003f68bf9e0] [c01e5424] .alloc_fdmem+0x24/0x70
[c003f68bfa60] [c01e54f8] .alloc_fdtable+0x88/0x130
[c003f68bfaf0] [c01e5924] .dup_fd+0x384/0x450
[c003f68bfbd0] [c009a310] .copy_process+0x880/0x11d0
[c003f68bfcd0] [c009aee0] .do_fork+0x70/0x400
[c003f68bfdc0] [c00141c4] .sys_clone+0x54/0x70
[c003f68bfe30] [c0009aa0] .ppc_clone+0x8/0xc
Instruction dump:
4bff9281 2ba30010 7c7f1b78 40dd00f4 e96d0040 e93f 7ce95a14 e9070008
7fa9582a 2fbd 41de0054 e81f0022 7f3d002a 3800 886d01f2 980d01f2
---[ end trace 366fe6c7ced3bfb0 ]---
This did not happen yesterday. Just wondering if anyone can think of
anything obvious. Full console log at
http://ozlabs.org/~sfr/next-20120411.log.bz2
I managed to bisect this down using pseries_defconfig with next-20120412
to this patch:
commit 85bbc003b24335e253a392f6a9874103b77abb36
Author: Jiri Slaby jsl...@suse.cz
Date: Mon Apr 2 13:54:22 2012 +0200
TTY: HVC, use tty from tty_port
The driver already used refcounting. So we just switch it to tty_port
helpers. And switch to tty_port-lock for tty.
Signed-off-by: Jiri Slaby jsl...@suse.cz
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
Reverting this commit (and 0146b6939074ebe14ece3604fd00e7be128a3812
otherwise git barfs) fixes the problem on next-20120412.
I'm assuming we got the ref count changes wrong somewhere in the patch
but the tty code is beyond me. Jiri, can you take a look?
Yeah, I see. I forgot to remove a couple of tty reference drops. The
reference is dropped by tty_port_tty_set in open/close/hangup now. Does
the attached patch help?
thanks,
--
js
suse labs
From cc51efe721f5aa184e119c52c661a1faf865e492 Mon Sep 17 00:00:00 2001
From: Jiri Slaby jsl...@suse.cz
Date: Fri, 13 Apr 2012 10:00:28 +0200
Subject: [PATCH 1/1] HVC: fix refcounting
Signed-off-by: Jiri Slaby jsl...@suse.cz
---
drivers/tty/hvc/hvc_console.c |3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 6c45cbf..260d4f2 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -338,7 +338,6 @@ static int hvc_open(struct tty_struct *tty, struct file * filp)
*/
if (rc) {
tty_port_tty_set(hp-port, NULL);
- tty_kref_put(tty);
tty-driver_data = NULL;
tty_port_put(hp-port);
printk(KERN_ERR hvc_open: request_irq failed with rc %d.\n, rc);
@@ -393,7 +392,6 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
spin_unlock_irqrestore(hp-port.lock, flags);
}
- tty_kref_put(tty);
tty_port_put(hp-port);
}
@@ -433,7 +431,6 @@ static void hvc_hangup(struct tty_struct *tty)
while(temp_open_count) {
--temp_open_count;
- tty_kref_put(tty);
tty_port_put(hp-port);
}
}
--
1.7.9.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: linux-next: boot failures with next-20120411
On 04/13/2012 10:02 AM, Jiri Slaby wrote:
On 04/13/2012 04:30 AM, Michael Neuling wrote:
Stephen Rothwell s...@canb.auug.org.au wrote:
Hi all,
Some (not all) of my PowerPC boot tests have failed like this after
getting into user mode (this one was just after udev started, but others
are after other processes getting going):
Unable to handle kernel paging request for data at address 0xc003f9d550
Faulting instruction address: 0xc01b7f40
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=32 NUMA pSeries
Modules linked in: ehea
NIP: c01b7f40 LR: c01b7f14 CTR: c00e04f0
REGS: c003f68bf6b0 TRAP: 0300 Not tainted (3.4.0-rc2-autokern1)
MSR: 8280b032 SF,VEC,VSX,EE,FP,ME,IR,DR,RI CR: 24422424 XER:
2001
SOFTE: 1
CFAR: 562c
DAR: 00c003f9d550, DSISR: 4000
TASK = c003f8818000[3192] 'kdump' THREAD: c003f68bc000 CPU: 5
GPR00: c003f68bf930 c0ce1d40 c003fe00ec00
GPR04: 02d0 0038 c003f8f935e8 c0e55280
GPR08: 0011 c0bcb280 c0bcb1e8 0028a000
GPR12: 24422424 cf33bc80 0fffdd90a770 00081000
GPR16: c003f846c000 0de4f7a0 fde4f7a0
GPR20: c003f8365408 c003f8365480 c003f8e5d110
GPR24: 0100 c003f8365400 c01e5424 02d0
GPR28: 0800 00c003f9d550 c0c5b718 c003fe00ec00
NIP [c01b7f40] .__kmalloc+0x70/0x230
LR [c01b7f14] .__kmalloc+0x44/0x230
Call Trace:
[c003f68bf930] [c003f68bf9b0] 0xc003f68bf9b0 (unreliable)
[c003f68bf9e0] [c01e5424] .alloc_fdmem+0x24/0x70
[c003f68bfa60] [c01e54f8] .alloc_fdtable+0x88/0x130
[c003f68bfaf0] [c01e5924] .dup_fd+0x384/0x450
[c003f68bfbd0] [c009a310] .copy_process+0x880/0x11d0
[c003f68bfcd0] [c009aee0] .do_fork+0x70/0x400
[c003f68bfdc0] [c00141c4] .sys_clone+0x54/0x70
[c003f68bfe30] [c0009aa0] .ppc_clone+0x8/0xc
Instruction dump:
4bff9281 2ba30010 7c7f1b78 40dd00f4 e96d0040 e93f 7ce95a14 e9070008
7fa9582a 2fbd 41de0054 e81f0022 7f3d002a 3800 886d01f2 980d01f2
---[ end trace 366fe6c7ced3bfb0 ]---
This did not happen yesterday. Just wondering if anyone can think of
anything obvious. Full console log at
http://ozlabs.org/~sfr/next-20120411.log.bz2
I managed to bisect this down using pseries_defconfig with next-20120412
to this patch:
commit 85bbc003b24335e253a392f6a9874103b77abb36
Author: Jiri Slaby jsl...@suse.cz
Date: Mon Apr 2 13:54:22 2012 +0200
TTY: HVC, use tty from tty_port
The driver already used refcounting. So we just switch it to tty_port
helpers. And switch to tty_port-lock for tty.
Signed-off-by: Jiri Slaby jsl...@suse.cz
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
Reverting this commit (and 0146b6939074ebe14ece3604fd00e7be128a3812
otherwise git barfs) fixes the problem on next-20120412.
I'm assuming we got the ref count changes wrong somewhere in the patch
but the tty code is beyond me. Jiri, can you take a look?
Yeah, I see. I forgot to remove a couple of tty reference drops. The
reference is dropped by tty_port_tty_set in open/close/hangup now. Does
the attached patch help?
And the patch is incomplete. Now we have a leak. This one should work.
thanks,
--
js
suse labs
From 7a55e2976cb5a47e499a6db335ad30ecac2e621c Mon Sep 17 00:00:00 2001
From: Jiri Slaby jsl...@suse.cz
Date: Fri, 13 Apr 2012 10:00:28 +0200
Subject: [PATCH 1/1] HVC: fix refcounting
Signed-off-by: Jiri Slaby jsl...@suse.cz
---
drivers/tty/hvc/hvc_console.c |5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 6c45cbf..2d691eb 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -317,8 +317,6 @@ static int hvc_open(struct tty_struct *tty, struct file * filp)
/* Check and then increment for fast path open. */
if (hp-port.count++ 0) {
spin_unlock_irqrestore(hp-port.lock, flags);
- /* FIXME why taking a reference here? */
- tty_kref_get(tty);
hvc_kick();
return 0;
} /* else count == 0 */
@@ -338,7 +336,6 @@ static int hvc_open(struct tty_struct *tty, struct file * filp)
*/
if (rc) {
tty_port_tty_set(hp-port, NULL);
- tty_kref_put(tty);
tty-driver_data = NULL;
tty_port_put(hp-port);
printk(KERN_ERR hvc_open: request_irq failed with rc %d.\n, rc);
@@ -393,7 +390,6 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
spin_unlock_irqrestore(hp-port.lock, flags);
}
- tty_kref_put(tty);
tty_port_put(hp-port);
}
@@ -433,7 +429,6 @@ static void hvc_hangup(struct tty_struct *tty)
while(temp_open_count
[PATCH 1/1] TTY: hvc, fix TTY refcounting
A -next commit TTY: HVC, use tty from tty_port switched the driver
to use tty_port helper for tty refcounting. But it omitted to remove
manual tty refcounting from open, close and hangup. So now we are
getting random crashes caused by use-after-free:
Unable to handle kernel paging request for data at address 0xc003f9d550
Faulting instruction address: 0xc01b7f40
Oops: Kernel access of bad area, sig: 11 [#1]
...
NIP: c01b7f40 LR: c01b7f14 CTR: c00e04f0
...
NIP [c01b7f40] .__kmalloc+0x70/0x230
LR [c01b7f14] .__kmalloc+0x44/0x230
Call Trace:
[c003f68bf930] [c003f68bf9b0] 0xc003f68bf9b0 (unreliable)
[c003f68bf9e0] [c01e5424] .alloc_fdmem+0x24/0x70
[c003f68bfa60] [c01e54f8] .alloc_fdtable+0x88/0x130
[c003f68bfaf0] [c01e5924] .dup_fd+0x384/0x450
[c003f68bfbd0] [c009a310] .copy_process+0x880/0x11d0
[c003f68bfcd0] [c009aee0] .do_fork+0x70/0x400
[c003f68bfdc0] [c00141c4] .sys_clone+0x54/0x70
[c003f68bfe30] [c0009aa0] .ppc_clone+0x8/0xc
Fix that by complete removal of tty_kref_get/put in open/close/hangup
paths.
Signed-off-by: Jiri Slaby jsl...@suse.cz
Reported-and-tested-by: Michael Neuling mi...@neuling.org
Cc: Stephen Rothwell s...@canb.auug.org.au
Cc: ppc-dev linuxppc-dev@lists.ozlabs.org
---
drivers/tty/hvc/hvc_console.c |5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 6c45cbf..2d691eb 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -317,8 +317,6 @@ static int hvc_open(struct tty_struct *tty, struct file *
filp)
/* Check and then increment for fast path open. */
if (hp-port.count++ 0) {
spin_unlock_irqrestore(hp-port.lock, flags);
- /* FIXME why taking a reference here? */
- tty_kref_get(tty);
hvc_kick();
return 0;
} /* else count == 0 */
@@ -338,7 +336,6 @@ static int hvc_open(struct tty_struct *tty, struct file *
filp)
*/
if (rc) {
tty_port_tty_set(hp-port, NULL);
- tty_kref_put(tty);
tty-driver_data = NULL;
tty_port_put(hp-port);
printk(KERN_ERR hvc_open: request_irq failed with rc %d.\n,
rc);
@@ -393,7 +390,6 @@ static void hvc_close(struct tty_struct *tty, struct file *
filp)
spin_unlock_irqrestore(hp-port.lock, flags);
}
- tty_kref_put(tty);
tty_port_put(hp-port);
}
@@ -433,7 +429,6 @@ static void hvc_hangup(struct tty_struct *tty)
while(temp_open_count) {
--temp_open_count;
- tty_kref_put(tty);
tty_port_put(hp-port);
}
}
--
1.7.9.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 42/69] TTY: hvcs, use tty from tty_port
No refcounting, just a switch. The locking in the driver prevents
races, so in fact the refcounting is not needed. But while we have a
tty in tty_port, don't duplicate that and remove the one from
hvcs_struct.
Signed-off-by: Jiri Slaby jsl...@suse.cz
Cc: linuxppc-dev@lists.ozlabs.org
---
drivers/tty/hvc/hvcs.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
index 817f94b..d56788c 100644
--- a/drivers/tty/hvc/hvcs.c
+++ b/drivers/tty/hvc/hvcs.c
@@ -270,8 +270,6 @@ struct hvcs_struct {
*/
unsigned int index;
- struct tty_struct *tty;
-
/*
* Used to tell the driver kernel_thread what operations need to take
* place upon this hvcs_struct instance.
@@ -560,7 +558,7 @@ static irqreturn_t hvcs_handle_interrupt(int irq, void
*dev_instance)
static void hvcs_try_write(struct hvcs_struct *hvcsd)
{
uint32_t unit_address = hvcsd-vdev-unit_address;
- struct tty_struct *tty = hvcsd-tty;
+ struct tty_struct *tty = hvcsd-port.tty;
int sent;
if (hvcsd-todo_mask HVCS_TRY_WRITE) {
@@ -598,7 +596,7 @@ static int hvcs_io(struct hvcs_struct *hvcsd)
spin_lock_irqsave(hvcsd-lock, flags);
unit_address = hvcsd-vdev-unit_address;
- tty = hvcsd-tty;
+ tty = hvcsd-port.tty;
hvcs_try_write(hvcsd);
@@ -850,7 +848,7 @@ static int __devexit hvcs_remove(struct vio_dev *dev)
spin_lock_irqsave(hvcsd-lock, flags);
- tty = hvcsd-tty;
+ tty = hvcsd-port.tty;
spin_unlock_irqrestore(hvcsd-lock, flags);
@@ -1137,7 +1135,7 @@ static int hvcs_open(struct tty_struct *tty, struct file
*filp)
goto error_release;
hvcsd-port.count = 1;
- hvcsd-tty = tty;
+ hvcsd-port.tty = tty;
tty-driver_data = hvcsd;
memset(hvcsd-buffer[0], 0x00, HVCS_BUFF_LEN);
@@ -1223,7 +1221,7 @@ static void hvcs_close(struct tty_struct *tty, struct
file *filp)
* execute any operations on the TTY even though it is obligated
* to deliver any pending I/O to the hypervisor.
*/
- hvcsd-tty = NULL;
+ hvcsd-port.tty = NULL;
irq = hvcsd-vdev-irq;
spin_unlock_irqrestore(hvcsd-lock, flags);
@@ -1271,8 +1269,8 @@ static void hvcs_hangup(struct tty_struct * tty)
hvcsd-todo_mask = 0;
/* I don't think the tty needs the hvcs_struct pointer after a hangup */
- hvcsd-tty-driver_data = NULL;
- hvcsd-tty = NULL;
+ tty-driver_data = NULL;
+ hvcsd-port.tty = NULL;
hvcsd-port.count = 0;
--
1.7.9.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
