From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Rostedt Subject: Re: [PATCH - resend] addition of loglevel for printf in Xen HV Date: Thu, 26 Oct 2006 09:30:27 -0400 Message-ID: <4540B873.4010906@redhat.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: chrisw@sous-sol.org, xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org Keir Fraser wrote: > > > On 20/10/06 20:23, "Steven Rostedt" wrote: > >> This patch really does need to go in. Whether in its current form, or >> modified greatly. The ability to give the developer a level of printing >> is a great asset. That is why Linux has this ability. > > I agree. Here are some comments: Keir, Thanks for the comments, they are very useful. > > * We don't need 8 log levels. Noone knows the difference between > EMERG/CRIT/ALERT, or at least I really doubt that they can be used > consistently in a large code base. I suggest: > XENLOG_ERR -- Bad errors, likely fatal (to a guest or the host) > XENLOG_WARN -- Weird stuff happening, recoverable (maybe) > XENLOG_INFO -- Interesting info, not too noisy > XENLOG_DEBUG -- Noisy as you like Perfectly agree. Four is fine, especially if we implement a option for guest. > > * I think your ratelimit stuff could be usefully integrated. Instead of > having a single threshold, have two: one above which everything is printed, > one below which nothing is printed. In between is rate limited. I'm not quite sure I understand this fully. So what you are saying is instead of having the print_log_level have a printk_upper_log_level and a printk_lower_log_level where, as you say, everything below the printk_lower_log_level is printed, and the printk_upper_log_level where everyting above is not printed, and thus everything in between is rate limited? (remember 0 is ERR and 3 is DEBUG) > > * I think the guest/not-guest printing is orthogonal to the log-level > scale. Perhaps we should have another <> specifier (?) that can be used > alongside a log level specifier. This would give another two threshold > values (two for guest, two for non-guest) which would maybe be overkill but > would be reasonable if we gave a dom0 tool to control it. Actually this > could be extended to other subsystems (shadow code for example). Then we > would have a control per subsystem falling back to default thresholds if not > specified. That probably really is overkill! > > * Likely defaults: > Non-guest prints ERR/WARN always, no INFO/DEBUG > Guest prints ERR/WARN rate-limited, no INFO/DEBUG > (Like Linux, we'd probably have slacker default controls until initial > bootstrap has completed, so you get useful info out). > > What do you think? > I have to admit I like the ideas you have thrown to me. So let me put this is my own words/code so that you can see what my view of this is. Have 4 thresholds. printk_dom0_upper_threshold printk_dom0_lower_threshold printk_guest_upper_threshold printk_guest_lower_threshold Have 4 log levels: XENLOG_ERR XENLOG_WARN XENLOG_INFO XENLOG_DEBUG Add a XENLOG_GUEST (defined as "") And for less typing add XENLOG_G_ERR XENLOG_G_WARN XENLOG_G_INFO XENLOG_G_DEBUG which would just append the XENLOG_GUEST in front of the associated warning. example; printk(XENLOG_G_ERR "fatal error in guest\n"); printk(XENLOG_INFO "dom0 is running happily\n"); Then in printk itself, we would have upper_thresh = printk_dom0_upper_threshold; lower_thresh = printk_dom0_lower_threshold; level = some_default; if (strncmp("",fmt,3)==0) { upper_thresh = printk_guest_upper_threshold; lower_thresh = printk_guest_lower_threshold; level = some_guest_default; fmt += 3; } if (strncmp("<[0-3]>",fmt,3)==0) { /* using regexpr and not C to make this readable */ level =~ s/<(0-3)>/$1/; /* perl format :-) */ } if (level > upper_thresh) return; /* do nothing */ if (level >= lower_thresh) if (rate_limit()) return; print as normal. So anything in between the upper and lower thresholds inclusive would be rate limited. Also I'm not sure of the best way dom0 can communicate threshold changes to the hypervisor. As well as if there should be a config to set the defaults. So what are your thoughts? -- Steve