Re: [PATCH 1/2] printk: Add dictionary information in structure cont

2013-12-20 Thread Kay Sievers
On Fri, Dec 20, 2013 at 10:41 AM, Yoshihiro YUNOMAE
 wrote:
> Add dictionary information in structure cont.
> Dictionary information is added when a driver uses structured printk, and the
> information is shown in /dev/kmsg. Current kernel directly stores the
> information to log_buf. This patch stores the dict information in structure 
> cont
> first, then the information in cont is stored to log_buf.
>
> Signed-off-by: Yoshihiro YUNOMAE 
> Cc: Kay Sievers 
> Cc: Andrew Morton 
> Cc: Joe Perches 
> Cc: Tejun Heo 
> Cc: Frederic Weisbecker 
> Cc: linux-kernel@vger.kernel.org

This is wrong. Please do not apply. Racy ontinuation lines and
strutured logging should not be mixed. Continuation lines are a nice
kernel hack for debugging, useful to have, but cannot be trusted by
userspace. The entire purpose of structured logging is to have
trustable atomic logging messages, the both facilities should not be
mixed.

Thanks,
Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/2] printk: Add dictionary information in structure cont

2013-12-20 Thread Yoshihiro YUNOMAE
Add dictionary information in structure cont.
Dictionary information is added when a driver uses structured printk, and the
information is shown in /dev/kmsg. Current kernel directly stores the
information to log_buf. This patch stores the dict information in structure cont
first, then the information in cont is stored to log_buf.

Signed-off-by: Yoshihiro YUNOMAE 
Cc: Kay Sievers 
Cc: Andrew Morton 
Cc: Joe Perches 
Cc: Tejun Heo 
Cc: Frederic Weisbecker 
Cc: linux-kernel@vger.kernel.org
---
 kernel/printk/printk.c |   70 
 1 file changed, 47 insertions(+), 23 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index be7c86b..c3662e6 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1391,8 +1391,10 @@ static inline void printk_delay(void)
  * reached the console in case of a kernel crash.
  */
 static struct cont {
-   char buf[LOG_LINE_MAX];
-   size_t len; /* length == 0 means unused buffer */
+   char text[LOG_LINE_MAX];
+   size_t text_len;/* length == 0 means unused buffer */
+   char dict[LOG_LINE_MAX];/* stores dict */
+   size_t dict_len;/* 0 means dict is unstored */
size_t cons;/* bytes written to console */
struct task_struct *owner;  /* task of first print*/
u64 ts_nsec;/* time of first print */
@@ -1406,7 +1408,7 @@ static void cont_flush(enum log_flags flags)
 {
if (cont.flushed)
return;
-   if (cont.len == 0)
+   if (cont.text_len == 0)
return;
 
if (cont.cons) {
@@ -1416,7 +1418,7 @@ static void cont_flush(enum log_flags flags)
 * line. LOG_NOCONS suppresses a duplicated output.
 */
log_store(cont.facility, cont.level, flags | LOG_NOCONS,
- cont.ts_nsec, NULL, 0, cont.buf, cont.len);
+ cont.ts_nsec, NULL, 0, cont.text, cont.text_len);
cont.flags = flags;
cont.flushed = true;
} else {
@@ -1425,23 +1427,32 @@ static void cont_flush(enum log_flags flags)
 * just submit it to the store and free the buffer.
 */
log_store(cont.facility, cont.level, flags, 0,
- NULL, 0, cont.buf, cont.len);
-   cont.len = 0;
+ NULL, 0, cont.text, cont.text_len);
+   cont.text_len = 0;
}
 }
 
