[Devel] [PATCH] kio_pcs: Compact the logs and make eyes happy

[Kirill Korotaev]

Guys, JFYI, we are getting rid of LOG_NONL and indent in logger as non multi-thread safe (we are moving towards multi threaded eventloop):
https://git.acronis.com/projects/STRG/repos/pstorage-core/pull-requests/765/diff


On 5 Dec 2018, at 12:29, Pavel Butsykin <pbutsykin at virtuozzo.com<mailto:pbutsykin at virtuozzo.com>> wrote:

On 04.12.2018 23:13, Pavel Emelianov wrote:
On 12/04/2018 10:28 PM, Pavel Emelyanov wrote:
On 12/03/2018 05:20 PM, Pavel Butsykin wrote:
On 30.11.2018 19:44, Pavel Emelianov wrote:
On 11/30/2018 05:57 PM, Pavel Butsykin wrote:
Hmm, __kfuse_trace() duplicates message to debugfs trace, without '\n'

Yes, and AFAIS most of the existing messages already come w/o it.

More likely logs without '\n' are present mainly for error cases,
perhaps this is why it wasn't noticed before. But yes, we have a mess
with logs.

I propose to fix all FUSE_K* traces with '\n' and use
trace_printk("%s\n") for duplicating messages to debugsfs. But
duplicating messages to debugfs should be turned off by default (with
the ability to enable via sysfs), because write messages to debugfs
without user's permission.. it's looks a little impolite :)

The issue is not in duplicating messages, but in \n creeping into
user-space logs. But anyway, OK, equipping them all with \n-s sounds
OK to me.

Not any more. Please, look at this PR: https://git.acronis.com/projects/STRG/repos/pstorage-core/pull-requests/765
The proposal is to drop the LOG_NONL thing from textual logs. If done, then
messages coming with \n at the end will have double \n-s in text logs.

Actually, we don't need to worry about merged messages in debugs,
because __trace_puts() adds \n if it necessary:
int __trace_puts(unsigned long ip, const char *str, int size)
...
/* Add a newline if necessary */
if (entry->buf[size - 1] != '\n') {
entry->buf[size] = '\n';
entry->buf[size + 1] = '\0';
} else
entry->buf[size] = '\0';

I missed it in the beginning, sorry for that.

Along with this, we can allow drop LOG_NONL options from usermode and
this patch is OK for me, except for a small correction:
void __kfuse_trace(struct fuse_conn * fc, unsigned long ip, const char *
fmt, ...)
...
- pr_debug("%s", buf);
+ pr_debug("%s\n", buf);

this messages will look weird. We need to fix __kfuse_trace() by adding
concatenation '\n' (or even drop duplication messages to debugsfs/dmesg,
as not a very useful thing).

On 30.11.2018 17:35, Pavel Butsykin wrote:
Would you like to fix all FUSE_KTRACE's in the kernel? I see a few more
FUSE_KTRACE with '\n'.

On 30.11.2018 17:23, Pavel Emelianov wrote:
After a recent (b)log (re)work it was noticed that some extra \n-s crept
into the classical log files. Indeed, the pstorage TRACE() call appends a
\n at the end of each message and so does the kernel code.

Sorry for inconvenience.

https://pmc.acronis.com/browse/VSTOR-18383

Signed-off-by: Pavel Emelyanov <xemul at virtuozzo.com>

---

diff --git a/fs/fuse/kio/pcs/pcs_map.c b/fs/fuse/kio/pcs/pcs_map.c
index 4596454..34fa88c 100644
--- a/fs/fuse/kio/pcs/pcs_map.c
+++ b/fs/fuse/kio/pcs/pcs_map.c
@@ -1946,7 +1946,7 @@ static int pcs_cslist_submit_read(struct pcs_int_request *ireq, struct pcs_cs_li
     WRITE_ONCE(csl->read_index, i);
     WRITE_ONCE(csl->select_stamp, jiffies);

- FUSE_KTRACE(ireq->cc->fc, "Selected read map " MAP_FMT " to CS" NODE_FMT "; is_seq=%d\n", MAP_ARGS(ireq->iochunk.map),
+ FUSE_KTRACE(ireq->cc->fc, "Selected read map " MAP_FMT " to CS" NODE_FMT "; is_seq=%d", MAP_ARGS(ireq->iochunk.map),
          NODE_ARGS(csl->cs[i].cslink.cs->id), is_seq);
     pcs_flow_bind_cs(ireq->iochunk.flow, csl->cs[i].cslink.cs);
     }


_______________________________________________
Devel mailing list
Devel at openvz.org<mailto:Devel at openvz.org>
https://lists.openvz.org/mailman/listinfo/devel





-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/devel/attachments/20181205/4aaa986d/attachment-0001.html>