From: Dmitry Kurochkin <dmitry.kurochkin@measurement-factory.com>
Date: Thu, 27 Oct 2011 23:14:28 +0000 (-0600)
Subject: Bug 3150: do not start useless unlinkd.
X-Git-Tag: BumpSslServerFirst.take01~64
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c521ad17dc07ce641e345555b991a03d4ed8beae;p=thirdparty%2Fsquid.git

Bug 3150: do not start useless unlinkd.

Unlinkd may be used only by UFS storage but, before the change, Squid
always started unlinkd if it was built, even if it was not needed.

Whether a SwapDir may use unlinkd depends on the SwapDir
implementation and DiskIO strategy it uses.  The patch adds
unlinkdUseful() method to SwapDir and DiskIOStrategy to decide if
unlinkd should be started.

After the change, unlinkd may be started during reconfiguration and
unlinkdInit() may be called multiple times.

After the change, unlinkdClose() may be called when unlinkd was never
started.  The patch removes a warning which was printed in this case
on Windows.
---

diff --git a/src/DiskIO/AIO/AIODiskIOStrategy.cc b/src/DiskIO/AIO/AIODiskIOStrategy.cc
index 96e11f4cb9..7a946375f8 100644
--- a/src/DiskIO/AIO/AIODiskIOStrategy.cc
+++ b/src/DiskIO/AIO/AIODiskIOStrategy.cc
@@ -100,6 +100,12 @@ AIODiskIOStrategy::sync()
         callback();
 }
 
+bool
+AIODiskIOStrategy::unlinkdUseful() const
+{
+    return false;
+}
+
 void
 AIODiskIOStrategy::unlinkFile (char const *)
 {}
diff --git a/src/DiskIO/AIO/AIODiskIOStrategy.h b/src/DiskIO/AIO/AIODiskIOStrategy.h
index 9d461d0b7e..c3c7d34e78 100644
--- a/src/DiskIO/AIO/AIODiskIOStrategy.h
+++ b/src/DiskIO/AIO/AIODiskIOStrategy.h
@@ -52,6 +52,8 @@ public:
     virtual RefCount<DiskFile> newFile (char const *path);
     /* flush all IO operations  */
     virtual void sync();
+    /** whether the IO Strategy can use unlinkd */
+    virtual bool unlinkdUseful() const;
     /* unlink a file by path */
     virtual void unlinkFile (char const *);
 
diff --git a/src/DiskIO/Blocking/BlockingIOStrategy.cc b/src/DiskIO/Blocking/BlockingIOStrategy.cc
index 9a619734ef..86100375de 100644
--- a/src/DiskIO/Blocking/BlockingIOStrategy.cc
+++ b/src/DiskIO/Blocking/BlockingIOStrategy.cc
@@ -57,6 +57,12 @@ BlockingIOStrategy::newFile (char const *path)
     return new BlockingFile (path);
 }
 
+bool
+BlockingIOStrategy::unlinkdUseful() const
+{
+    return true;
+}
+
 void
 BlockingIOStrategy::unlinkFile(char const *path)
 {
diff --git a/src/DiskIO/Blocking/BlockingIOStrategy.h b/src/DiskIO/Blocking/BlockingIOStrategy.h
index 5dea6e2a8e..8181dd1b4a 100644
--- a/src/DiskIO/Blocking/BlockingIOStrategy.h
+++ b/src/DiskIO/Blocking/BlockingIOStrategy.h
@@ -45,6 +45,7 @@ public:
     virtual bool shedLoad();
     virtual int load();
     virtual RefCount<DiskFile> newFile(char const *path);
+    virtual bool unlinkdUseful() const;
     virtual void unlinkFile (char const *);
 };
 
diff --git a/src/DiskIO/DiskDaemon/DiskdIOStrategy.cc b/src/DiskIO/DiskDaemon/DiskdIOStrategy.cc
index 6d054e3056..64c70128c5 100644
--- a/src/DiskIO/DiskDaemon/DiskdIOStrategy.cc
+++ b/src/DiskIO/DiskDaemon/DiskdIOStrategy.cc
@@ -103,6 +103,12 @@ DiskdIOStrategy::newFile(char const *path)
 DiskdIOStrategy::DiskdIOStrategy() : magic1(64), magic2(72), away(0) , smsgid(-1), rmsgid(-1), wfd(-1) , instanceID(newInstance())
 {}
 
