Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Home work #9

Open
wants to merge 12 commits into
base: Ruslan.Sukhov
Choose a base branch
from
Open

Conversation

ruslansukhov
Copy link

No description provided.

@AleksandrBulyshchenko AleksandrBulyshchenko changed the base branch from master to Ruslan.Sukhov October 31, 2017 11:50
Copy link
Collaborator

@AleksandrBulyshchenko AleksandrBulyshchenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ruslansukhov,

  • Please format the PR according to wiki:
    • conform to code standards;
    • add better messages;
  • take into account my comments to Home work #7 (as these implementations are identical except hrtimer addon);
  • present more flexible example of memory allocation,
    you can do that while implementing kmem_cache-based solution;
  • fix issues in time-management solutions;
  • regarding hrtimer solution:
    • adapted Tsiliuric's example is a good start, but at least requires cleaning up;
    • I hope to see polished solution if not created from the scratch.


int __init x_init(void) {
int res;
buf_msg = kmalloc(LEN_MSG+1,GFP_KERNEL | __GFP_ZERO);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very straightforward replacement of static buffer.
But dynamic allocation allow more flexible memory management.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was my first attempt to make pull request. Not very successful attempt. Please see pull requests 39 and 40. They refer to dynamic memory allocation and time management respectively. P.S. How can I delete this pull request (№9)?

@@ -0,0 +1,22 @@
#
# Memory management example
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to update comments

static ssize_t xxx_show( struct class *class, struct class_attribute *attr, char *buf ) {
struct timeval tv_cur;
strcpy( buf, buf_msg );
do_gettimeofday(&tv_cur);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, absolute time API (do_gettimeofday) isn't the best choice for time interval measurement in uptime (but may be used).


static ssize_t xxx_show( struct class *class, struct class_attribute *attr, char *buf ) {
strcpy( buf, buf_msg );
printk( "previous absolute read time: %ld s %ld mks\n", tv->tv_sec, tv->tv_usec );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Time value should be returned, not just logged.
Also if you construct such a long human readable strings (you should remember, that sysfs is not intended for such usage), it make sense to provide time in human-readable form as well instead of raw timestamp.

return strlen( buf );
}

static ssize_t xxx_store( struct class *class, struct class_attribute *attr, const char *buf, size_t count ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant interface for this exercise.

// printk( KERN_INFO "%s\n", buf );
xxx_store(x_class, &class_attr_xxx,buf,strlen(buf)+1);
if (fib_n < 93) fib_n++;
else fib_n = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good intention to handle overflow at least somehow,
But I believe, the task assumed a bit more than 93 seconds (at least not via magic number).

ruvi added 3 commits January 10, 2018 22:16
# Conflicts:
#	lesson-04-memory-management/mm/.gitignore
#	lesson-04-memory-management/mm/Makefile
# Conflicts:
#	lesson-05-time-management/xxx_1.c
#	lesson-05-time-management/xxx_2.c
#	lesson-05-time-management/xxx_hrtimer.c
#	lesson-05-time-management/xxx_timer.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants