[PATCH 0/2] cyclictest: fix growing shm stat file

[PATCH 0/2] cyclictest: fix growing shm stat file

[Lukas Beckmann]

I recently started using cyclictests with get_cyclictest_snapshot and
noticed that the shm stat files grows indefinitely.

Example:
for i in {1..10}; do
  killall -USR2 cyclictest
  sleep 0.01
  ls -lh /dev/shm/cyclictest* | awk '{print "Size: " $5}'
done
Size: 1,4K
Size: 2,8K
Size: 4,1K
Size: 5,5K
Size: 6,8K
Size: 8,2K
Size: 9,5K
Size: 11K
Size: 13K
Size: 14K
   
Signed-off-by: Lukas Beckmann <lbckmnn@mailbox.org>

Lukas Beckmann (2):
  cyclictest: fix growing shm stat file
  cyclictest: simplify rstat_setup

 src/cyclictest/cyclictest.c | 90 ++++++-------------------------------
 1 file changed, 14 insertions(+), 76 deletions(-)

-- 
2.53.0

[PATCH 1/2] cyclictest: fix growing shm stat file

[Lukas Beckmann]

On a received USR2 signal, the current statistics are written to a file
in shared memory. Before that happens, it is truncated to zero to clear
it.

However, currently the shm file keeps growing with each received signal
because the seek pointer is not updated by ftruncate.

This is fixed by calling fseek after truncating.

Signed-off-by: Lukas Beckmann <lbckmnn@mailbox.org>
---
 src/cyclictest/cyclictest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 191945e..960c905 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -1472,6 +1472,7 @@ static void sighand(int sig)
 			return;
 		}
 		rstat_ftruncate(rstat_fd, 0);
+		lseek(rstat_fd, 0, SEEK_SET);
 		quiet = 0;
 		dprintf(rstat_fd, "#---------------------------\n");
 		dprintf(rstat_fd, "# cyclictest current status:\n");
-- 
2.53.0

[PATCH 2/2] cyclictest: simplify rstat_setup

[Lukas Beckmann]

The shm file is truncated to zero each time before it is written.
It does not make sense to truncate it to something non zero on setup or
to mlock it.

Signed-off-by: Lukas Beckmann <lbckmnn@mailbox.org>
---
 src/cyclictest/cyclictest.c | 89 ++++++-------------------------------
 1 file changed, 13 insertions(+), 76 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 960c905..7b959ac 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -1771,33 +1771,6 @@ static void trigger_update(struct thread_param *par, int diff, int64_t ts)
 	pthread_mutex_unlock(&trigger_lock);
 }
 
