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

click: Compatibility changes for kernel lock debugging #17

Merged
merged 1 commit into from
May 12, 2011

Conversation

kph
Copy link
Contributor

@kph kph commented May 11, 2011

Lock correctness debugging requires that various kernel data structures
be statically allocated, instead of being allocated in dynamic
memory. One such structre is the VFS file_system_type.

Previously, proclikefs imbedded this structure inside its
proclikefs_file_system structure, which was allocated dynamically.
Instead, each client of proclikefs (actually just clickfs) now
statically allocates a file_system_type structure, and registers
this with proclikefs.

The include file semaphore.h contains a C99-style structure
initializer which is not legal in C++. This file only gets
included when spin lock debugging is enabled. Since Click does
not use sema_init(), the simple fix is to skip its compilation
by surrounding it in #ifndef __cplusplus.

The include file semaphore.h is also mangled by the fixincludes.pl
changes which are intended to only modify ktime.h. Limit the scope
of ktime-related changes to only ktime.h

@kohler
Copy link
Owner

kohler commented May 11, 2011

Kevin!!!! Awesome. Some comments:

(1) Can this be split into two commits?
(2) The fixincludes.pl is I think ready to apply immediately.
(3) The proclikefs one is not. I think you're missing the point of proclikefs. The purpose of proclikefs is to allow the Click module to be unloaded and possibly reloaded later, even if the /click file system continues to exist because some userlevel program has a /click/WHATEVER file open. For that reason, most memory having to do with the ifle system MUST be owned by proclikefs, NOT by its user. So statically-allocated file_system_type in the click module is a no go I think: it gives the file system the wrong owner. (Do I misunderstand?) I think a cleaner, better change would simply be to allocate a small static array of struct file_system_types in proclikefs.c. And refuse a proclikefs_register_filesystem request if that array was all allocated. Doe sthis make sense?

initializer which is not legal in C++. This file only gets
included when spin lock debugging is enabled. Since Click does
not use sema_init(), the simple fix is to skip its compilation
by surrounding it in #ifndef __cplusplus.

The include file semaphore.h is also mangled by the fixincludes.pl
changes which are intended to only modify ktime.h. Limit the scope
of ktime-related changes to only ktime.h.
@kph
Copy link
Contributor Author

kph commented May 12, 2011

OK, I fixed the fixincludes.pl and made that the only thing in my pull request. I'll work on a corrected proclikefs patch tomorrow.

Thanks,
Kevin

kohler pushed a commit that referenced this pull request May 12, 2011
click: Compatibility changes for kernel lock debugging
@kohler kohler merged commit dc346cf into kohler:master May 12, 2011
@kohler
Copy link
Owner

kohler commented May 12, 2011

Excellent. I pulled the fixincludes part.

On 5/11/11 9:09 PM, kph wrote:

OK, I fixed the fixincludes.pl and made that the only thing in my pull request. I'll work on a corrected proclikefs patch tomorrow.

Thanks,
Kevin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants