System hangs when running two different LKMs to get battery statistics every second

Question

I am developing a Linux Kernel Module (LKM) which analyzes my laptop's battery parameters and writes to Kernel log file (/var/log/kern.log) for every given time interval. Using the data from log file, I will be plotting a real time graph.

You may think as why I am doing this stuff. This is a part of my academics is the answer to it. Also, I find this a pretty interesting one.

My entire analysis for this task went on simple, until recently. The steps I followed are:

1. The parameters can be fetched as given in the best answer here and some more parameters were found here. Using these two links, my task got an easy start.
2. The code to execute a function of LKM at a periodic time was fetched from here. This was modified to run for every second.

With these two LKMs, I misunderstood that my task was a piece of cake, as the only task left out was combining these two LKMs and writing the required parameter data to log file.

I clubbed the code of both the LKMs and made one single LKM, and plugged it into my kernel of Ubuntu 14.04 64 bit OS. The system froze instantly making me shocked. I was clueless at this point.

After this I reduced the code of battery analyzer LKM and timer LKM which are least required to be run for my desired output. I found the reduced code which runs comfortably.

Later I migrated the code to analyze battery parameters to a function and exported it (using EXPORT_SYMBOL(<function_name>), to be called from the timer method. I was damn sure that this would run for sure, as both the LKMs were running cool.

I would like to give you the real picture of what happened in my machine with following LKM codes and video.

bat_stat_analyzer.c - LKM to fetch battery parameters and write to kernel log file.

#include <linux/module.h>
#include <linux/version.h>
#include <linux/kernel.h>
#include <linux/types.h>
#include <linux/kdev_t.h>
#include <linux/fs.h>
#include <linux/power_supply.h>

static int result = 0;
static struct power_supply *psy;
static void bat_stat(void);

EXPORT_SYMBOL(bat_stat);

static void bat_stat(void) {
  union power_supply_propval value;
  int charge_status, voltage_now, current_now, capacity;

  psy = power_supply_get_by_name("BAT1");
  result = psy->get_property(psy,POWER_SUPPLY_PROP_STATUS, &value);
  charge_status = (!result) ? value.intval : -1;
  result = psy->get_property(psy,POWER_SUPPLY_PROP_VOLTAGE_NOW, &value);
  voltage_now = (!result) ? value.intval : -1;
  result = psy->get_property(psy,POWER_SUPPLY_PROP_CURRENT_NOW, &value);
  current_now = (!result) ? value.intval : -1;
  result = psy->get_property(psy,POWER_SUPPLY_PROP_CAPACITY, &value);
  capacity = (!result) ? value.intval : -1;

  printk(KERN_INFO "%s:%d,%d,%d,%d\n",
    __func__, charge_status, voltage_now, current_now, capacity);
}

static int __init bat_stat_init(void) /* Constructor */
{
  bat_stat();
  return 0;
}

static void __exit bat_stat_exit(void) /* Destructor */
{
  printk(KERN_INFO "Good bye\n");
}

module_init(bat_stat_init);
module_exit(bat_stat_exit);

MODULE_LICENSE("GPL");
MODULE_AUTHOR("Sriram Kumar <sriramhearing_at_gmail_dot_com>");
MODULE_DESCRIPTION("First Battery Analyzer");

bat_stat_repeater.c - LKM to call a function repeatedly

#include <linux/module.h>   /* Needed by all modules */
#include <linux/kernel.h>   /* Needed for KERN_INFO */
#include <linux/init.h>     /* Needed for the macros */
#include <linux/jiffies.h>
#include <linux/time.h>
#include <linux/hrtimer.h>

static unsigned long period_ms;
static unsigned long period_ns;
static ktime_t ktime_period_ns;
static struct hrtimer my_hrtimer;
extern int bat_stat(void);


//~ static void bat_stat_repeat(unsigned long data)
static enum hrtimer_restart bat_stat_repeat(struct hrtimer *timer)
{
  unsigned long tjnow;
  ktime_t kt_now;
  bat_stat();
  printk(KERN_INFO "Repeating...\n");

  tjnow = jiffies;
  kt_now = hrtimer_cb_get_time(&my_hrtimer);
  hrtimer_forward(&my_hrtimer, kt_now, ktime_period_ns);
  return HRTIMER_RESTART;
}

static int __init bat_stat_init(void)
{
  struct timespec tp_hr_res;
  period_ms = 1000;
  hrtimer_get_res(CLOCK_MONOTONIC, &tp_hr_res);

  hrtimer_init(&my_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
  my_hrtimer.function = &bat_stat_repeat;
  period_ns = period_ms*( (unsigned long)1E6L );
  ktime_period_ns = ktime_set(0,period_ns);
  hrtimer_start(&my_hrtimer, ktime_period_ns, HRTIMER_MODE_REL);

  return 0;
}

static void __exit bat_stat_exit(void)
{
  int ret_cancel = 0;
  while( hrtimer_callback_running(&my_hrtimer) ) {
    ret_cancel++;
  }
  if (ret_cancel != 0) {
    printk(KERN_INFO " testjiffy Waited for hrtimer callback to finish (%d)\n", ret_cancel);
  }
  if (hrtimer_active(&my_hrtimer) != 0) {
    ret_cancel = hrtimer_cancel(&my_hrtimer);
    printk(KERN_INFO " testjiffy active hrtimer cancelled: %d\n", ret_cancel);
  }
  if (hrtimer_is_queued(&my_hrtimer) != 0) {
    ret_cancel = hrtimer_cancel(&my_hrtimer);
    printk(KERN_INFO " testjiffy queued hrtimer cancelled: %d\n", ret_cancel);
  }
  printk(KERN_INFO "Exit testjiffy\n");
}

module_init(bat_stat_init);
module_exit(bat_stat_exit);

MODULE_LICENSE("GPL");

The Makefile I used looked like below as it was used to compile both of the LKMs.

obj-m := <file_name>.o
KDIR := /lib/modules/$(shell uname -r)/build
PWD := $(shell pwd)
default:
    $(MAKE) -C $(KDIR) SUBDIRS=$(PWD) modules

The output that I obtained using these two LKMs can be viewed on this youtube link (just 2 mins video). If you wish to see it as an image, it is attached below:

I would like to know:

• Is there a better and/or easier way to achieve what I am trying? or
• What's the trouble in my LKMs that make my CPU get choked?

Thanks in advance.

Answer 1

You've got a problem because of using hrtimers. This mechanism is designed for high precision, the callback is called in hardirq context (with IRQs disabled) so your callback function has to be atomic. Yet, the functions you are calling from the callback are not atomic and may sleep (because of mutexes). Normal timers have similar problems so in order to fix this, you should use some other way or repeating task, like workqueues, for example.

Some other small problems in your code:

• your batt_stat function declarations in both modules are different
• you don't check the output of power_supply_get_by_name() in first module

Also, I really don't see any reason to split this to two kernel modules, you should use only one.