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 96e11f4cb985c2ebb157584997c903d6a63bd043..7a946375f8560db57980cee50158a0e5c77b4c2c 100644 (file)
--- 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 9d461d0b7e65dac92b2588ea5991c344dff19b37..c3c7d34e78b16384cb02aac7d196b0e911e2b4fe 100644 (file)
--- 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 9a619734ef8c4695570ac71bdbcc4080fd0f5fb5..86100375de52f246f93e9ec1220b26e2cbac428b 100644 (file)
--- 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 5dea6e2a8e36944c0c0785304a81ddbead832a3b..8181dd1b4a1669fcec96daa38c2c816e637be146 100644 (file)
--- 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 6d054e3056c9a21876538dd5168fd091566e6352..64c70128c5897a932ad92c1e33395afe9e40ef37 100644 (file)
--- 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 161d7edde722dc94bdcbea76b2c31d13da544a2a..650400e9223b01bb9c7f0408843077004c6d65d9 100644 (file)
--- 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 2ec561714db86bf767f99cc30ca7117eea08688d..91990d2e46ee3d5c5ff32a60a35992213af066b1 100644 (file)
--- 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 e7c0fd9a428be2f46121b80f704172c238f734bb..1ba1ecf7665aeaf099e21a38ccc827ad1f4c10f9 100644 (file)
--- 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 12da07692ae0afe76a6d1dd4be3a0c7aeea4383a..d7e54b1db3c33ce04d36534b7aaff131e649d7b0 100644 (file)
--- 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 10a165e0b6cb9fda82e318a094465967d11ea3aa..dc3a191ed83fa9702dd324300b615dc5cf88e0ee 100644 (file)
--- 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 3bd2945bbcb9887b7885f2cac08c19596e807c6c..447e8e34b106eca3f1c573131e7e0ceab424d3f9 100644 (file)
--- 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 6f015905b6bbfce0b401027f5844fc7c9848264d..3f6d68184a9e34ea6e4a859cf8069cda3ecf4dd1 100644 (file)
--- 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 c29542726952c4e7ec7359b2c7cbb5d46eaa3c48..2d17524bb4202d618b5154692983b55cb06941b0 100644 (file)
--- 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 a1c54cac20f4125cfe8f2b29173ddd5227df4d3f..8247143775614cd651708f515bad6228737da9d3 100644 (file)
--- 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 6929d81d7769e3c753ba7210b4e995ceb22d87c2..8279d27c49d9ec3e2d693f0f4bf3df5bdd34c16f 100644 (file)
--- 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 bb4c09db4e4e338b8ca6fa3cc686f96949c7a2da..8019d13ccacacde7378cb78509bef6c0f216ac33 100644 (file)
--- 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 77b62b6dcfd60334228b500ea95c1b56b408b437..ab56abdf6b4285d44ab1b3a336a6ad3c8a47d505 100644 (file)
--- 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 0fbcada4ff48e775fb0443a42cb701a6383eb187..f427c57fff42e8a4765a2c3c9f6ee9fab57d86c7 100644 (file)
--- 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 129629c97f05b7704b34f4586e23f5e50b31afc7..7ee1df8cf552abed658b78de0b9ce51a608ca2b8 100644 (file)
--- 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 7561e8b9657d9259c2361fc9764cd79b1c64cbc6..3af061937a574cba31207db2839f3a5507329cda 100644 (file)
--- 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 21e45a24ba478debd41637dc2d57acf151673720..64eae87481e78ab9a2bf4dc9826779214698195f 100644 (file)
--- 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 46cb7b833ee7fd7b342c0efd0d6814064a471299..00bae33f82fa7f99976eec082568836b1c5bb89e 100644 (file)
--- 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 d99d28bd7f910efcec4bd786d967831e3c65c82b..b650b646ce2eeda3915e521a97f4b26d4c9e3e08 100644 (file)
--- 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 e5d51cb87016d62168dca37feb8bc8d9e564d86b..ad6f81d63ed9f3320d0be8cdaa6713341965172b 100644 (file)
--- 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 c46e1c20fa9a5aa69761ee0c4fcab050585d7fcc..09066fbb6466f528bbeb83aeca039354aafce8cf 100644 (file)
--- 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;