From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linuxfoundation.org ([140.211.169.12]:40108 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933013AbeFTXpM (ORCPT ); Wed, 20 Jun 2018 19:45:12 -0400 Date: Wed, 20 Jun 2018 16:45:11 -0700 From: Andrew Morton To: Jens Axboe Cc: linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk Subject: Re: [PATCHSET v3 0/4] Submit ->readpages() IO as read-ahead Message-Id: <20180620164511.5bda5d9d37123af787b6f5c4@linux-foundation.org> In-Reply-To: <96d97ad3-5dfb-7f45-6056-7de918e622ad@kernel.dk> References: <1527691329-2371-1-git-send-email-axboe@kernel.dk> <20180619165640.0c36473a33e20ea6c0fda463@linux-foundation.org> <9b1d2225-104d-865f-d821-8670271d6d4a@kernel.dk> <20180620125801.6e563efc5ed6af2f12c27f07@linux-foundation.org> <20180620162353.58d931d64ebf7400a259030e@linux-foundation.org> <96d97ad3-5dfb-7f45-6056-7de918e622ad@kernel.dk> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, 20 Jun 2018 17:33:47 -0600 Jens Axboe wrote: > > If we are to remove callers' ability to use readpages for non-readahead > > purposes we should rename the address_space field. > > Totally agree, and I'd be happy to do that. So how about we do that? I > rename it to ->readahead (or ->readaheadpages()?), and then it's > perfectly clear what is going on. I'm not sure I have the heart to recommend that. akpm3:/usr/src/linux-4.18-rc1> grep -r readpages .|wc -l 233 Really, every damn one will need an edit. I'd understand if we left it at ->readpages and sprinkled some loud comments around the place. > >> Besides, this isn't going to be just about tracing. Yes, it'll be > >> awesome to actually get the right information from blktrace, since right > >> now nobody knows which parts are read-ahead and which ones are explicit > >> reads. Might be pretty darn useful to debug read-ahead issues. > >> > >> The read-ahead information must be reliable, otherwise it's impossible > >> to introduce other dependencies on top of that. We're having a lot of > >> issues with termination of tasks that are stuck in issuing read-ahead. > >> If we can rely on the read-ahead information being there AND being > >> correct, then we can terminate reads early. This might not sound like > >> huge win, but we're talking tens of minutes for some cases. That's the > >> planned functional change behind this, but can't be done before we make > >> progress on the topic at hand. > > > > Changelog material right there. > > I deliberately didn't want to include that, since it'd muddy the waters > on what is a 100% standalone change. Omitting the info muddied the waters! Shrug, I give up. "readpages is really only for readahead" can become another kernel wart, I guess. Anyone who wants that capability in the future can sit there looping on ->readpage.