[stgt] [PATCH] fix leak of shared memory

[FUJITA Tomonori]

On Wed, 24 Sep 2008 18:33:52 +0300
Doron Shoham <dorons at Voltaire.COM> wrote:

> > No, terminating tgtd with SIG_TERM is not the right way to stop
> > tgtd.
> > 
> > You should use the following command:
> > 
> > tgtadm --op delete --mode system
> 
> I wasn't aware of this option.
> I couldn't find any info about "--mode system".
> It's not written in tgtadm help nor in any man/readme file.

We have lots of undocumented features (they are mentioned only on the
mailing list). Documents are always welcome.


> Anyway, using this command doesn't solve the memory leakage.

Yeah, I know. I didn't say that 'tgtadm --op delete --mode system'
removes the shared memory.


> > If you handle SIG_TERM, it's more logical to handle SIG_KILL. If you
> > handle both, we can say that we always clean up shared memory (this is
> > optimal).
> 
> How can you handle SIG_KILL?
> 
> This from the sigaction man page:
> "EINVAL An  invalid  signal  was  specified.   This  will also be generated if an attempt is made to change the action for SIGKILL or
> SIGSTOP, which cannot be caught or ignored."
> 
> In the patch I've added the handle of SIG_INT as well (for running tgtd in the foreground).

Well, you can't catch SIG_KILL, but it means that we can't clean up
the shared memory?

How about the attached patch? I'm not sure it's a proper way to handle
the shared memory but it always works for me.

The manpage says:

IPC_RMID

Mark the segment to be destroyed. The segment will only actually be
destroyed after the last process detaches it (i.e., when the
shm_nattch member of the associated structure shmid_ds is zero). 


As the patch does, if tgtd uses IPC_RMID after attaching tgtd, the
shared memory will not be destroyed until tgtd dies?


> > If you ignore SIG_KILL because it's not the right way to shut down
> > tgtd, you should ignore SIG_TERM too since SIG_TERM is not the right
> > way too.
> > 
> > Well, the leak of shared memory happens only when shutting down
> > tgtd. So I don't care much how to handle it.
> 
> No, it happen all the time...

What do you mean?

tgtd creates shared memory at startup and exits without cleaning up
it. So unless you start and stop tgtd again and again, the leak is not
an issue at all.


diff --git a/usr/log.c b/usr/log.c
index 076c770..f844a7c 100644
--- a/usr/log.c
+++ b/usr/log.c
@@ -59,6 +59,8 @@ static int logarea_init (int size)
 	if (!la)
 		return 1;
 
+	shmctl(shmid, IPC_RMID, NULL);
+
 	if (size < MAX_MSG_SIZE)
 		size = LOG_SPACE_SIZE;
 
@@ -75,6 +77,8 @@ static int logarea_init (int size)
 	}
 	memset(la->start, 0, size);
 
+	shmctl(shmid, IPC_RMID, NULL);
+
 	la->empty = 1;
 	la->end = la->start + size;
 	la->head = la->start;
@@ -93,6 +97,8 @@ static int logarea_init (int size)
 		return 1;
 	}
 
+	shmctl(shmid, IPC_RMID, NULL);
+
 	if ((la->semid = semget(SEMKEY, 1, 0666 | IPC_CREAT)) < 0) {
 		shmdt(la->buff);
 		shmdt(la->start);
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html