From mboxrd@z Thu Jan 1 00:00:00 1970 From: Artem Bityutskiy Subject: Re: [PATCHv4 17/17] writeback: lessen sync_supers wakeup count Date: Thu, 27 May 2010 13:51:09 +0300 Message-ID: <1274957469.13159.20.camel@localhost> References: <1274795352-3551-1-git-send-email-dedekind1@gmail.com> <1274795352-3551-18-git-send-email-dedekind1@gmail.com> <20100527065041.GA31073@ZenIV.linux.org.uk> <20100527072240.GM22536@laptop> Reply-To: dedekind1@gmail.com Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Al Viro , LKML , Jens Axboe , linux-fsdevel@vger.kernel.org To: Nick Piggin Return-path: Received: from smtp.nokia.com ([192.100.122.230]:47285 "EHLO mgw-mx03.nokia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755034Ab0E0Kw6 (ORCPT ); Thu, 27 May 2010 06:52:58 -0400 In-Reply-To: <20100527072240.GM22536@laptop> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Nick, thanks for serialization suggestion. On Thu, 2010-05-27 at 17:22 +1000, Nick Piggin wrote: > Yeah, we definitely don't want to add global cacheline writes in the > common case. Also I don't know why you do the strange -1 value. I > couldn't seem to find where you defined bdi_arm_supers_timer(); It is in mm/backing-dev.c:376 in today's Linus' tree. The -1 is used to indicate that 'sync_supers()' is in progress and avoid arming timer in that case. But yes, this is not really needed. > But why doesn't this work? >=20 > sb->s_dirty =3D 1; > smp_mb(); /* corresponding MB is in test_and_clear_bit */ AFAIU, test_and_clear_bit assumes 2 barriers - before the test and afte= r the clear. Then I do not really understand why this smp_mb is needed. > if (unlikely(!supers_timer_armed)) { > if (!test_and_set_bit(0, &supers_timer_armed)) > bdi_arm_supers_timer(); > } > =20 > vs >=20 > supers_timer_armed =3D 0; > again: > sync_supers(); > if (test_and_clear_bit(0, &supers_timer_armed)) > goto again; AFAIU, the following should be fine, no?: if (unlikely(!supers_timer_armed)) if (!test_and_set_bit(0, &supers_timer_armed)) bdi_arm_supers_timer(); vs again: sync_supers(); if (test_and_clear_bit(0, &supers_timer_armed)) goto again; I assume that it is harmless to run 'bdi_arm_supers_timer()' concurrently; --=20 Best Regards, Artem Bityutskiy (=D0=90=D1=80=D1=82=D1=91=D0=BC =D0=91=D0=B8=D1=82=D1=8E= =D1=86=D0=BA=D0=B8=D0=B9) -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html