-static bool cont_add(int facility, int level, const char *text, size_t len)
+static void cont_dict_add(const char *dict, size_t dict_len)
 {
-   if (cont.len && cont.flushed)
+   if (cont.dict_len + dict_len > sizeof(cont.dict))
+   return;
+
+   memcpy(cont.dict + cont.dict_len, dict, dict_len);
+   cont.dict_len += dict_len;
+}
+
+static bool cont_add(int facility, int level, const char *text, size_t 
text_len)
+{
+   if (cont.text_len && cont.flushed)
return false;
 
-   if (cont.len + len > sizeof(cont.buf)) {
+   if (cont.text_len + text_len > sizeof(cont.text)) {
/* the line gets too long, split it up in separate records */
cont_flush(LOG_CONT);
return false;
}
 
-   if (!cont.len) {
+   if (!cont.text_len) {
cont.facility = facility;
cont.level = level;
cont.owner = current;
@@ -1451,10 +1462,10 @@ static bool cont_add(int facility, int level, const 
char *text, size_t len)
cont.flushed = false;
}
 
-   memcpy(cont.buf + cont.len, text, len);
-   cont.len += len;
+   memcpy(cont.text + cont.text_len, text, text_len);
+   cont.text_len += text_len;
 
-   if (cont.len > (sizeof(cont.buf) * 80) / 100)
+   if (cont.text_len > (sizeof(cont.text) * 80) / 100)
cont_flush(LOG_CONT);
 
return true;
@@ -1470,20 +1481,20 @@ static size_t cont_print_text(char *text, size_t size)
size -= textlen;
}
 
-   len = cont.len - cont.cons;
+   len = cont.text_len - cont.cons;
if (len > 0) {
if (len+1 > size)
len = size-1;
-   memcpy(text + textlen, cont.buf + cont.cons, len);
+   memcpy(text + textlen, cont.text + cont.cons, len);
textlen += len;
-   cont.cons = cont.len;
+   cont.cons = cont.text_len;
}
 
if (cont.flushed) {
if (cont.flags & LOG_NEWLINE)
text[textlen++] = '\n';
/* got everything, release buffer */
-   cont.len = 0;
+   cont.text_len = 0;
}
return textlen;
 }
@@ -1576,21 +1587,29 @@ asmlinkage int vprintk_emit(int facility, int level,
if (level == -1)
level = 

[PATCH 1/2] printk: Add dictionary information in structure cont

2013-12-20 Thread Yoshihiro YUNOMAE
Add dictionary information in structure cont.
Dictionary information is added when a driver uses structured printk, and the
information is shown in /dev/kmsg. Current kernel directly stores the
information to log_buf. This patch stores the dict information in structure cont
first, then the information in cont is stored to log_buf.

Signed-off-by: Yoshihiro YUNOMAE yoshihiro.yunomae...@hitachi.com
Cc: Kay Sievers k...@vrfy.org
Cc: Andrew Morton a...@linux-foundation.org
Cc: Joe Perches j...@perches.com
Cc: Tejun Heo t...@kernel.org
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: linux-kernel@vger.kernel.org
---
 kernel/printk/printk.c |   70 
 1 file changed, 47 insertions(+), 23 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index be7c86b..c3662e6 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1391,8 +1391,10 @@ static inline void printk_delay(void)
  * reached the console in case of a kernel crash.
  */
 static struct cont {
-   char buf[LOG_LINE_MAX];
-   size_t len; /* length == 0 means unused buffer */
+   char text[LOG_LINE_MAX];
+   size_t text_len;/* length == 0 means unused buffer */
+   char dict[LOG_LINE_MAX];/* stores dict */
+   size_t dict_len;/* 0 means dict is unstored */
size_t cons;/* bytes written to console */
struct task_struct *owner;  /* task of first print*/
u64 ts_nsec;/* time of first print */
@@ -1406,7 +1408,7 @@ static void cont_flush(enum log_flags flags)
 {
if (cont.flushed)
return;
-   if (cont.len == 0)
+   if (cont.text_len == 0)
return;
 
if (cont.cons) {
@@ -1416,7 +1418,7 @@ static void cont_flush(enum log_flags flags)
 * line. LOG_NOCONS suppresses a duplicated output.
 */
log_store(cont.facility, cont.level, flags | LOG_NOCONS,
- cont.ts_nsec, NULL, 0, cont.buf, cont.len);
+ cont.ts_nsec, NULL, 0, cont.text, cont.text_len);
cont.flags = flags;
cont.flushed = true;
} else {
@@ -1425,23 +1427,32 @@ static void cont_flush(enum log_flags flags)
 * just submit it to the store and free the buffer.
 */
log_store(cont.facility, cont.level, flags, 0,
- NULL, 0, cont.buf, cont.len);
-   cont.len = 0;
+ NULL, 0, cont.text, cont.text_len);
+   cont.text_len = 0;
}
 }
 
-static bool cont_add(int facility, int level, const char *text, size_t len)
+static void cont_dict_add(const char *dict, size_t dict_len)
 {
-   if (cont.len  cont.flushed)
+   if (cont.dict_len + dict_len  sizeof(cont.dict))
+   return;
+
+   memcpy(cont.dict + cont.dict_len, dict, dict_len);
+   cont.dict_len += dict_len;
+}
+
+static bool cont_add(int facility, int level, const char *text, size_t 
text_len)
+{
+   if (cont.text_len  cont.flushed)
return false;
 
-   if (cont.len + len  sizeof(cont.buf)) {
+   if (cont.text_len + text_len  sizeof(cont.text)) {
/* the line gets too long, split it up in separate records */
cont_flush(LOG_CONT);
return false;
}
 
-   if (!cont.len) {
+   if (!cont.text_len) {
cont.facility = facility;
cont.level = level;
cont.owner = current;
@@ -1451,10 +1462,10 @@ static bool cont_add(int facility, int level, const 
char *text, size_t len)
cont.flushed = false;
}
 
-   memcpy(cont.buf + cont.len, text, len);
-   cont.len += len;
+   memcpy(cont.text + cont.text_len, text, text_len);
+   cont.text_len += text_len;
 
-   if (cont.len  (sizeof(cont.buf) * 80) / 100)
+   if (cont.text_len  (sizeof(cont.text) * 80) / 100)
cont_flush(LOG_CONT);
 
return true;
@@ -1470,20 +1481,20 @@ static size_t cont_print_text(char *text, size_t size)
size -= textlen;
}
 
-   len = cont.len - cont.cons;
+   len = cont.text_len - cont.cons;
if (len  0) {
if (len+1  size)
len = size-1;
-   memcpy(text + textlen, cont.buf + cont.cons, len);
+   memcpy(text + textlen, cont.text + cont.cons, len);
textlen += len;
-   cont.cons = cont.len;
+   cont.cons = cont.text_len;
}
 
if (cont.flushed) {
if (cont.flags  LOG_NEWLINE)
text[textlen++] = '\n';
/* got everything, release buffer */
-   cont.len = 0;
+   cont.text_len = 0;
}
return textlen;
 }
@@ -1576,21 

Re: [PATCH 1/2] printk: Add dictionary information in structure cont

2013-12-20 Thread Kay Sievers
On Fri, Dec 20, 2013 at 10:41 AM, Yoshihiro YUNOMAE
yoshihiro.yunomae...@hitachi.com wrote:
 Add dictionary information in structure cont.
 Dictionary information is added when a driver uses structured printk, and the
 information is shown in /dev/kmsg. Current kernel directly stores the
 information to log_buf. This patch stores the dict information in structure 
 cont
 first, then the information in cont is stored to log_buf.

 Signed-off-by: Yoshihiro YUNOMAE yoshihiro.yunomae...@hitachi.com
 Cc: Kay Sievers k...@vrfy.org
 Cc: Andrew Morton a...@linux-foundation.org
 Cc: Joe Perches j...@perches.com
 Cc: Tejun Heo t...@kernel.org
 Cc: Frederic Weisbecker fweis...@gmail.com
 Cc: linux-kernel@vger.kernel.org

This is wrong. Please do not apply. Racy ontinuation lines and
strutured logging should not be mixed. Continuation lines are a nice
kernel hack for debugging, useful to have, but cannot be trusted by
userspace. The entire purpose of structured logging is to have
trustable atomic logging messages, the both facilities should not be
mixed.

Thanks,
Kay
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/