[LTP] [PATCH v2] shm_test: Fix parameter passing to threads
Cyril Hrubis
chrubis@suse.cz
Fri Oct 11 11:08:52 CEST 2019
Hi!
> The arguments to all threads were passed using a pointer to the same memory.
> So they all point to the same data, that is overriden by the main thread
> to prepare it for the next thread.
Good catch, a few comments below.
> Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
> ---
> testcases/kernel/mem/mtest07/shm_test.c | 70 ++++++++++++-------------
> 1 file changed, 35 insertions(+), 35 deletions(-)
>
> diff --git a/testcases/kernel/mem/mtest07/shm_test.c b/testcases/kernel/mem/mtest07/shm_test.c
> index de91b7427..852c51b43 100644
> --- a/testcases/kernel/mem/mtest07/shm_test.c
> +++ b/testcases/kernel/mem/mtest07/shm_test.c
> @@ -81,8 +81,16 @@ void noprintf(char *string, ...)
>
> #define MAXT 30 /* default number of threads to create. */
> #define MAXR 1000 /* default number of repatetions to execute */
> -#define WRITER 0 /* cause thread function to shmat and write */
> -#define READER 1 /* cause thread function to shmat and read */
> +
> +struct child_args
> +{
> + pthread_t threadid;
> + int num_reps;
> + int shmkey;
> + int map_size;
> + int isReader;
Mixed case is frowned upon in LKML coding style, so this should be
is_reader instead.
> +};
> +
>
> /******************************************************************************/
> /* */
> @@ -166,28 +174,25 @@ static int rm_shared_mem(key_t shm_id, /* id of shared memory segment to be remo
> /* Return: exits with -1 on error, 0 on success */
> /* */
> /******************************************************************************/
> -static void *shmat_rd_wr(void *args)
> +static void *shmat_rd_wr(void *vargs)
> { /* arguments to the thread function */
> int shmndx = 0; /* index to the number of attach and detach */
> int index = 0; /* index to the number of blocks touched */
> - int reader = 0; /* this thread is a reader thread if set to 1 */
> key_t shm_id = 0; /* shared memory id */
> - long *locargs = /* local pointer to arguments */
> - (long *)args;
> + struct child_args *args = (struct child_args *)vargs;
^
This cast is useless in C.
> volatile int exit_val = 0; /* exit value of the pthread */
> char *read_from_mem; /* ptr to touch each (4096) block in memory */
> char *write_to_mem; /* ptr to touch each (4096) block in memory */
> char *shmat_addr; /* address of the attached memory */
> char buff; /* temporary buffer */
>
> - reader = (int)locargs[3];
> - while (shmndx++ < (int)locargs[0]) {
> + while (shmndx++ < args->num_reps) {
> dprt("pid[%d]: shmat_rd_wr(): locargs[1] = %#x\n",
> - getpid(), (int)locargs[1]);
> + getpid(), args->shmkey);
>
> /* get shared memory id */
> if ((shm_id =
> - shmget((int)locargs[1], (int)locargs[2], IPC_CREAT | 0666))
> + shmget(args->shmkey, args->map_size, IPC_CREAT | 0666))
> == -1) {
> dprt("pid[%d]: shmat_rd_wr(): shmget failed\n",
> getpid());
> @@ -213,11 +218,11 @@ static void *shmat_rd_wr(void *args)
> "pid[%d]: do_shmat_shmadt(): got shmat address = %#lx\n",
> getpid(), (long)shmat_addr);
>
> - if (!reader) {
> + if (args->isReader) {
> /* write character 'Y' to that memory area */
> index = 0;
> write_to_mem = shmat_addr;
> - while (index < (int)locargs[2]) {
> + while (index < args->num_reps) {
Isn't locargs[2] map_size, or did I miss something?
> dprt("pid[%d]: do_shmat_shmatd(): write_to_mem = %#x\n", getpid(), write_to_mem);
> *write_to_mem = 'Y';
> index++;
> @@ -228,7 +233,7 @@ static void *shmat_rd_wr(void *args)
> /* read from the memory area */
> index = 0;
> read_from_mem = shmat_addr;
> - while (index < (int)locargs[2]) {
> + while (index < args->num_reps) {
Here as well.
> buff = *read_from_mem;
> index++;
> read_from_mem++;
> @@ -272,12 +277,11 @@ int main(int argc, /* number of input parameters */
> int c; /* command line options */
> int num_thrd = MAXT; /* number of threads to create */
> int num_reps = MAXR; /* number of repatitions the test is run */
> - int thrd_ndx; /* index into the array of thread ids */
> + int i;
> void *th_status; /* exit status of LWP's */
> int map_size; /* size of the file mapped. */
> int shmkey = 1969; /* key used to generate shmid by shmget() */
> - pthread_t thrdid[30]; /* maxinum of 30 threads allowed */
> - long chld_args[4]; /* arguments to the thread function */
> + struct child_args chld_args[30]; /* arguments to the thread function */
> char *map_address = NULL;
> /* address in memory of the mapped file */
> extern int optopt; /* options to the program */
> @@ -299,7 +303,7 @@ int main(int argc, /* number of input parameters */
> case 't':
> if ((num_thrd = atoi(optarg)) == 0)
> OPT_MISSING(argv[0], optopt);
> - else if (num_thrd < 0) {
> + else if (num_thrd < 0 || num_thrd > MAXT) {
> fprintf(stdout,
> "WARNING: bad argument. Using default\n");
> num_thrd = MAXT;
> @@ -311,31 +315,27 @@ int main(int argc, /* number of input parameters */
> }
> }
>
> - chld_args[0] = num_reps;
> -
> - for (thrd_ndx = 0; thrd_ndx < num_thrd; thrd_ndx += 2) {
> + for (i = 0; i < num_thrd; i += 2) {
> srand(time(NULL) % 100);
> - map_size =
> - (1 + (int)(1000.0 * rand() / (RAND_MAX + 1.0))) * 4096;
> -
> - chld_args[1] = shmkey++;
> - chld_args[2] = map_size;
> + map_size = (1 + (int)(1000.0 * rand() / (RAND_MAX + 1.0))) * 4096;
>
> dprt("main(): thrd_ndx = %d map_address = %#x map_size = %d\n",
> - thrd_ndx, map_address, map_size);
> -
> - chld_args[3] = WRITER;
> + i, map_address, map_size);
>
> + chld_args[i].num_reps = num_reps;
> + chld_args[i].map_size = map_size;
> + chld_args[i].shmkey = shmkey++;
> + chld_args[i].isReader = 0;
> if (pthread_create
> - (&thrdid[thrd_ndx], NULL, shmat_rd_wr, chld_args)) {
> + (&chld_args[i].threadid, NULL, shmat_rd_wr, &chld_args[i])) {
> perror("shmat_rd_wr(): pthread_create()");
> exit(-1);
> }
>
> - chld_args[3] = READER;
> -
> + chld_args[i + 1] = chld_args[i];
> + chld_args[i + 1].isReader = 1;
> if (pthread_create
> - (&thrdid[thrd_ndx + 1], NULL, shmat_rd_wr, chld_args)) {
> + (&chld_args[i + 1].threadid, NULL, shmat_rd_wr, &chld_args[i + 1])) {
> perror("shmat_rd_wr(): pthread_create()");
> exit(-1);
> }
> @@ -343,8 +343,8 @@ int main(int argc, /* number of input parameters */
>
> sync();
>
> - for (thrd_ndx = 0; thrd_ndx < num_thrd; thrd_ndx++) {
> - if (pthread_join(thrdid[thrd_ndx], &th_status) != 0) {
> + for (i = 0; i < num_thrd; i++) {
> + if (pthread_join(chld_args[i].threadid, &th_status) != 0) {
> perror("shmat_rd_wr(): pthread_join()");
> exit(-1);
> } else {
> @@ -352,7 +352,7 @@ int main(int argc, /* number of input parameters */
> if (th_status == (void *)-1) {
> fprintf(stderr,
> "thread [%ld] - process exited with errors\n",
> - (long)thrdid[thrd_ndx]);
> + (long)chld_args[i].threadid);
> exit(-1);
> }
> }
> --
> 2.20.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
More information about the ltp
mailing list