-/* Running status shared memory open */
-static int rstat_shm_open(void)
-{
-	int fd;
-	pid_t pid;
-
-	pid = getpid();
-
-	snprintf(shm_name, SHM_BUF_SIZE, "%s%d", "/cyclictest", pid);
-
-	errno = 0;
-	fd = shm_unlink(shm_name);
-
-	if ((fd == -1) && (errno != ENOENT)) {
-		fprintf(stderr, "ERROR: shm_unlink %s\n", strerror(errno));
-		return fd;
-	}
-
-	errno = 9;
-	fd = shm_open(shm_name, O_RDWR|O_CREAT|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
-	if (fd == -1)
-		fprintf(stderr, "ERROR: shm_open %s\n", strerror(errno));
-
-	rstat_fd = fd;
-
-	return fd;
-}
 
 static int rstat_ftruncate(int fd, off_t len)
 {
@@ -1811,62 +1784,26 @@ static int rstat_ftruncate(int fd, off_t len)
 	return err;
 }
 
-static void *rstat_mmap(int fd)
-{
-	void *mptr;
 
-	errno = 0;
-	mptr = mmap(0, _SC_PAGE_SIZE, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+static void rstat_setup(void)
+{
+	int fd;
+	pid_t pid;
 
-	if (mptr == (void*)-1)
-		fprintf(stderr, "ERROR: mmap, %s\n", strerror(errno));
+	pid = getpid();
 
-	return mptr;
-}
+	snprintf(shm_name, SHM_BUF_SIZE, "%s%d", "/cyclictest", pid);
 
-static int rstat_mlock(void *mptr)
-{
-	int err;
+	fd = shm_unlink(shm_name);
 
-	errno = 0;
-	err = mlock(mptr, _SC_PAGE_SIZE);
-	if (err == -1)
-		fprintf(stderr, "ERROR, mlock %s\n", strerror(errno));
+	if ((fd == -1) && (errno != ENOENT))
+		fprintf(stderr, "ERROR: shm_unlink %s\n", strerror(errno));
 
-	return err;
-}
+	fd = shm_open(shm_name, O_RDWR|O_CREAT|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
+	if (fd == -1)
+		fprintf(stderr, "ERROR: shm_open %s\n", strerror(errno));
 
-static void rstat_setup(void)
-{
-	int res;
-	void *mptr = NULL;
-
-	int sfd = rstat_shm_open();
-	if (sfd < 0)
-		goto rstat_err;
-
-	res = rstat_ftruncate(sfd, _SC_PAGE_SIZE);
-	if (res)
-		goto rstat_err1;
-
-	mptr = rstat_mmap(sfd);
-	if (mptr == MAP_FAILED)
-		goto rstat_err1;
-
-	res = rstat_mlock(mptr);
-	if (res)
-		goto rstat_err2;
-
-	return;
-
-rstat_err2:
-	munmap(mptr, _SC_PAGE_SIZE);
-rstat_err1:
-	close(sfd);
-	shm_unlink(shm_name);
-rstat_err:
-	rstat_fd = -1;
-	return;
+	rstat_fd = fd;
 }
 
 static void write_stats(FILE *f, void *data)
-- 
2.53.0

Re: [PATCH 1/2] cyclictest: fix growing shm stat file

[John Kacur]

On Sun, 1 Mar 2026, Lukas Beckmann wrote:

> On a received USR2 signal, the current statistics are written to a file
> in shared memory. Before that happens, it is truncated to zero to clear
> it.
> 
> However, currently the shm file keeps growing with each received signal
> because the seek pointer is not updated by ftruncate.
> 
> This is fixed by calling fseek after truncating.
> 
> Signed-off-by: Lukas Beckmann <lbckmnn@mailbox.org>
> ---
>  src/cyclictest/cyclictest.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 191945e..960c905 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -1472,6 +1472,7 @@ static void sighand(int sig)
>  			return;
>  		}
>  		rstat_ftruncate(rstat_fd, 0);
> +		lseek(rstat_fd, 0, SEEK_SET);
>  		quiet = 0;
>  		dprintf(rstat_fd, "#---------------------------\n");
>  		dprintf(rstat_fd, "# cyclictest current status:\n");
> -- 
> 2.53.0
> 
> 
> 
Signed-off-by: John Kacur <jkacur@redhat.com>

Re: [PATCH 2/2] cyclictest: simplify rstat_setup

[John Kacur]

On Sun, 1 Mar 2026, Lukas Beckmann wrote:

> The shm file is truncated to zero each time before it is written.
> It does not make sense to truncate it to something non zero on setup or
> to mlock it.
> 
> Signed-off-by: Lukas Beckmann <lbckmnn@mailbox.org>
> ---
>  src/cyclictest/cyclictest.c | 89 ++++++-------------------------------
>  1 file changed, 13 insertions(+), 76 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 960c905..7b959ac 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -1771,33 +1771,6 @@ static void trigger_update(struct thread_param *par, int diff, int64_t ts)
>  	pthread_mutex_unlock(&trigger_lock);
>  }
>  
> -/* Running status shared memory open */
> -static int rstat_shm_open(void)
> -{
> -	int fd;
> -	pid_t pid;
> -
> -	pid = getpid();
> -
> -	snprintf(shm_name, SHM_BUF_SIZE, "%s%d", "/cyclictest", pid);
> -
> -	errno = 0;
> -	fd = shm_unlink(shm_name);
> -
> -	if ((fd == -1) && (errno != ENOENT)) {
> -		fprintf(stderr, "ERROR: shm_unlink %s\n", strerror(errno));
> -		return fd;
> -	}
> -
> -	errno = 9;
> -	fd = shm_open(shm_name, O_RDWR|O_CREAT|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
> -	if (fd == -1)
> -		fprintf(stderr, "ERROR: shm_open %s\n", strerror(errno));
> -
> -	rstat_fd = fd;
> -
> -	return fd;
> -}
>  
>  static int rstat_ftruncate(int fd, off_t len)
>  {
> @@ -1811,62 +1784,26 @@ static int rstat_ftruncate(int fd, off_t len)
>  	return err;
>  }
>  
> -static void *rstat_mmap(int fd)
> -{
> -	void *mptr;
>  
> -	errno = 0;
> -	mptr = mmap(0, _SC_PAGE_SIZE, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
> +static void rstat_setup(void)
> +{
> +	int fd;
> +	pid_t pid;
>  
> -	if (mptr == (void*)-1)
> -		fprintf(stderr, "ERROR: mmap, %s\n", strerror(errno));
> +	pid = getpid();
>  
> -	return mptr;
> -}
> +	snprintf(shm_name, SHM_BUF_SIZE, "%s%d", "/cyclictest", pid);
>  
> -static int rstat_mlock(void *mptr)
> -{
> -	int err;

errno = 0 before the call to shm_unlink for good defensive programming
other than that the patch looks good, I will do a little more testing

> +	fd = shm_unlink(shm_name);
>  
> -	errno = 0;
> -	err = mlock(mptr, _SC_PAGE_SIZE);
> -	if (err == -1)
> -		fprintf(stderr, "ERROR, mlock %s\n", strerror(errno));
> +	if ((fd == -1) && (errno != ENOENT))
> +		fprintf(stderr, "ERROR: shm_unlink %s\n", strerror(errno));
>  
> -	return err;
> -}
> +	fd = shm_open(shm_name, O_RDWR|O_CREAT|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
> +	if (fd == -1)
> +		fprintf(stderr, "ERROR: shm_open %s\n", strerror(errno));
>  
> -static void rstat_setup(void)
> -{
> -	int res;
> -	void *mptr = NULL;
> -
> -	int sfd = rstat_shm_open();
> -	if (sfd < 0)
> -		goto rstat_err;
> -
> -	res = rstat_ftruncate(sfd, _SC_PAGE_SIZE);
> -	if (res)
> -		goto rstat_err1;
> -
> -	mptr = rstat_mmap(sfd);
> -	if (mptr == MAP_FAILED)
> -		goto rstat_err1;
> -
> -	res = rstat_mlock(mptr);
> -	if (res)
> -		goto rstat_err2;
> -
> -	return;
> -
> -rstat_err2:
> -	munmap(mptr, _SC_PAGE_SIZE);
> -rstat_err1:
> -	close(sfd);
> -	shm_unlink(shm_name);
> -rstat_err:
> -	rstat_fd = -1;
> -	return;
> +	rstat_fd = fd;
>  }
>  
>  static void write_stats(FILE *f, void *data)
> -- 
> 2.53.0
> 
> 
> 

Re: [PATCH 2/2] cyclictest: simplify rstat_setup

[Crystal Wood]

On Mon, 2026-03-09 at 18:55 -0400, John Kacur wrote:
> 
> On Sun, 1 Mar 2026, Lukas Beckmann wrote:
> > +static void rstat_setup(void)
> > +{
> > +	int fd;
> > +	pid_t pid;
> >  
> > -	if (mptr == (void*)-1)
> > -		fprintf(stderr, "ERROR: mmap, %s\n", strerror(errno));
> > +	pid = getpid();
> >  
> > -	return mptr;
> > -}
> > +	snprintf(shm_name, SHM_BUF_SIZE, "%s%d", "/cyclictest", pid);
> >  
> > -static int rstat_mlock(void *mptr)
> > -{
> > -	int err;
> 
> errno = 0 before the call to shm_unlink for good defensive programming
> other than that the patch looks good, I will do a little more testing

Defensive against the library returning -1 without setting errno?  Seems
a bit paranoid and cluttery... all for the payoff of an "ERROR:
shm_unlink Success" message if it ever does happen *and* the old errno
happened to be ENOENT.

And the current code uses it even in places that print the message
regardless.

-Crystal

Re: [PATCH 2/2] cyclictest: simplify rstat_setup

[John Kacur]

On Tue, 10 Mar 2026, Crystal Wood wrote:

> On Mon, 2026-03-09 at 18:55 -0400, John Kacur wrote:
> > 
> > On Sun, 1 Mar 2026, Lukas Beckmann wrote:
> > > +static void rstat_setup(void)
> > > +{
> > > +	int fd;
> > > +	pid_t pid;
> > >  
> > > -	if (mptr == (void*)-1)
> > > -		fprintf(stderr, "ERROR: mmap, %s\n", strerror(errno));
> > > +	pid = getpid();
> > >  
> > > -	return mptr;
> > > -}
> > > +	snprintf(shm_name, SHM_BUF_SIZE, "%s%d", "/cyclictest", pid);
> > >  
> > > -static int rstat_mlock(void *mptr)
> > > -{
> > > -	int err;
> > 
> > errno = 0 before the call to shm_unlink for good defensive programming
> > other than that the patch looks good, I will do a little more testing
> 
> Defensive against the library returning -1 without setting errno?  Seems
> a bit paranoid and cluttery... all for the payoff of an "ERROR:
> shm_unlink Success" message if it ever does happen *and* the old errno
> happened to be ENOENT.
> 
> And the current code uses it even in places that print the message
> regardless.
> 
> -Crystal
> 
> 

Isn't that what defensive programming is? Guarding against something that 
is unlikely? It's one line of code that I requested in a patch that gets 
rid of many lines of code, is that really worth arguing over?

John Kacur

Re: [PATCH 2/2] cyclictest: simplify rstat_setup

[Crystal Wood]

On Tue, 2026-03-10 at 12:18 -0400, John Kacur wrote:
> 
> On Tue, 10 Mar 2026, Crystal Wood wrote:
> 
> > On Mon, 2026-03-09 at 18:55 -0400, John Kacur wrote:
> > > 
> > > On Sun, 1 Mar 2026, Lukas Beckmann wrote:
> > > > +static void rstat_setup(void)
> > > > +{
> > > > +	int fd;
> > > > +	pid_t pid;
> > > >  
> > > > -	if (mptr == (void*)-1)
> > > > -		fprintf(stderr, "ERROR: mmap, %s\n", strerror(errno));
> > > > +	pid = getpid();
> > > >  
> > > > -	return mptr;
> > > > -}
> > > > +	snprintf(shm_name, SHM_BUF_SIZE, "%s%d", "/cyclictest", pid);
> > > >  
> > > > -static int rstat_mlock(void *mptr)
> > > > -{
> > > > -	int err;
> > > 
> > > errno = 0 before the call to shm_unlink for good defensive programming
> > > other than that the patch looks good, I will do a little more testing
> > 
> > Defensive against the library returning -1 without setting errno?  Seems
> > a bit paranoid and cluttery... all for the payoff of an "ERROR:
> > shm_unlink Success" message if it ever does happen *and* the old errno
> > happened to be ENOENT.
> > 
> > And the current code uses it even in places that print the message
> > regardless.
> > 
> > -Crystal
> > 
> > 
> 
> Isn't that what defensive programming is? Guarding against something that 
> is unlikely? It's one line of code that I requested in a patch that gets 
> rid of many lines of code, is that really worth arguing over?

Eh, it wasn't so much an argument over this patch, as a "we're supposed
to do *what* before each library call?" reflex.

-Crystal