| Message ID | 20200715031120.1002016-3-vitor@massaru.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kunit: add support to use modules | expand |
On Tue, Jul 14, 2020 at 8:11 PM Vitor Massaru Iha <vitor@massaru.org> wrote: Probably want to add more of a description here as what you are doing is not entirely trivial to someone not familiar with mm contexts. > > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org> > --- > include/kunit/test.h | 1 + > lib/kunit/try-catch.c | 15 ++++++++++++++- > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/include/kunit/test.h b/include/kunit/test.h > index 59f3144f009a..49c38bdcb93e 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -222,6 +222,7 @@ struct kunit { > * protect it with some type of lock. > */ > struct list_head resources; /* Protected by lock. */ > + struct mm_struct *mm; Part of me thinks we should put a better name here, part of me thinks it is fine because it matches the convention. Either way, this DEFINITELY deserves a comment explaining what it is, why it exists, and how it should/shouldn't be used. > }; > > void kunit_init_test(struct kunit *test, const char *name, char *log); > diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c > index 0dd434e40487..f677c2f2a51a 100644 > --- a/lib/kunit/try-catch.c > +++ b/lib/kunit/try-catch.c > @@ -11,7 +11,8 @@ > #include <linux/completion.h> > #include <linux/kernel.h> > #include <linux/kthread.h> > - > +#include <linux/sched/mm.h> > +#include <linux/sched/task.h> > #include "try-catch-impl.h" > > void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch) > @@ -24,8 +25,17 @@ EXPORT_SYMBOL_GPL(kunit_try_catch_throw); > static int kunit_generic_run_threadfn_adapter(void *data) > { > struct kunit_try_catch *try_catch = data; > + struct kunit *test = try_catch->test; > + > + if (test->mm != NULL) > + kthread_use_mm(try_catch->test->mm); > > try_catch->try(try_catch->context); > + if (try_catch->test->mm) { Here and below: You already have a pointer to test. You should use it. > + if (test->mm != NULL) > + kthread_unuse_mm(try_catch->test->mm); > + try_catch->test->mm = NULL; > + } > > complete_and_exit(try_catch->try_completion, 0); > } > @@ -65,6 +75,9 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context) > try_catch->context = context; > try_catch->try_completion = &try_completion; > try_catch->try_result = 0; > + > + test->mm = get_task_mm(current); > + > task_struct = kthread_run(kunit_generic_run_threadfn_adapter, > try_catch, > "kunit_try_catch_thread"); > -- > 2.26.2 >
On Wed, 2020-07-15 at 17:37 -0700, Brendan Higgins wrote: > On Tue, Jul 14, 2020 at 8:11 PM Vitor Massaru Iha <vitor@massaru.org> > wrote: > > Probably want to add more of a description here as what you are doing > is not entirely trivial to someone not familiar with mm contexts. > > > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org> > > --- > > include/kunit/test.h | 1 + > > lib/kunit/try-catch.c | 15 ++++++++++++++- > > 2 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/include/kunit/test.h b/include/kunit/test.h > > index 59f3144f009a..49c38bdcb93e 100644 > > --- a/include/kunit/test.h > > +++ b/include/kunit/test.h > > @@ -222,6 +222,7 @@ struct kunit { > > * protect it with some type of lock. > > */ > > struct list_head resources; /* Protected by lock. */ > > + struct mm_struct *mm; > > Part of me thinks we should put a better name here, part of me thinks > it is fine because it matches the convention. > > Either way, this DEFINITELY deserves a comment explaining what it is, > why it exists, and how it should/shouldn't be used. > > > }; > > > > void kunit_init_test(struct kunit *test, const char *name, char > > *log); > > diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c > > index 0dd434e40487..f677c2f2a51a 100644 > > --- a/lib/kunit/try-catch.c > > +++ b/lib/kunit/try-catch.c > > @@ -11,7 +11,8 @@ > > #include <linux/completion.h> > > #include <linux/kernel.h> > > #include <linux/kthread.h> > > - > > +#include <linux/sched/mm.h> > > +#include <linux/sched/task.h> > > #include "try-catch-impl.h" > > > > void __noreturn kunit_try_catch_throw(struct kunit_try_catch > > *try_catch) > > @@ -24,8 +25,17 @@ EXPORT_SYMBOL_GPL(kunit_try_catch_throw); > > static int kunit_generic_run_threadfn_adapter(void *data) > > { > > struct kunit_try_catch *try_catch = data; > > + struct kunit *test = try_catch->test; > > + > > + if (test->mm != NULL) > > + kthread_use_mm(try_catch->test->mm); > > > > try_catch->try(try_catch->context); > > + if (try_catch->test->mm) { > > Here and below: You already have a pointer to test. You should use > it. > > > + if (test->mm != NULL) > > + kthread_unuse_mm(try_catch->test->mm); > > + try_catch->test->mm = NULL; > > + } > > > > complete_and_exit(try_catch->try_completion, 0); > > } > > @@ -65,6 +75,9 @@ void kunit_try_catch_run(struct kunit_try_catch > > *try_catch, void *context) > > try_catch->context = context; > > try_catch->try_completion = &try_completion; > > try_catch->try_result = 0; > > + > > + test->mm = get_task_mm(current); > > + > > task_struct = > > kthread_run(kunit_generic_run_threadfn_adapter, > > try_catch, > > "kunit_try_catch_thread"); > > -- > > 2.26.2 > >
On Wed, 2020-07-15 at 17:37 -0700, Brendan Higgins wrote: > On Tue, Jul 14, 2020 at 8:11 PM Vitor Massaru Iha <vitor@massaru.org> > wrote: > > Probably want to add more of a description here as what you are doing > is not entirely trivial to someone not familiar with mm contexts. > > > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org> > > --- > > include/kunit/test.h | 1 + > > lib/kunit/try-catch.c | 15 ++++++++++++++- > > 2 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/include/kunit/test.h b/include/kunit/test.h > > index 59f3144f009a..49c38bdcb93e 100644 > > --- a/include/kunit/test.h > > +++ b/include/kunit/test.h > > @@ -222,6 +222,7 @@ struct kunit { > > * protect it with some type of lock. > > */ > > struct list_head resources; /* Protected by lock. */ > > + struct mm_struct *mm; > > Part of me thinks we should put a better name here, part of me thinks > it is fine because it matches the convention. > > Either way, this DEFINITELY deserves a comment explaining what it is, > why it exists, and how it should/shouldn't be used. > > > }; > > > > void kunit_init_test(struct kunit *test, const char *name, char > > *log); > > diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c > > index 0dd434e40487..f677c2f2a51a 100644 > > --- a/lib/kunit/try-catch.c > > +++ b/lib/kunit/try-catch.c > > @@ -11,7 +11,8 @@ > > #include <linux/completion.h> > > #include <linux/kernel.h> > > #include <linux/kthread.h> > > - > > +#include <linux/sched/mm.h> > > +#include <linux/sched/task.h> > > #include "try-catch-impl.h" > > > > void __noreturn kunit_try_catch_throw(struct kunit_try_catch > > *try_catch) > > @@ -24,8 +25,17 @@ EXPORT_SYMBOL_GPL(kunit_try_catch_throw); > > static int kunit_generic_run_threadfn_adapter(void *data) > > { > > struct kunit_try_catch *try_catch = data; > > + struct kunit *test = try_catch->test; > > + > > + if (test->mm != NULL) > > + kthread_use_mm(try_catch->test->mm); > > > > try_catch->try(try_catch->context); > > + if (try_catch->test->mm) { > > Here and below: You already have a pointer to test. You should use > it. Ops, thanks! > > > + if (test->mm != NULL) > > + kthread_unuse_mm(try_catch->test->mm); > > + try_catch->test->mm = NULL; > > + } > > > > complete_and_exit(try_catch->try_completion, 0); > > } > > @@ -65,6 +75,9 @@ void kunit_try_catch_run(struct kunit_try_catch > > *try_catch, void *context) > > try_catch->context = context; > > try_catch->try_completion = &try_completion; > > try_catch->try_result = 0; > > + > > + test->mm = get_task_mm(current); > > + > > task_struct = > > kthread_run(kunit_generic_run_threadfn_adapter, > > try_catch, > > "kunit_try_catch_thread"); > > -- > > 2.26.2 > >
diff --git a/include/kunit/test.h b/include/kunit/test.h index 59f3144f009a..49c38bdcb93e 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -222,6 +222,7 @@ struct kunit { * protect it with some type of lock. */ struct list_head resources; /* Protected by lock. */ + struct mm_struct *mm; }; void kunit_init_test(struct kunit *test, const char *name, char *log); diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c index 0dd434e40487..f677c2f2a51a 100644 --- a/lib/kunit/try-catch.c +++ b/lib/kunit/try-catch.c @@ -11,7 +11,8 @@ #include <linux/completion.h> #include <linux/kernel.h> #include <linux/kthread.h> - +#include <linux/sched/mm.h> +#include <linux/sched/task.h> #include "try-catch-impl.h" void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch) @@ -24,8 +25,17 @@ EXPORT_SYMBOL_GPL(kunit_try_catch_throw); static int kunit_generic_run_threadfn_adapter(void *data) { struct kunit_try_catch *try_catch = data; + struct kunit *test = try_catch->test; + + if (test->mm != NULL) + kthread_use_mm(try_catch->test->mm); try_catch->try(try_catch->context); + if (try_catch->test->mm) { + if (test->mm != NULL) + kthread_unuse_mm(try_catch->test->mm); + try_catch->test->mm = NULL; + } complete_and_exit(try_catch->try_completion, 0); } @@ -65,6 +75,9 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context) try_catch->context = context; try_catch->try_completion = &try_completion; try_catch->try_result = 0; + + test->mm = get_task_mm(current); + task_struct = kthread_run(kunit_generic_run_threadfn_adapter, try_catch, "kunit_try_catch_thread");
Signed-off-by: Vitor Massaru Iha <vitor@massaru.org> --- include/kunit/test.h | 1 + lib/kunit/try-catch.c | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-)