From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-f195.google.com ([209.85.167.195]:39421 "EHLO mail-oi1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388578AbfDRQFS (ORCPT ); Thu, 18 Apr 2019 12:05:18 -0400 Received: by mail-oi1-f195.google.com with SMTP id n187so2008701oih.6 for ; Thu, 18 Apr 2019 09:05:17 -0700 (PDT) MIME-Version: 1.0 References: <20190410040826.24371-1-pagupta@redhat.com> <20190410040826.24371-2-pagupta@redhat.com> <20190412083230.GA29850@quack2.suse.cz> In-Reply-To: From: Dan Williams Date: Thu, 18 Apr 2019 09:05:05 -0700 Message-ID: Subject: Re: [PATCH v5 1/6] libnvdimm: nd_region flush callback support Content-Type: text/plain; charset="UTF-8" Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Jeff Moyer Cc: Jan Kara , Pankaj Gupta , linux-nvdimm , Linux Kernel Mailing List , virtualization@lists.linux-foundation.org, KVM list , linux-fsdevel , Linux ACPI , Qemu Developers , linux-ext4 , linux-xfs , Ross Zwisler , Vishal L Verma , Dave Jiang , "Michael S. Tsirkin" , Jason Wang , Matthew Wilcox , "Rafael J. Wysocki" , Christoph Hellwig , Len Brown , Theodore Ts'o , Andreas Dilger , "Darrick J. Wong" , lcapitulino@redhat.com, Kevin Wolf , Igor Mammedov , Nitesh Narayan Lal , Rik van Riel , Stefan Hajnoczi , Andrea Arcangeli , David Hildenbrand , david , cohuck@redhat.com, Xiao Guangrong , Paolo Bonzini , kilobyte@angband.pl, yuval shaia On Fri, Apr 12, 2019 at 6:12 AM Jeff Moyer wrote: > > Jan Kara writes: > > > On Thu 11-04-19 07:51:48, Dan Williams wrote: > >> On Tue, Apr 9, 2019 at 9:09 PM Pankaj Gupta wrote: > >> > + } else { > >> > + if (nd_region->flush(nd_region)) > >> > + rc = -EIO; > >> > >> Given the common case wants to be fast and synchronous I think we > >> should try to avoid retpoline overhead by default. So something like > >> this: > >> > >> if (nd_region->flush == generic_nvdimm_flush) > >> rc = generic_nvdimm_flush(...); > > > > I'd either add a comment about avoiding retpoline overhead here or just > > make ->flush == NULL mean generic_nvdimm_flush(). Just so that people don't > > get confused by the code. > > Isn't this premature optimization? I really don't like adding things > like this without some numbers to show it's worth it. I don't think it's premature given this optimization technique is already being deployed elsewhere, see: https://lwn.net/Articles/774347/