| Message ID | fd8b05453a0c72f2810a53e49ca1dc9467d9dbf9.1560410182.git.jstancek@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | None | expand |
On Thu, Jun 13, 2019 at 3:25 PM Jan Stancek <jstancek@redhat.com> wrote: > There are no users outside of lib. > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > --- > include/tst_clone.h | 2 +- > lib/cloner.c | 8 ++++---- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/include/tst_clone.h b/include/tst_clone.h > index 786cee5d1209..fd52097e2072 100644 > --- a/include/tst_clone.h > +++ b/include/tst_clone.h > @@ -29,7 +29,7 @@ int ltp_clone(unsigned long flags, int (*fn)(void *arg), > void *arg, > size_t stack_size, void *stack); > int ltp_clone7(unsigned long flags, int (*fn)(void *arg), void *arg, > size_t stack_size, void *stack, ...); > -int ltp_clone_malloc(unsigned long clone_flags, int (*fn)(void *arg), > +int ltp_clone_alloc(unsigned long clone_flags, int (*fn)(void *arg), > void *arg, size_t stacksize); > int ltp_clone_quick(unsigned long clone_flags, int (*fn)(void *arg), > void *arg); > diff --git a/lib/cloner.c b/lib/cloner.c > index cf37184aa22a..8469745d004b 100644 > --- a/lib/cloner.c > +++ b/lib/cloner.c > @@ -135,11 +135,11 @@ void *ltp_alloc_stack(size_t size) > } > > /* > - * ltp_clone_malloc: also does the memory allocation for clone with a > + * ltp_clone_alloc: also does the memory allocation for clone with a > * caller-specified size. > */ > int > -ltp_clone_malloc(unsigned long clone_flags, int (*fn) (void *arg), void > *arg, > +ltp_clone_alloc(unsigned long clone_flags, int (*fn) (void *arg), void > *arg, > size_t stack_size) > { > void *stack; > @@ -162,7 +162,7 @@ ltp_clone_malloc(unsigned long clone_flags, int (*fn) > (void *arg), void *arg, > } > > /* > - * ltp_clone_quick: calls ltp_clone_malloc with predetermined stack size. > + * ltp_clone_quick: calls ltp_clone_alloc with predetermined stack size. > * Experience thus far suggests that one page is often insufficient, > * while 6*getpagesize() seems adequate. > */ > @@ -170,5 +170,5 @@ int ltp_clone_quick(unsigned long clone_flags, int > (*fn) (void *arg), void *arg) > { > size_t stack_size = getpagesize() * 6; > > - return ltp_clone_malloc(clone_flags, fn, arg, stack_size); > + return ltp_clone_alloc(clone_flags, fn, arg, stack_size); > } > There is another legacy problem maybe we need take care. Any thought about how releasing the stack memory[1] after calling ltp_clone_quick() in a test? [1] which allocated memory in ltp_clone_alloc().
Hi! > > @@ -170,5 +170,5 @@ int ltp_clone_quick(unsigned long clone_flags, int > > (*fn) (void *arg), void *arg) > > { > > size_t stack_size = getpagesize() * 6; > > > > - return ltp_clone_malloc(clone_flags, fn, arg, stack_size); > > + return ltp_clone_alloc(clone_flags, fn, arg, stack_size); > > } > > > > There is another legacy problem maybe we need take care. > > Any thought about how releasing the stack memory[1] after calling > ltp_clone_quick() in a test? > > [1] which allocated memory in ltp_clone_alloc(). Well, we can free the memory in ltp_clone_alloc() if the child runs in a separate memory space, but if CLONE_VM was passed in flags there is no way how to free the memory...
On Thu, Jun 13, 2019 at 9:58 PM Cyril Hrubis <chrubis@suse.cz> wrote: > Hi! > > > @@ -170,5 +170,5 @@ int ltp_clone_quick(unsigned long clone_flags, int > > > (*fn) (void *arg), void *arg) > > > { > > > size_t stack_size = getpagesize() * 6; > > > > > > - return ltp_clone_malloc(clone_flags, fn, arg, stack_size); > > > + return ltp_clone_alloc(clone_flags, fn, arg, stack_size); > > > } > > > > > > > There is another legacy problem maybe we need take care. > > > > Any thought about how releasing the stack memory[1] after calling > > ltp_clone_quick() in a test? > > > > [1] which allocated memory in ltp_clone_alloc(). > > Well, we can free the memory in ltp_clone_alloc() if the child runs in a > separate memory space, but if CLONE_VM was passed in flags there is no > way how to free the memory... > A stupid way come up to my mind is just to export the stack as global pointer and releasing in do_cleanup() if possible. diff --git a/include/tst_clone.h b/include/tst_clone.h index fd52097..c98eb44 100644 --- a/include/tst_clone.h +++ b/include/tst_clone.h diff --git a/include/tst_clone.h b/include/tst_clone.h index fd52097..c98eb44 100644 --- a/include/tst_clone.h +++ b/include/tst_clone.h @@ -24,6 +24,8 @@ #ifndef TST_CLONE_H__ #define TST_CLONE_H__ +void *tst_clone_stack; + /* Functions from lib/cloner.c */ int ltp_clone(unsigned long flags, int (*fn)(void *arg), void *arg, size_t stack_size, void *stack); diff --git a/lib/cloner.c b/lib/cloner.c index 8469745..4534982 100644 --- a/lib/cloner.c +++ b/lib/cloner.c @@ -142,19 +142,17 @@ int ltp_clone_alloc(unsigned long clone_flags, int (*fn) (void *arg), void *arg, size_t stack_size) { - void *stack; int ret; int saved_errno; - - stack = ltp_alloc_stack(stack_size); - if (stack == NULL) + tst_clone_stack = ltp_alloc_stack(stack_size); + if (tst_clone_stack == NULL) return -1; - ret = ltp_clone(clone_flags, fn, arg, stack_size, stack); + ret = ltp_clone(clone_flags, fn, arg, stack_size, tst_clone_stack); if (ret == -1) { saved_errno = errno; - free(stack); + free(tst_clone_stack); errno = saved_errno; } diff --git a/lib/tst_test.c b/lib/tst_test.c index 95f389d..e96a9c7 100644 --- a/lib/tst_test.c +++ b/lib/tst_test.c @@ -909,6 +909,9 @@ static void do_cleanup(void) if (mntpoint_mounted) tst_umount(tst_test->mntpoint); + if (tst_clone_stack) + free(tst_clone_stack); + if (tst_test->needs_device && tdev.dev) tst_release_device(tdev.dev);
Hi! > > Well, we can free the memory in ltp_clone_alloc() if the child runs in a > > separate memory space, but if CLONE_VM was passed in flags there is no > > way how to free the memory... > > > > A stupid way come up to my mind is just to export the stack as global > pointer and releasing in do_cleanup() if possible. This only works if you call the function only once, if you clone two or more you end up in the very same situation as before. Also we will have to figure out similar problem as a part of https://github.com/linux-test-project/ltp/issues/531 anyways, so I wouldn't bother at this point.
diff --git a/include/tst_clone.h b/include/tst_clone.h index 786cee5d1209..fd52097e2072 100644 --- a/include/tst_clone.h +++ b/include/tst_clone.h @@ -29,7 +29,7 @@ int ltp_clone(unsigned long flags, int (*fn)(void *arg), void *arg, size_t stack_size, void *stack); int ltp_clone7(unsigned long flags, int (*fn)(void *arg), void *arg, size_t stack_size, void *stack, ...); -int ltp_clone_malloc(unsigned long clone_flags, int (*fn)(void *arg), +int ltp_clone_alloc(unsigned long clone_flags, int (*fn)(void *arg), void *arg, size_t stacksize); int ltp_clone_quick(unsigned long clone_flags, int (*fn)(void *arg), void *arg); diff --git a/lib/cloner.c b/lib/cloner.c index cf37184aa22a..8469745d004b 100644 --- a/lib/cloner.c +++ b/lib/cloner.c @@ -135,11 +135,11 @@ void *ltp_alloc_stack(size_t size) } /* - * ltp_clone_malloc: also does the memory allocation for clone with a + * ltp_clone_alloc: also does the memory allocation for clone with a * caller-specified size. */ int -ltp_clone_malloc(unsigned long clone_flags, int (*fn) (void *arg), void *arg, +ltp_clone_alloc(unsigned long clone_flags, int (*fn) (void *arg), void *arg, size_t stack_size) { void *stack; @@ -162,7 +162,7 @@ ltp_clone_malloc(unsigned long clone_flags, int (*fn) (void *arg), void *arg, } /* - * ltp_clone_quick: calls ltp_clone_malloc with predetermined stack size. + * ltp_clone_quick: calls ltp_clone_alloc with predetermined stack size. * Experience thus far suggests that one page is often insufficient, * while 6*getpagesize() seems adequate. */ @@ -170,5 +170,5 @@ int ltp_clone_quick(unsigned long clone_flags, int (*fn) (void *arg), void *arg) { size_t stack_size = getpagesize() * 6; - return ltp_clone_malloc(clone_flags, fn, arg, stack_size); + return ltp_clone_alloc(clone_flags, fn, arg, stack_size); }
There are no users outside of lib. Signed-off-by: Jan Stancek <jstancek@redhat.com> --- include/tst_clone.h | 2 +- lib/cloner.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)