Re: [PATCH 4/4] printk: Add config option for disabling printk offloading

[Jan Kara <jack@suse.cz>]

On Fri 18-09-15 15:15:36, Andrew Morton wrote:
> On Wed, 19 Aug 2015 17:38:31 +0200 Jan Kara <jack@suse.com> wrote:
> 
> > From: Jan Kara <jack@suse.cz>
> > 
> > Necessity for offloading of printing was observed only for large
> > systems. So add a config option (disabled by default)
> 
> The Kconfig has "default y"?

Changed to 'n'. Thanks for spotting this.

> > which removes most
> > of the overhead added by this functionality.
> > 
> > ...
> >
> > +#ifdef CONFIG_PRINTK_OFFLOAD
> >  /*
> >   * Returns true iff there is other cpu waiting to take over printing. This
> >   * function also takes are of setting PRINTK_HANDOVER_B if we want to hand over
> > @@ -2278,6 +2283,14 @@ static bool cpu_stop_printing(int printed_chars)
> >  
> >  	return false;
> >  }
> > +#else
> > +
> > +static bool cpu_stop_printing(int printed_chars, bool *woken)
> > +{
> > +	return false;
> > +}
> > +
> > +#endif
> >  
> >  /**
> >   * console_unlock - unlock the console system
> > @@ -2316,7 +2329,9 @@ void console_unlock(void)
> >  	/* flush buffered message fragment immediately to console */
> >  	console_cont_flush(text, sizeof(text));
> >  again:
> > +#ifdef CONFIG_PRINTK_OFFLOAD
> >  	spin_lock(&print_lock);
> > +#endif
> 
> You could nuke a couple of ugly ifdefs by adding
> spin_[un]lock_printk_lock() wrappers into that ifdef/else/endif block
> which holds cpu_stop_printing().

Yup, done.

> >  	for (;;) {
> >  		struct printk_log *msg;
> >  		size_t ext_len = 0;
> > @@ -2399,12 +2414,14 @@ skip:
> >  
> >  	console_locked = 0;
> >  	up_console_sem();
> > +#ifdef CONFIG_PRINTK_OFFLOAD
> >  	/*
> >  	 * Release print_lock after console_sem so that printing_task()
> >  	 * succeeds in getting console_sem (unless someone else takes it and
> >  	 * then he'll be responsible for printing).
> >           */
> >  	spin_unlock(&print_lock);
> > +#endif
> >  
> >  	/*
> >  	 * Subtlety: We have interrupts disabled iff hand_over == false (to
> > @@ -2770,6 +2787,7 @@ int unregister_console(struct console *console)
> >  }
> >  EXPORT_SYMBOL(unregister_console);
> >  
> > +#ifdef CONFIG_PRINTK_OFFLOAD
> >  /* Kthread which takes over printing from a CPU which asks for help */
> >  static int printing_task(void *arg)
> >  {
> > @@ -2838,6 +2856,7 @@ static int offload_chars_set(const char *val, const struct kernel_param *kp)
> >  	mutex_unlock(&printk_kthread_mutex);
> >  	return 0;
> >  }
> > +#endif	/* CONFIG_PRINTK_OFFLOAD */
> >  
> >  static int __init printk_late_init(void)
> >  {
> > @@ -2850,9 +2869,11 @@ static int __init printk_late_init(void)
> >  	}
> >  	hotcpu_notifier(console_cpu_notify, 0);
> >  
> > +#ifdef CONFIG_PRINTK_OFFLOAD
> >  	mutex_lock(&printk_kthread_mutex);
> >  	printk_start_offload_kthreads();
> >  	mutex_unlock(&printk_kthread_mutex);
> > +#endif
> 
> Possibly ditto here.  Maybe move the mutex_lock() into
> printk_start_offload_kthreads() too?

OK, done.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR