Lesson06: Implement kernel module for test kernel time functions and timers #87

OlegH-ua · 2018-11-13T08:53:24Z

No description provided.

Signed-off-by: Oleg.Khokhlov <[email protected]>

DevyatovAndrey

Please fix mentioned issues

DevyatovAndrey · 2018-11-13T14:12:28Z

06-TimeManagement/omod6_kern_time.c

+	int n;
+
+	n = sscanf(buf, "%d %s", &interval, LogMessage);
+	pr_info("omod6 store (%d, \'%s\')\n", interval, LogMessage);


@OlegH-ua
As far as I can see, the message passed to store method was "Hello from timer", but then it prints only "Hello":
"omod6 store (5000, 'Hello')"

IMO it's better to divide setting of timer period and message into two sys/proc files

You are right, this piece of code is really not perfect. And even non-safe.

DevyatovAndrey · 2018-11-13T14:14:42Z

06-TimeManagement/omod6_kern_time.c

+				interval * HZ / 1000;
+			//if (timer_pending(&MyTimer)) {
+			//	mod_timer_pending(&MyTimer, expired);
+			//} else {


What is the purpose of these lines commented out?

I had to do some experiments and kernel code inspection to clarify the behavior of timers and timer functions. A lot of lecture information about timers was unclear, obsolete and non-actual.

That commented code has been left to accentuate that used functions was tested and recognized as excessive. Everything works perfect without them. But it works with them, too.

DevyatovAndrey · 2018-11-13T14:17:52Z

06-TimeManagement/omod6_kern_time.c

+	sprintf(strchr(buf, 0), "%u module call\n", ModCallCount);
+	sprintf(strchr(buf, 0), "%u sec from last call\n", dT);
+	sprintf(strchr(buf, 0), "%u sec curr abs time from Epoch (year:%d)\n", TS, 1970+TS/(3600*24*365));
+	sprintf(strchr(buf, 0), "%u sec prev call abs time from Epoch\n", LastCallTimeSec);


1542096137 sec curr abs time from Epoch (year:2018)

IMO it's better to format absolute time stamp as human readable string aka YY.MM.DD hh:mm:ss

Good idea. I had been thinking about it when I was written the code but I decided that it's extraneous. Maybe you can recomend proper kernel function to convert epoch timestamp to struct time ?

I see you've managed to format the epoch time. Good job!

DevyatovAndrey

Could you please squash the two commits? The intermediate state after the first commit is actually not needed.

DevyatovAndrey · 2018-11-14T07:36:30Z

06-TimeManagement/omod6_kern_time.c

+	sprintf(strchr(buf, 0), "%u module call\n", ModCallCount);
+	sprintf(strchr(buf, 0), "%u sec from last call\n", dT);
+	sprintf(strchr(buf, 0), "%u sec curr abs time from Epoch (year:%d)\n", TS, 1970+TS/(3600*24*365));
+	sprintf(strchr(buf, 0), "%u sec prev call abs time from Epoch\n", LastCallTimeSec);


I see you've managed to format the epoch time. Good job!

DevyatovAndrey · 2018-11-14T07:40:00Z

06-TimeManagement/omod6_kern_time.c

+		if (cp) {
+			while (*cp == ' ')
+				cp++;
+			for (n = 0; n < sizeof(LogMessage) - 1; n++) {


What would be if user enter the wrong/garbage string? Have you tested this case?

No, I haven't tested this case. But as for this code, it's made a priory safe by design. It copies only limited number of chars into message string and is protected against strange ascii codes, e.g. CR/LF, ESC, etc. Which are unwanted in system log. All other ascii chars are OK. This method of string copying was selected after I realized that the 'store' function gets the string from user space which is ended this '\n'. It was a piece of bad news for me.

OlegH-ua · 2018-11-14T15:01:42Z

Last two commits was squashed using git rebase -i HEAD~1 and forcepush. please check it up.

DevyatovAndrey

@OlegH-ua
Please get rid off all unnesessary comments and dead code.
Thank you!

DevyatovAndrey · 2018-11-15T08:24:25Z

06-TimeManagement/omod6_kern_time.c

+		//--- commented code is not needed - timer works without it ---
+		//if (timer_pending(&MyTimer)) {
+		//	mod_timer_pending(&MyTimer, expired);
+		//} else {


It's not clear still - what is the purpose of commented chunks of code, if that code is not needed?
Any PR aims to add some useful code/comments into the project. But not dead code commented out.

(almost) everything to make you happy ;-)

Signed-off-by: Oleg.Khokhlov <[email protected]>

OlegH-ua added 2 commits November 13, 2018 10:49

Lesson06: Implemented module (sysfs) for test kernel time and timers

c19e1f0

Signed-off-by: Oleg.Khokhlov <[email protected]>

Lesson06: add checkpatch and test run log for omod6_kern_time

bfd9066

Signed-off-by: Oleg.Khokhlov <[email protected]>

OlegH-ua requested a review from an1kh November 13, 2018 08:54

OlegH-ua added the ready label Nov 13, 2018

DevyatovAndrey requested changes Nov 13, 2018

View reviewed changes

DevyatovAndrey added changes requested The PR review revealed issues which should be fixed (set by reviewer). and removed ready labels Nov 13, 2018

OlegH-ua added ready and removed changes requested The PR review revealed issues which should be fixed (set by reviewer). labels Nov 13, 2018

OlegH-ua requested review from DevyatovAndrey and removed request for an1kh November 13, 2018 19:56

DevyatovAndrey requested changes Nov 14, 2018

View reviewed changes

DevyatovAndrey added changes requested The PR review revealed issues which should be fixed (set by reviewer). and removed ready labels Nov 14, 2018

OlegH-ua force-pushed the lesson06 branch from df80387 to 18ea689 Compare November 14, 2018 14:58

OlegH-ua added ready and removed changes requested The PR review revealed issues which should be fixed (set by reviewer). labels Nov 14, 2018

OlegH-ua requested a review from DevyatovAndrey November 14, 2018 15:07

DevyatovAndrey requested changes Nov 15, 2018

View reviewed changes

DevyatovAndrey added changes requested The PR review revealed issues which should be fixed (set by reviewer). and removed ready labels Nov 15, 2018

Lesson06: Update module source (show real datetime, new string parsing)

a547caf

Signed-off-by: Oleg.Khokhlov <[email protected]>

OlegH-ua force-pushed the lesson06 branch from 18ea689 to a547caf Compare November 15, 2018 10:20

OlegH-ua added ready and removed changes requested The PR review revealed issues which should be fixed (set by reviewer). labels Nov 15, 2018

OlegH-ua requested a review from DevyatovAndrey November 15, 2018 10:22

marynasergiyenko removed the ready label Oct 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lesson06: Implement kernel module for test kernel time functions and timers #87

Lesson06: Implement kernel module for test kernel time functions and timers #87

OlegH-ua commented Nov 13, 2018

DevyatovAndrey left a comment

DevyatovAndrey Nov 13, 2018

OlegH-ua Nov 13, 2018

DevyatovAndrey Nov 13, 2018

OlegH-ua Nov 13, 2018

DevyatovAndrey Nov 13, 2018

OlegH-ua Nov 13, 2018

DevyatovAndrey Nov 14, 2018

DevyatovAndrey left a comment

DevyatovAndrey Nov 14, 2018

DevyatovAndrey Nov 14, 2018

OlegH-ua Nov 14, 2018 •

edited

Loading

OlegH-ua commented Nov 14, 2018

DevyatovAndrey left a comment

DevyatovAndrey Nov 15, 2018

OlegH-ua Nov 15, 2018

Lesson06: Implement kernel module for test kernel time functions and timers #87

Are you sure you want to change the base?

Lesson06: Implement kernel module for test kernel time functions and timers #87

Conversation

OlegH-ua commented Nov 13, 2018

DevyatovAndrey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DevyatovAndrey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OlegH-ua Nov 14, 2018 • edited Loading

Choose a reason for hiding this comment

OlegH-ua commented Nov 14, 2018

DevyatovAndrey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OlegH-ua Nov 14, 2018 •

edited

Loading