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

 On Thu, 27 Oct 2011 01:59:41 +0400, Dmitry Kurochkin wrote:
> On Thu, 27 Oct 2011 10:34:34 +1300, Amos Jeffries
> <squid3_at_treenet.co.nz> wrote:
>> On Thu, 27 Oct 2011 01:25:59 +0400, Dmitry Kurochkin wrote:
>> > Hi all.
>> >
>> > There has been no discussion on this thread for a while. The
>> latest
>> > patch version (attached) fixes all the issues from Amos and Alex I
>> > know
>> > of. So I would like to get an approval for the patch or hints at
>> > what
>> > else needs to be fixed.
>> >
>> > The patch does not change the way unlinkd is started. But after
>> the
>> > changes, unlinkd is started iff it is needed. Whether unlinkd is
>> > needed
>> > is determined by both cache dir and DiskIO strategy. Unlinkd may
>> be
>> > started during reconfiguration if a cache dir that needs it was
>> added
>> > or
>> > DiskIO strategy changed.
>> >
>> > There was discussion about starting unlinkd on demand. But the
>> > consensus seems to be that it Squid needs substantial changes to
>> > minimize fork() performance penalty. So while we want to
>> implement
>> > this
>> > long term, for now we should start unlinkd on startup and
>> > reconfiguration.
>> >
>> > Please let me know if I missed any comments or issues with the
>> patch.
>> >
>> > Regards,
>> > Dmitry
>>
>>
>> This seems fine to go in for the bug fix. The rest was us going off
>> on
>> a tangent.
>>
>> The only worry I have left is the dependency on IamWorkerProcess()
>> in
>> src/fs/ufs/store_dir_ufs.cc.
>>
>> If you have checked that SMP with Diskers is not broken by that
>> check
>> then this is fine to go in now.
>>
>
> Yes. IamWorkerProcess() prevents UFS from starting unlinkd in
> coordinator, master and disker processes (though in non-daemon mode,
> master process is worker and). This is what we want.
>

 Then I'm happy for it to go in now.

 +1.

 Amos
Received on Thu Oct 27 2011 - 01:00:03 MDT