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

Lesson 4: Add dynamic memory allocation to ”xxx” driver #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion lesson-04-memory-management/mm/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,16 @@ obj-m += xxx.o

else

KERNELDIR := $(BUILD_KERNEL)
ifeq ($(KERNELDIR),)
ifeq ($(BBB_KERNEL),)
$(error Path to kernel tree - KERNELDIR or BBB_KERNEL variable is not defined!)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indentation with tabs aren't allowed outside of rules.

I should fix that in mpu6050 example.
Sometimes I hate make syntax.

endif
endif

KERNELDIR ?= $(BBB_KERNEL)

export ARCH ?= arm
export CROSS_COMPILE ?= arm-linux-gnueabihf-
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why have you made this exercise cross-compiled?
Don't you like qemu that much?


.PHONY: all clean
all:
Expand Down
144 changes: 136 additions & 8 deletions lesson-04-memory-management/mm/xxx.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,136 @@
#include <linux/pci.h>
#include <linux/version.h>
#include <linux/init.h>
#include <linux/slab.h>

#define LEN_MSG 160
static char buf_msg[LEN_MSG + 1] = "Hello from module!\n";
#define MEM_TYPE_KMALLOC 0
#define MEM_TYPE_KCACHE 1
#define MEM_TYPE_KVMALLOC 2

#define BUFFER_SIZE 160

static size_t buf_size = 0;
static char *pbuf_msg = 0;
static int mem_type = MEM_TYPE_KMALLOC;
struct KCACHE_STRUCT {
char buf[BUFFER_SIZE];
};

static struct kmem_cache* pmem_cache = NULL;

const char* get_mem_type(int type)
{
switch (type) {
case MEM_TYPE_KMALLOC:
return "MEM_TYPE_KMALLOC";
case MEM_TYPE_KCACHE:
return "MEM_TYPE_KCACHE";
case MEM_TYPE_KVMALLOC:
return "MEM_TYPE_KVMALLOC";
}
return "UNKNOWN";
}

static void* mem_alloc(int type, size_t size)
{
void* buf = NULL;
switch (type) {
case MEM_TYPE_KMALLOC:
buf = kmalloc(size, GFP_KERNEL);
break;
case MEM_TYPE_KCACHE:
buf = kmem_cache_alloc( pmem_cache, GFP_KERNEL );
break;
case MEM_TYPE_KVMALLOC:
buf = vmalloc(size);
break;
}

if (buf) buf_size = size;

pr_info("[%s:%s] {%s} %zu bytes --> %p\n", THIS_MODULE->name, __FUNCTION__, get_mem_type(type), size, buf);
return buf;
}

static void mem_free(int type, void* buf)
{
if (!buf) {
pr_warning("[%s:%s] Wrong parameter %p\n", THIS_MODULE->name, __FUNCTION__, buf);
return;
}
switch (type) {
case MEM_TYPE_KMALLOC:
kfree(buf);
break;
case MEM_TYPE_KCACHE:
kmem_cache_free( pmem_cache, buf );
buf = kmem_cache_alloc( pmem_cache, GFP_KERNEL );
break;
case MEM_TYPE_KVMALLOC:
vfree(buf);
break;
}
pr_info("[%s:%s] {%s} bytes --> %p\n", THIS_MODULE->name, __FUNCTION__, get_mem_type(type), buf);
}