+bool
+DiskdIOStrategy::unlinkdUseful() const
+{
+    return true;
+}
+
 void
 DiskdIOStrategy::unlinkFile(char const *path)
 {
diff --git a/src/DiskIO/DiskDaemon/DiskdIOStrategy.h b/src/DiskIO/DiskDaemon/DiskdIOStrategy.h
index 161d7edde7..650400e922 100644
--- a/src/DiskIO/DiskDaemon/DiskdIOStrategy.h
+++ b/src/DiskIO/DiskDaemon/DiskdIOStrategy.h
@@ -76,6 +76,7 @@ public:
     virtual bool shedLoad();
     virtual int load();
     virtual RefCount<DiskFile> newFile(char const *path);
+    virtual bool unlinkdUseful() const;
     virtual void unlinkFile (char const *);
     virtual ConfigOption *getOptionTree() const;
     virtual void init();
diff --git a/src/DiskIO/DiskIOStrategy.h b/src/DiskIO/DiskIOStrategy.h
index 2ec561714d..91990d2e46 100644
--- a/src/DiskIO/DiskIOStrategy.h
+++ b/src/DiskIO/DiskIOStrategy.h
@@ -59,6 +59,9 @@ public:
     /** flush all IO operations  */
     virtual void sync() {}
 
+    /** whether the IO Strategy can use unlinkd */
+    virtual bool unlinkdUseful() const = 0;
+
     /** unlink a file by path */
     virtual void unlinkFile(char const *) = 0;
 
@@ -92,6 +95,8 @@ public:
 
     virtual void sync() { io->sync(); }
 
+    virtual bool unlinkdUseful() const { return io->unlinkdUseful(); }
+
     virtual void unlinkFile (char const *path) { io->unlinkFile(path); }
 
     virtual int callback() { return io->callback(); }
diff --git a/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.cc b/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.cc
index e7c0fd9a42..1ba1ecf766 100644
--- a/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.cc
+++ b/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.cc
@@ -254,6 +254,12 @@ DiskThreadsIOStrategy::newFile (char const *path)
     return new DiskThreadsDiskFile (path, this);
 }
 
+bool
+DiskThreadsIOStrategy::unlinkdUseful() const
+{
+    return false;
+}
+
 void
 DiskThreadsIOStrategy::unlinkFile(char const *path)
 {
diff --git a/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.h b/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.h
index 12da07692a..d7e54b1db3 100644
--- a/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.h
+++ b/src/DiskIO/DiskThreads/DiskThreadsIOStrategy.h
@@ -54,6 +54,7 @@ public:
     virtual bool shedLoad();
     virtual int load();
     virtual RefCount<DiskFile> newFile(char const *path);
+    virtual bool unlinkdUseful() const;
     virtual void unlinkFile (char const *);
     virtual int callback();
     virtual void sync();
diff --git a/src/DiskIO/IpcIo/IpcIoIOStrategy.cc b/src/DiskIO/IpcIo/IpcIoIOStrategy.cc
index 10a165e0b6..dc3a191ed8 100644
--- a/src/DiskIO/IpcIo/IpcIoIOStrategy.cc
+++ b/src/DiskIO/IpcIo/IpcIoIOStrategy.cc
@@ -28,6 +28,12 @@ IpcIoIOStrategy::newFile (char const *path)
     return new IpcIoFile (path);
 }
 
+bool
+IpcIoIOStrategy::unlinkdUseful() const
+{
+    return true;
+}
+
 void
 IpcIoIOStrategy::unlinkFile(char const *path)
 {
diff --git a/src/DiskIO/IpcIo/IpcIoIOStrategy.h b/src/DiskIO/IpcIo/IpcIoIOStrategy.h
index 3bd2945bbc..447e8e34b1 100644
--- a/src/DiskIO/IpcIo/IpcIoIOStrategy.h
+++ b/src/DiskIO/IpcIo/IpcIoIOStrategy.h
@@ -9,6 +9,7 @@ public:
     virtual bool shedLoad();
     virtual int load();
     virtual RefCount<DiskFile> newFile(char const *path);
+    virtual bool unlinkdUseful() const;
     virtual void unlinkFile (char const *);
 };
 
diff --git a/src/DiskIO/Mmapped/MmappedIOStrategy.cc b/src/DiskIO/Mmapped/MmappedIOStrategy.cc
index 6f015905b6..3f6d68184a 100644
--- a/src/DiskIO/Mmapped/MmappedIOStrategy.cc
+++ b/src/DiskIO/Mmapped/MmappedIOStrategy.cc
@@ -28,6 +28,12 @@ MmappedIOStrategy::newFile (char const *path)
     return new MmappedFile (path);
 }
 
