Re: [PATCH] Bug 3150: do not start useless unlinkd.

On 07/10/11 18:53, Dmitry Kurochkin wrote:
>
> Unlinkd is used only by UFS storage. Bug before the change, Squid
> always started unlinkd if it is built, even if it is not needed.
> Moreover, unlinkd was started in non-daemon mode.
>
> The patch adds unlinks() method for SwapDir to determine if it uses
> unlinkd. Unlinkd is started iff:
>
> * Squid is running in daemon mode
> * a configured cache_dir uses unlinkd
>
> 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.
> ---
> src/SwapDir.h | 2 ++
> src/fs/ufs/store_dir_ufs.cc | 6 ++++++
> src/fs/ufs/ufscommon.h | 1 +
> src/main.cc | 3 ++-
> src/protos.h | 1 +
> src/unlinkd.cc | 21 +++++++++++++++++++--
> 6 files changed, 31 insertions(+), 3 deletions(-)
>

There are a few important details overlooked in this patch.

The use of unlinkd is a property of cache_dir DiskIO strategy, not the
SwapDir type itself. Definitely not the use of -N option on startup.
   The disabling of unlinkd on -N will cause problems on all systems
needing ufs or diskd in that mode. Meaning all BSD default configs, and
those OS using upstart (Mac, Debian, and derivatives) which utilizes -N
mode to get direct control of a single worker process.

If unlindNeeded() intention was to see that unlinkd was always enabled
for mmapped and IPC DiskIO methods unless daemon mode was used, then it
should be inverted to identify the true case and placed after the for loop.

But, I think the better way to cover that is to move the unlinks() T/F
decision down to the particular DiskIO strategy and have SwapDir check
relay the answer from there.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.15
   Beta testers wanted for 3.2.0.12