void init_kcache(void)
{
if (mem_type != MEM_TYPE_KCACHE) return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, it's more logically to use opposite code structure:

if (mem_type == MEM_TYPE_KCACHE) {
        /* init cache */
}

And in this particular case - I'd move mem_type verification to the x_init()
and made call of init_kcache() conditional.
Otherwise init_kcache() should be renamed to reflect, it inits any kind of buffer (even if for other types nothing is done).

/*
* 140 * Please use this macro to create slab caches. Simply specify the
* 141 * name of the structure and maybe some flags that are listed above.
* 142 *
* 143 * The alignment of the struct determines object alignment. If you
* 144 * f.e. add ____cacheline_aligned_in_smp to the struct declaration
* 145 * then the objects will be properly aligned in SMP configurations.
* 146
* #define KMEM_CACHE(__struct, __flags) kmem_cache_create(#__struct,\
* 148 sizeof(struct __struct), __alignof__(struct __struct),\
* 149 (__flags), NULL)
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please avoid such copy-paste comments in sources.
If you need some manual to be left for future, rewrite it accordingly (or write a link to original definition).

pmem_cache = KMEM_CACHE(KCACHE_STRUCT, 0);
pr_info("[%s:%s] pmem_cache: %p\n", THIS_MODULE->name, __FUNCTION__, pmem_cache);
}

void deinit_kcache(void)
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again - function name doesn't correspond to its function.

if (mem_type != MEM_TYPE_KCACHE) {
mem_free(mem_type, pbuf_msg);
return;
}
kmem_cache_destroy( pmem_cache );
}


static ssize_t xxx_show(struct class *class, struct class_attribute *attr,
char *buf)
{
strcpy(buf, buf_msg);
pr_info("read %ld\n", (long)strlen(buf));
return strlen(buf);
if (!pbuf_msg) {
printk( "[%s:%s] nothing to read\n", THIS_MODULE->name, __FUNCTION__);
return 0;
}
strcpy( buf, pbuf_msg );
pr_info( "[%s:%s] read %ld\n", THIS_MODULE->name, __FUNCTION__, (long)strlen( buf ) );
return strlen( buf );
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's better to avoid calling strlen() twice.

}

static ssize_t xxx_store(struct class *class, struct class_attribute *attr,
const char *buf, size_t count)
{
pr_info("write %ld\n", (long)count);
strncpy(buf_msg, buf, count);
buf_msg[count] = '\0';
if (!pbuf_msg) {
pbuf_msg = mem_alloc(mem_type, BUFFER_SIZE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The idea was to allocate required amount of memory dynamically.
Not to do that once (even if on first write), but be able to adjust buffer size at least for bigger count.

if (!pbuf_msg) {
pr_err( "[%s:%s] Wrong alloc buffer! \n", THIS_MODULE->name, __FUNCTION__);
return 0;
}
}
if (count >= BUFFER_SIZE) {
pr_err( "[%s:%s] Wrong buffer size %lu\n", THIS_MODULE->name, __FUNCTION__, (long)count );
return 0;
}
pr_info( "[%s:%s] write %ld\n", THIS_MODULE->name, __FUNCTION__, (long)count );
strncpy( pbuf_msg, buf, count );
pbuf_msg[ count ] = '\0';
return count;
}

Expand All @@ -35,20 +146,37 @@ int __init x_init(void)
{
int res;

pr_info("mem_type is %d\n", mem_type);
x_class = class_create(THIS_MODULE, "x-class");
if (IS_ERR(x_class))
pr_info("bad class create\n");
res = class_create_file(x_class, &class_attr_xxx);
Copy link
Collaborator

Choose a reason for hiding this comment

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

xxx_store may already be called at this point,
while pmem_cache hasn't yet been created.

pr_info("'xxx' module initialized\n");
init_kcache();
return res;
}

void x_cleanup(void)
{
class_remove_file(x_class, &class_attr_xxx);
class_destroy(x_class);
deinit_kcache();
}

/**
* 137 * module_param_named - typesafe helper for a renamed module/cmdline parameter
* 138 * @name: a valid C identifier which is the parameter name.
* 139 * @value: the actual lvalue to alter.
* 140 * @type: the type of the parameter
* 141 * @perm: visibility in sysfs.
* 142 *
* 143 * Usually it's a good idea to have variable names and user-exposed names the
* 144 * same, but that's harder if the variable must be non-static or is inside a
* 145 * structure. This allows exposure under a different name.
* 146 */
module_param( mem_type, int, 0444 );
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it intended to be read-only?

Copy link
Author

Choose a reason for hiding this comment

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

I made access right to read-only, because of didn't see any reasons to made other rights

MODULE_PARM_DESC(mem_type, "Type of memory allocation: 0 - kmalloc; 1 - cache; 2 - vmalloc");

module_init(x_init);
module_exit(x_cleanup);

Expand Down