+bool
+MmappedIOStrategy::unlinkdUseful() const
+{
+    return true;
+}
+
 void
 MmappedIOStrategy::unlinkFile(char const *path)
 {
diff --git a/src/DiskIO/Mmapped/MmappedIOStrategy.h b/src/DiskIO/Mmapped/MmappedIOStrategy.h
index c295427269..2d17524bb4 100644
--- a/src/DiskIO/Mmapped/MmappedIOStrategy.h
+++ b/src/DiskIO/Mmapped/MmappedIOStrategy.h
@@ -9,6 +9,7 @@ public:
     virtual bool shedLoad();
     virtual int load();
     virtual RefCount<DiskFile> newFile(char const *path);
+    virtual bool unlinkdUseful() const;
     virtual void unlinkFile (char const *);
 };
 
diff --git a/src/SwapDir.h b/src/SwapDir.h
index a1c54cac20..8247143775 100644
--- a/src/SwapDir.h
+++ b/src/SwapDir.h
@@ -133,6 +133,8 @@ public:
     virtual bool active() const; ///< may be used in this strand
     /// whether stat should be reported by this SwapDir
     virtual bool doReportStat() const { return active(); }
+    /// whether SwapDir may benefit from unlinkd
+    virtual bool unlinkdUseful() const = 0;
 
     /* official Store interface functions */
     virtual void diskFull();
diff --git a/src/fs/coss/CossSwapDir.h b/src/fs/coss/CossSwapDir.h
index 6929d81d77..8279d27c49 100644
--- a/src/fs/coss/CossSwapDir.h
+++ b/src/fs/coss/CossSwapDir.h
@@ -35,6 +35,7 @@ public:
     virtual void dump(StoreEntry &)const;
     ~CossSwapDir();
     virtual StoreSearch *search(String const url, HttpRequest *);
+    virtual bool unlinkdUseful() const;
     virtual void unlink (StoreEntry &);
     virtual void statfs (StoreEntry &)const;
     virtual bool canStore(const StoreEntry &e, int64_t diskSpaceNeeded, int &load) const;
diff --git a/src/fs/coss/store_io_coss.cc b/src/fs/coss/store_io_coss.cc
index bb4c09db4e..8019d13cca 100644
--- a/src/fs/coss/store_io_coss.cc
+++ b/src/fs/coss/store_io_coss.cc
@@ -129,6 +129,13 @@ CossSwapDir::allocate(const StoreEntry * e, int which)
     }
 }
 
+bool
+CossSwapDir::unlinkdUseful() const
+{
+    // no entry-specific files to unlink
+    return false;
+}
+
 void
 CossSwapDir::unlink(StoreEntry & e)
 {
diff --git a/src/fs/rock/RockSwapDir.cc b/src/fs/rock/RockSwapDir.cc
index 77b62b6dcf..ab56abdf6b 100644
--- a/src/fs/rock/RockSwapDir.cc
+++ b/src/fs/rock/RockSwapDir.cc
@@ -749,6 +749,13 @@ Rock::SwapDir::dereference(StoreEntry &e)
     return false;
 }
 
+bool
+Rock::SwapDir::unlinkdUseful() const
+{
+    // no entry-specific files to unlink
+    return false;
+}
+
 void
 Rock::SwapDir::unlink(StoreEntry &e)
 {
diff --git a/src/fs/rock/RockSwapDir.h b/src/fs/rock/RockSwapDir.h
index 0fbcada4ff..f427c57fff 100644
--- a/src/fs/rock/RockSwapDir.h
+++ b/src/fs/rock/RockSwapDir.h
@@ -54,6 +54,7 @@ protected:
     virtual void diskFull();
     virtual void reference(StoreEntry &e);
     virtual bool dereference(StoreEntry &e);
+    virtual bool unlinkdUseful() const;
     virtual void unlink(StoreEntry &e);
     virtual void statfs(StoreEntry &e) const;
 
diff --git a/src/fs/ufs/store_dir_ufs.cc b/src/fs/ufs/store_dir_ufs.cc
index 129629c97f..7ee1df8cf5 100644
--- a/src/fs/ufs/store_dir_ufs.cc
+++ b/src/fs/ufs/store_dir_ufs.cc
@@ -1287,6 +1287,13 @@ UFSSwapDir::unlinkFile(sfileno f)
     IO->unlinkFile(fullPath(f,NULL));
 }
 
