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

On 10/26/2011 06:59 PM, Amos Jeffries wrote:
> 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.

Committed to trunk as r11826.

Thank you,

Alex.
Received on Thu Oct 27 2011 - 23:17:26 MDT