From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sgi.com (relay1.corp.sgi.com [137.38.102.111]) by oss.sgi.com (Postfix) with ESMTP id 44A4E7CA0 for ; Mon, 25 Apr 2016 13:11:48 -0500 (CDT) Received: from cuda.sgi.com (cuda2.sgi.com [192.48.176.25]) by relay1.corp.sgi.com (Postfix) with ESMTP id 19D9B8F8033 for ; Mon, 25 Apr 2016 11:11:44 -0700 (PDT) Received: from lynxeye.de (ns.lynxeye.de [87.118.118.114]) by cuda.sgi.com with ESMTP id qot6rW0x7aRhTxNX for ; Mon, 25 Apr 2016 11:11:41 -0700 (PDT) Message-ID: <1461607897.2364.27.camel@lynxeye.de> Subject: Re: [PATCH] xfs: idle aild if the AIL is pushed up to the target LSN From: Lucas Stach Date: Mon, 25 Apr 2016 20:11:37 +0200 In-Reply-To: <20160425142444.GC33882@bfoster.bfoster> References: <1461570163-4083-1-git-send-email-dev@lynxeye.de> <20160425142444.GC33882@bfoster.bfoster> Mime-Version: 1.0 List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: xfs-bounces@oss.sgi.com Sender: xfs-bounces@oss.sgi.com To: Brian Foster Cc: linux-kernel@vger.kernel.org, xfs@oss.sgi.com QW0gTW9udGFnLCBkZW4gMjUuMDQuMjAxNiwgMTA6MjQgLTA0MDAgc2NocmllYiBCcmlhbiBGb3N0 ZXI6Cj4gT24gTW9uLCBBcHIgMjUsIDIwMTYgYXQgMDk6NDI6NDNBTSArMDIwMCwgTHVjYXMgU3Rh Y2ggd3JvdGU6Cj4gPiAKPiA+IFRoZSBjdXJyZW50IGxvZ2ljIG9ubHkgaWRsZXMgYWlsZCBpZiB0 aGUgQUlMIGlzIGVtcHR5LCByZXNjaGVkdWxpbmcKPiA+IHRoZSB3b3JrZXIgd2l0aCBhIHRpbWVv dXQgb2YgNTBtcyBvdGhlcndpc2UuIElmIHRoZSB0YXJnZXQgTFNOCj4gPiBpc24ndAo+ID4gbW92 ZWQgZm9yd2FyZCwgdGhlIHdvcmtlciB3aWxsIG5vdCBtYWtlIGFueSBwcm9ncmVzcyBhcyBpdCBv bmx5Cj4gPiBwdXNoZXMgdGhlIEFJTCB1cCB0byB0aGUgdGFyZ2V0IExTTiwgbGVhZGluZyB0byB0 aGUgZW1wdHkgQUlMCj4gPiBjb25kaXRpb24gdG8gb25seSBiZSBtZXQgYWZ0ZXIgdGhlIGxvZyB3 b3JrZXIgcHVzaGVkIHRoZSBjb21wbGV0ZQo+ID4gQUlMLiBBcyB0aGUgbG9nIHdvcmtlciBydW5z IG9uIHRoZSBzY2FsZSBvZiBtaW51dGVzLCB0aGlzIGxlYXZlcwo+ID4gYWlsZCBpbiB0aGUgIm5v IHByb2dyZXNzLCA1MG1zIHRpbWVvdXQiIHN0YXRlIGZvciBleHRlbmRlZCBwZXJpb2RzCj4gPiBv Zgo+ID4gdGltZSwgY2F1c2luZyBtYW55IHVuZWNlc3Nhcnkgd2FrZXVwcy4KPiA+IAo+IFdoZXJl IGRvIHlvdSBnZXQgdGhlICJzY2FsZSBvZiBtaW51dGVzIiBmcm9tPyBJIGJlbGlldmUgdGhlIGRl ZmF1bHQKPiBsb2cKPiB3b3JrZXIgaW50ZXJ2YWwgaXMgMzBzLCB0aG91Z2ggaXQgY2FuIGJlIGlu Y3JlYXNlZCBieSB0aGUgYWRtaW4uCj4gClVoLCByaWdodC4gSXQncyAzMHMsIHdoaWNoIGlzIHN0 aWxsIGEgdmVyeSBsb25nIHRpbWUgd2hlbiB3ZSBjb25zaWRlcgp0aGF0IG5ldyBpdGVtcyBjYW4g YmUgYWRkZWQgdG8gdGhlIEFJTCByaWdodCBhZnRlciB0aGUgbG9nIHdvcmtlcgpmb3JjZWQgaXQg b3V0LgoKPiBUaGUgb3RoZXIgdGhpbmcgdG8gbm90ZSBpcyB0aGF0IHRoZSBBSUwgdGFyZ2V0IGlz IGFsc28gZHJpdmVuIGJ5Cj4gZGVtYW5kCj4gb24gbG9nIHNwYWNlLiBFLmcuLCBzZWUgeGxvZ19n cmFudF9wdXNoX2FpbCgpLCB3aGljaCBleGVjdXRlcyB3aGVuIGEKPiB0cmFuc2FjdGlvbiBhdHRl bXB0cyB0byByZXNlcnZlIGxvZyBzcGFjZSB0aGF0IGlzbid0IGN1cnJlbnRseQo+IGF2YWlsYWJs ZQo+IGluIHRoZSBsb2cuCj4gCkFuZCB0aGF0J3Mgd2hlcmUgSSB0aGluayB0aGUgcHJvYmxlbSBz dGVtcyBmcm9tOiB0aGUgYWJvdmUgZnVuY3Rpb24KcHVzaGVzIHRoZSBBSUwgb25seSB1cCB0byBh IHRhcmdldCBMU04gdG8gbWFrZSByb29tIGZvciB0aGUgcmVxdWlyZWQKbG9nIHNwYWNlLiBPbmx5 IHRoZSBsb2cgd29ya2VyIGVtcHRpZXMgdGhlIEFJTCBjb21wbGV0ZWx5IGJ5IGlzc3VpbmcgYQp4 ZnNfYWlsX3B1c2hfYWxsX3N5bmMoKS4KCj4gVGhhdCBzYWlkLCBJJ20gbm90IHN1cmUgd2hldGhl ciB0aGVyZSdzIGEgbm90YWJsZSBiZW5lZml0IG9mIGlkbGluZwo+IGZvcgo+IDUwbXMgb3ZlciBq dXN0IHNjaGVkdWxpbmcgb3V0IHdoZW4gd2UndmUgaGl0IHRoZSB0YXJnZXQgbHNuLiBJdCBzZWVt cwo+IGxpa2UgdGhhdCBhbnlib2R5IHdobyBwdXNoZXMgdGhlIHRhcmdldCBmb3J3YXJkIGFnYWlu IGlzIGdvaW5nIHRvCj4gd2FrZQo+IHVwIHRoZSB0aHJlYWQgYW55d2F5cy4gT24gdGhlIG90aGVy IGhhbmQsIGlmIHRoZSBmcyBpcyBpZGxlIHRoZQo+IHRocmVhZAo+IHdpbGwgZXZlbnR1YWxseSBz Y2hlZHVsZSBvdXQgaW5kZWZpbml0ZWx5LiAKCklzIHRoaXMgYSBwcm9ibGVtPyBUaGUgcGF0Y2gg dHJpZXMgdG8gZG8gZXhhY3RseSB0aGF0OiBzY2hlZHVsZSBvdXQKYWlsZCBpbmRlZmluaXRlbHkg d2hlbiB0aGVyZSBpcyBubyBtb3JlIHdvcmsgdG8gZG8gYXMgbm9ib2R5IGlzIHB1c2hpbmcKdGhl IHRhcmdldCBMU04gZm9yd2FyZC4KCk9uIGFuIGlkbGUgRlMgdGhpcyB3aWxsIHN0b3AgYWlsZCB3 aGVuIHRoZSB0YXJnZXQgTFNOIGlzIGhpdCwgd2lsbCB3YWtlCml0IHVwIGFnYWluIGFzIHNvb24g YXMgdGhlIGxvZyB3b3JrZXIgcmVxdWVzdHMgdGhlIGNvbXBsZXRlIEFJTCB0byBiZQpwdXNoZWQg b3V0LCB0aGVuIGl0IGdvZXMgYmFjayB0byBzbGVlcCBpbmRlZmluaXRlbHkgdW50aWwgdGhlIGxv ZyBmaWxscwp1cCBhZ2Fpbi4KCj4gSGF2ZSB5b3Ugb2JzZXJ2ZWQgcHJvYmxlbWF0aWMKPiBiZWhh dmlvciBkdWUgdG8gdGhlIGN1cnJlbnQgaGV1cmlzdGljPwo+IApZZXMuIE9uIGEgRlMgd2l0aCBh IGxvdywgYnV0IGNvbnN0YW50IG1ldGFkYXRhIHdvcmtsb2FkIEkgd291bGQgZXhwZWN0CmFpbGQg dG8gYmUgc2NoZWR1bGVkIG9ubHkgd2hlbiB0aGUgbG9nIHNwYWNlIGlzIHJ1bm5pbmcgb3V0IGFu ZC9vciB0aGUKbG9nIHdvcmtlciBmb3JjZXMgYSBzeW5jLgpXaGF0IGN1cnJlbnRseSBoYXBwZW5z IGlzIHRoYXQgYWlsZCBzdGF5cyBpbiB0aGUgInRhcmdldCBMU04gaGl0IC0+IG5vCndvcmsgdG8g ZG8sIHNjaGVkdWxlIGEgdGltZW91dCBlYWNoIDUwbXMiIHN0YXRlIGZvciBhbG1vc3QgdGhlIGVu dGlyZQpydW50aW1lIG9mIHRoZSBzeXN0ZW0uIFRoaXMgY2F1c2VzIGFib3V0IDIwIENQVSB3YWtl dXBzIHBlciBzZWNvbmQgcGVyCm1vdW50cG9pbnQsIG1vc3Qgb2Ygd2hpY2ggYXJlIGNvbXBsZXRl bHkgd2FzdGVkLCBhcyB0aGVyZSBpcyBubyB3b3JrIHRvCmRvLgoKTXkgdXNlIGNhc2UgbWF5IGJl IGF0eXBpY2FsLCBhcyBJJ20gdXNpbmcgWEZTIG9uIGEgbGFwdG9wLCBidXQgaW4gdGhhdApjYXNl IHRoZSBYRlMgYWlsZCBpcyBhY3R1YWxseSBjYXVzaW5nIG1vc3Qgb2YgdGhlIENQVSB3YWtldXBz IGluIGFuCm90aGVyd2lzZSBpZGxlIHN5c3RlbSwgd2hpY2ggaXMgY2xlYXJseSB1bmRlc2lyYWJs ZSwgYXMgdGhvc2Ugd2FrZXVwcwphcmVuJ3QgcmVhbGx5IG5lZWRlZCBhcyB0aGV5IGRvbid0IGxl YWQgdG8gYWN0dWFsIHdvcmsgZ2V0dGluZyBkb25lLgoKPiA+IAo+ID4gRml4IHRoaXMgYnkgaWRs aW5nIGFpbGQgYXMgc29vbiBhcyB0aGUgQUlMIGlzIHB1c2hlZCB1cCB0byB0aGUKPiA+IHRhcmdl dAo+ID4gTFNOLiBBbGwgY29kZSBwYXRocyB0aGF0IG1vdmUgdGhlIHRhcmdldCBMU04gZm9yd2Fy ZCBhbHJlYWR5IGVuc3VyZQo+ID4gdGhhdCBhaWxkIGlzIHdva2VuIHVwIGFnYWluIHdoZW4gbmVl ZGVkLgo+ID4gCj4gPiBTaWduZWQtb2ZmLWJ5OiBMdWNhcyBTdGFjaCA8ZGV2QGx5bnhleWUuZGU+ Cj4gPiAtLS0KPiA+IEknbSBwcmV0dHkgc3VyZSB0aGF0IHRoZSBhYm92ZSBkZWR1Y3Rpb24gaXMg Y29ycmVjdCwgYnV0IGFzIHRoaXMgbXkKPiA+IGZpcnN0IHJlYWwgZW5jb3VudGVyIHdpdGggdGhl IFhGUyBjb2RlIGJhc2UsIHNvbWVvbmUgd2l0aCBhIGJpdAo+ID4gbW9yZQo+ID4ga25vd2xlZGdl IHRoYW4gbWUgc2hvdWxkIGdpdmUgdGhpcyBzb21lIHRob3VnaHQuCj4gPiAtLS0KPiA+IMKgZnMv eGZzL3hmc190cmFuc19haWwuYyB8IDcgKysrKystLQo+ID4gwqAxIGZpbGUgY2hhbmdlZCwgNSBp bnNlcnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQo+ID4gCj4gPiBkaWZmIC0tZ2l0IGEvZnMveGZz L3hmc190cmFuc19haWwuYyBiL2ZzL3hmcy94ZnNfdHJhbnNfYWlsLmMKPiA+IGluZGV4IGQ2Yzlj M2UuLjdlY2E4NTIgMTAwNjQ0Cj4gPiAtLS0gYS9mcy94ZnMveGZzX3RyYW5zX2FpbC5jCj4gPiAr KysgYi9mcy94ZnMveGZzX3RyYW5zX2FpbC5jCj4gPiBAQCAtNDk1LDYgKzQ5NSw3IEBAIHhmc2Fp bGQoCj4gPiDCoHsKPiA+IMKgCXN0cnVjdCB4ZnNfYWlsCSphaWxwID0gZGF0YTsKPiA+IMKgCWxv bmcJCXRvdXQgPSAwOwkvKiBtaWxsaXNlY29uZHMgKi8KPiA+ICsJc3RydWN0IHhmc19sb2dfaXRl bSAqbGlwOwo+ID4gwqAKPiA+IMKgCWN1cnJlbnQtPmZsYWdzIHw9IFBGX01FTUFMTE9DOwo+ID4g wqAJc2V0X2ZyZWV6YWJsZSgpOwo+ID4gQEAgLTUwOCw3ICs1MDksOCBAQCB4ZnNhaWxkKAo+ID4g wqAJCXNwaW5fbG9jaygmYWlscC0+eGFfbG9jayk7Cj4gPiDCoAo+ID4gwqAJCS8qCj4gPiAtCQnC oCogSWRsZSBpZiB0aGUgQUlMIGlzIGVtcHR5IGFuZCB3ZSBhcmUgbm90IHJhY2luZwo+ID4gd2l0 aCBhIHRhcmdldAo+ID4gKwkJwqAqIElkbGUgaWYgdGhlIEFJTCBpcyBlbXB0eSBvciBwdXNoZWQg dXAgdG8gdGhlCj4gPiByZXF1ZXN0ZWQKPiA+ICsJCcKgKiB0YXJnZXQgTFNOIGFuZCB3ZSBhcmUg bm90IHJhY2luZyB3aXRoIGEgdGFyZ2V0Cj4gPiDCoAkJwqAqIHVwZGF0ZS4gV2UgY2hlY2sgdGhl IEFJTCBhZnRlciB3ZSBzZXQgdGhlIHRhc2sKPiA+IHRvIGEgc2xlZXAKPiA+IMKgCQnCoCogc3Rh dGUgdG8gZ3VhcmFudGVlIHRoYXQgd2UgZWl0aGVyIGNhdGNoIGFuCj4gPiB4YV90YXJnZXQgdXBk YXRlCj4gPiDCoAkJwqAqIG9yIHRoYXQgYSB3YWtlX3VwIHJlc2V0cyB0aGUgc3RhdGUgdG8KPiA+ IFRBU0tfUlVOTklORy4KPiA+IEBAIC01MTcsNyArNTE5LDggQEAgeGZzYWlsZCgKPiA+IMKgCQnC oCogVGhlIGJhcnJpZXIgbWF0Y2hlcyB0aGUgeGFfdGFyZ2V0IHVwZGF0ZSBpbgo+ID4geGZzX2Fp bF9wdXNoKCkuCj4gPiDCoAkJwqAqLwo+ID4gwqAJCXNtcF9ybWIoKTsKPiA+IC0JCWlmICgheGZz X2FpbF9taW4oYWlscCkgJiYKPiA+ICsJCWxpcCA9IHhmc19haWxfbWluKGFpbHApOwo+ID4gKwkJ aWYgKCghbGlwIHx8IFhGU19MU05fQ01QKGxpcC0+bGlfbHNuLCBhaWxwLQo+ID4gPnhhX3Rhcmdl dCkgPj0gMCkgJiYKPiBJIHRoaW5rIHlvdSdkIHdhbnQgdG8gdXNlID4gMCwgYXMgPT0gMCBzdWdn ZXN0cyB5b3UgaGF2ZSBhbiBpdGVtIHRoYXQKPiBtYXRjaGVzIHRoZSB0YXJnZXQuCj4gClRoaXMg Y2hlY2sganVzdCBtaXJyb3JzIHdoYXQgaXMgZG9uZSBpbsKgeGZzYWlsZF9wdXNoKCkgdG8gY2hl Y2sgaWYgdGhlCnRhcmdldCBoYXMgYmVlbiBoaXQuIEVpdGhlciBib3RoIHNpdGVzIHNob3VsZCBi ZSBjaGFuZ2VkICh3aGljaCB3b3VsZAphc3N1bWUgdGhlIHRhcmdldCBMU04gaXMgaW5jbHVzaXZl IGFuZCB3ZSBzaG91bGQgcHVzaCB0aGUgQUlMIHRvIGF0CmxlYXN0IHRoZSB0YXJnZXQpIG9yIGxl ZnQgYXMgaXMgKGFzc3VtaW5nIHRoZSB0YXJnZXQgTFNOKSBpcyBleGNsdXNpdmUKYW5kIHdlIHNo b3VsZCBwdXNoIHVwIHRvIHRoZSB0YXJnZXQpLgoKPiBGV0lXLCBhbm90aGVyIG9wdGlvbiBtaWdo dCBiZSB0byBpbmNyZWFzZSB0aGUgdGltZW91dCB0byBzb21ldGhpbmcKPiBtdWNoCj4gbGFyZ2Vy IHRoYW4gdGhlIGN1cnJlbnQgNTBtcyB3aGVuIHdlJ3ZlIGhpdCB0aGUgdGFyZ2V0LgoKSSBkb24n dCB0aGluayB3ZSB3YW50IHRvIGV4dGVuZCB0aGUgdGltZW91dCBpZiBzb21lb25lIGFscmVhZHkg cHVzaGVkCnRoZSB0YXJnZXQgTFNOIGZvcndhcmQsIHNvIHdlIHdvdWxkIG5lZWQgdG8gbWlycm9y IHRoZSBjaGVjayBmb3IgdGhlCnRhcmdldCBMU04gcmFjZSBoZXJlLiBJIGJlbGlldmUgbXkgY2hh bmdlIGlzIGxlc3MgaW50cnVzaXZlLgoKUmVnYXJkcywKTHVjYXMKCj4gIFNlZSB0b3dhcmRzIHRo ZQo+IGVuZCBvZiB4ZnNhaWxkX3B1c2goKSB3aGVyZSB0b3V0IGlzIGRldGVybWluZWQgYmFzZWQg b24gdGhlIHN0YXRlIG9mCj4gdGhlCj4gcHVzaCAoYW5kIHdlIGFscmVhZHkgaGF2ZSBsb2dpYyB0 byBjaGVjayBhZ2FpbnN0IHhhX3RhcmdldCkuCj4gCj4gQnJpYW4KPiAKPiA+IAo+ID4gwqAJCcKg wqDCoMKgYWlscC0+eGFfdGFyZ2V0ID09IGFpbHAtPnhhX3RhcmdldF9wcmV2KSB7Cj4gPiDCoAkJ CXNwaW5fdW5sb2NrKCZhaWxwLT54YV9sb2NrKTsKPiA+IMKgCQkJZnJlZXphYmxlX3NjaGVkdWxl KCk7CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwp4ZnMg bWFpbGluZyBsaXN0Cnhmc0Bvc3Muc2dpLmNvbQpodHRwOi8vb3NzLnNnaS5jb20vbWFpbG1hbi9s aXN0aW5mby94ZnMK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933355AbcDYSLp (ORCPT ); Mon, 25 Apr 2016 14:11:45 -0400 Received: from ns.lynxeye.de ([87.118.118.114]:58726 "EHLO lynxeye.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932905AbcDYSLo (ORCPT ); Mon, 25 Apr 2016 14:11:44 -0400 Message-ID: <1461607897.2364.27.camel@lynxeye.de> Subject: Re: [PATCH] xfs: idle aild if the AIL is pushed up to the target LSN From: Lucas Stach To: Brian Foster Cc: Dave Chinner , linux-kernel@vger.kernel.org, xfs@oss.sgi.com Date: Mon, 25 Apr 2016 20:11:37 +0200 In-Reply-To: <20160425142444.GC33882@bfoster.bfoster> References: <1461570163-4083-1-git-send-email-dev@lynxeye.de> <20160425142444.GC33882@bfoster.bfoster> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.2 (3.18.5.2-1.fc23) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am Montag, den 25.04.2016, 10:24 -0400 schrieb Brian Foster: > On Mon, Apr 25, 2016 at 09:42:43AM +0200, Lucas Stach wrote: > > > > The current logic only idles aild if the AIL is empty, rescheduling > > the worker with a timeout of 50ms otherwise. If the target LSN > > isn't > > moved forward, the worker will not make any progress as it only > > pushes the AIL up to the target LSN, leading to the empty AIL > > condition to only be met after the log worker pushed the complete > > AIL. As the log worker runs on the scale of minutes, this leaves > > aild in the "no progress, 50ms timeout" state for extended periods > > of > > time, causing many unecessary wakeups. > > > Where do you get the "scale of minutes" from? I believe the default > log > worker interval is 30s, though it can be increased by the admin. > Uh, right. It's 30s, which is still a very long time when we consider that new items can be added to the AIL right after the log worker forced it out. > The other thing to note is that the AIL target is also driven by > demand > on log space. E.g., see xlog_grant_push_ail(), which executes when a > transaction attempts to reserve log space that isn't currently > available > in the log. > And that's where I think the problem stems from: the above function pushes the AIL only up to a target LSN to make room for the required log space. Only the log worker empties the AIL completely by issuing a xfs_ail_push_all_sync(). > That said, I'm not sure whether there's a notable benefit of idling > for > 50ms over just scheduling out when we've hit the target lsn. It seems > like that anybody who pushes the target forward again is going to > wake > up the thread anyways. On the other hand, if the fs is idle the > thread > will eventually schedule out indefinitely. Is this a problem? The patch tries to do exactly that: schedule out aild indefinitely when there is no more work to do as nobody is pushing the target LSN forward. On an idle FS this will stop aild when the target LSN is hit, will wake it up again as soon as the log worker requests the complete AIL to be pushed out, then it goes back to sleep indefinitely until the log fills up again. > Have you observed problematic > behavior due to the current heuristic? > Yes. On a FS with a low, but constant metadata workload I would expect aild to be scheduled only when the log space is running out and/or the log worker forces a sync. What currently happens is that aild stays in the "target LSN hit -> no work to do, schedule a timeout each 50ms" state for almost the entire runtime of the system. This causes about 20 CPU wakeups per second per mountpoint, most of which are completely wasted, as there is no work to do. My use case may be atypical, as I'm using XFS on a laptop, but in that case the XFS aild is actually causing most of the CPU wakeups in an otherwise idle system, which is clearly undesirable, as those wakeups aren't really needed as they don't lead to actual work getting done. > > > > Fix this by idling aild as soon as the AIL is pushed up to the > > target > > LSN. All code paths that move the target LSN forward already ensure > > that aild is woken up again when needed. > > > > Signed-off-by: Lucas Stach > > --- > > I'm pretty sure that the above deduction is correct, but as this my > > first real encounter with the XFS code base, someone with a bit > > more > > knowledge than me should give this some thought. > > --- > >  fs/xfs/xfs_trans_ail.c | 7 +++++-- > >  1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c > > index d6c9c3e..7eca852 100644 > > --- a/fs/xfs/xfs_trans_ail.c > > +++ b/fs/xfs/xfs_trans_ail.c > > @@ -495,6 +495,7 @@ xfsaild( > >  { > >   struct xfs_ail *ailp = data; > >   long tout = 0; /* milliseconds */ > > + struct xfs_log_item *lip; > >   > >   current->flags |= PF_MEMALLOC; > >   set_freezable(); > > @@ -508,7 +509,8 @@ xfsaild( > >   spin_lock(&ailp->xa_lock); > >   > >   /* > > -  * Idle if the AIL is empty and we are not racing > > with a target > > +  * Idle if the AIL is empty or pushed up to the > > requested > > +  * target LSN and we are not racing with a target > >    * update. We check the AIL after we set the task > > to a sleep > >    * state to guarantee that we either catch an > > xa_target update > >    * or that a wake_up resets the state to > > TASK_RUNNING. > > @@ -517,7 +519,8 @@ xfsaild( > >    * The barrier matches the xa_target update in > > xfs_ail_push(). > >    */ > >   smp_rmb(); > > - if (!xfs_ail_min(ailp) && > > + lip = xfs_ail_min(ailp); > > + if ((!lip || XFS_LSN_CMP(lip->li_lsn, ailp- > > >xa_target) >= 0) && > I think you'd want to use > 0, as == 0 suggests you have an item that > matches the target. > This check just mirrors what is done in xfsaild_push() to check if the target has been hit. Either both sites should be changed (which would assume the target LSN is inclusive and we should push the AIL to at least the target) or left as is (assuming the target LSN) is exclusive and we should push up to the target). > FWIW, another option might be to increase the timeout to something > much > larger than the current 50ms when we've hit the target. I don't think we want to extend the timeout if someone already pushed the target LSN forward, so we would need to mirror the check for the target LSN race here. I believe my change is less intrusive. Regards, Lucas > See towards the > end of xfsaild_push() where tout is determined based on the state of > the > push (and we already have logic to check against xa_target). > > Brian > > > > >       ailp->xa_target == ailp->xa_target_prev) { > >   spin_unlock(&ailp->xa_lock); > >   freezable_schedule();