+bool
+UFSSwapDir::unlinkdUseful() const
+{
+    // unlinkd may be useful only in workers
+    return IamWorkerProcess() && IO->io->unlinkdUseful();
+}
+
 void
 UFSSwapDir::unlink(StoreEntry & e)
 {
diff --git a/src/fs/ufs/ufscommon.h b/src/fs/ufs/ufscommon.h
index 7561e8b965..3af061937a 100644
--- a/src/fs/ufs/ufscommon.h
+++ b/src/fs/ufs/ufscommon.h
@@ -59,6 +59,7 @@ public:
     ~UFSSwapDir();
     virtual StoreSearch *search(String const url, HttpRequest *);
     virtual bool doubleCheck(StoreEntry &);
+    virtual bool unlinkdUseful() const;
     virtual void unlink(StoreEntry &);
     virtual void statfs(StoreEntry &)const;
     virtual void maintain();
diff --git a/src/main.cc b/src/main.cc
index 21e45a24ba..64eae87481 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -872,6 +872,11 @@ mainReconfigureFinish(void *)
 
     mimeInit(Config.mimeTablePathname);
 
+#if USE_UNLINKD
+    if (unlinkdNeeded())
+        unlinkdInit();
+#endif
+
 #if USE_DELAY_POOLS
     Config.ClientDelay.finalize();
 #endif
@@ -1085,7 +1090,8 @@ mainInitialize(void)
 
     if (!configured_once) {
 #if USE_UNLINKD
-        unlinkdInit();
+        if (unlinkdNeeded())
+            unlinkdInit();
 #endif
 
         urlInitialize();
diff --git a/src/protos.h b/src/protos.h
index 46cb7b833e..00bae33f82 100644
--- a/src/protos.h
+++ b/src/protos.h
@@ -591,6 +591,7 @@ SQUIDCEXTERN void PrintRusage(void);
 SQUIDCEXTERN void dumpMallocStats(void);
 
 #if USE_UNLINKD
+SQUIDCEXTERN bool unlinkdNeeded(void);
 SQUIDCEXTERN void unlinkdInit(void);
 SQUIDCEXTERN void unlinkdClose(void);
 SQUIDCEXTERN void unlinkdUnlink(const char *);
diff --git a/src/tests/TestSwapDir.cc b/src/tests/TestSwapDir.cc
index d99d28bd7f..b650b646ce 100644
--- a/src/tests/TestSwapDir.cc
+++ b/src/tests/TestSwapDir.cc
@@ -35,6 +35,12 @@ void
 TestSwapDir::init()
 {}
 
+bool
+TestSwapDir::unlinkdUseful() const
+{
+    return false;
+}
+
 bool
 TestSwapDir::canStore(const StoreEntry &, int64_t, int &load) const
 {
diff --git a/src/tests/TestSwapDir.h b/src/tests/TestSwapDir.h
index e5d51cb870..ad6f81d63e 100644
--- a/src/tests/TestSwapDir.h
+++ b/src/tests/TestSwapDir.h
@@ -20,6 +20,7 @@ public:
 
     virtual void reconfigure();
     virtual void init();
+    virtual bool unlinkdUseful() const;
     virtual bool canStore(const StoreEntry &e, int64_t diskSpaceNeeded, int &load) const;
     virtual StoreIOState::Pointer createStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *);
     virtual StoreIOState::Pointer openStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *);
diff --git a/src/unlinkd.cc b/src/unlinkd.cc
index c46e1c20fa..09066fbb64 100644
--- a/src/unlinkd.cc
+++ b/src/unlinkd.cc
@@ -35,6 +35,7 @@
 
 #include "squid.h"
 #include "SquidTime.h"
+#include "SwapDir.h"
 #include "fde.h"
 #include "xusleep.h"
 
@@ -156,8 +157,7 @@ unlinkdClose(void)
         unlinkd_wfd = -1;
 
         unlinkd_rfd = -1;
-    } else
-        debugs(2, 0, "unlinkdClose: WARNING: unlinkd_wfd is " << unlinkd_wfd);
+    }
 
     if (hIpc) {
         if (WaitForSingleObject(hIpc, 5000) != WAIT_OBJECT_0) {
@@ -188,9 +188,25 @@ unlinkdClose(void)
 
 #endif
 
+bool
+unlinkdNeeded(void)
+{
+    // we should start unlinkd if there are any cache_dirs using it
+    for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
+        const RefCount<SwapDir> sd = Config.cacheSwap.swapDirs[i];
+        if (sd->unlinkdUseful())
+            return true;
+    }
+
+    return false;
+}
+
 void
 unlinkdInit(void)
 {
+    if (unlinkd_wfd >= 0)
+        return; // unlinkd already started
+
     const char *args[2];
     Ip::Address localhost;