Re: [PATCH] fix leak of shared memory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 23 Sep 2008 15:43:12 +0300
Doron Shoham <dorons@xxxxxxxxxxxx> wrote:

> FUJITA Tomonori wrote:
> > On Mon, 15 Sep 2008 17:55:04 +0300
> > Doron Shoham <dorons@xxxxxxxxxxxx> wrote:
> > 
> >> fix problem of shared memory (used for logging) not released when tgtd shutdown.
> >> this patch fixes the shared memory leak when tgtd is killed.
> >> add error messages when log initialization fails.
> >>
> >> Signed-off-by: Doron Shoham <dorons@xxxxxxxxxxxx>
> >> ---
> >>  usr/log.c  |  181 ++++++++++++++++++++++++++++++++++++++---------------------
> >>  usr/log.h  |    7 +-
> >>  usr/tgtd.c |   37 ++++++++++--
> >>  3 files changed, 150 insertions(+), 75 deletions(-)
> > 
> > This patch can't handle the leak due to SIG_KILL?
> 
> Normally you should terminate tgtd with SIG_TERM.

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


> Currently tgtd isn't catching this signal.
> So, when you use SIG_KILL the shared memory, which was allocated, is not being cleaned up.
> In this patch I tried to do the same as in open-iscsi -
> clean up all the shared memory when stopping tgtd with SIG_TERM.

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).

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.

--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux SCSI]     [Linux RAID]     [Linux Clusters]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]

  Powered by Linux