From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8C96BC433E1 for ; Thu, 20 Aug 2020 19:04:49 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 245012075E for ; Thu, 20 Aug 2020 19:04:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="cT3s3iCA" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 245012075E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6DAFA6B00AB; Thu, 20 Aug 2020 15:04:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 68C926B00AC; Thu, 20 Aug 2020 15:04:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 57A828D0005; Thu, 20 Aug 2020 15:04:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0220.hostedemail.com [216.40.44.220]) by kanga.kvack.org (Postfix) with ESMTP id 3F4C56B00AB for ; Thu, 20 Aug 2020 15:04:48 -0400 (EDT) Received: from smtpin06.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id DD689181AEF07 for ; Thu, 20 Aug 2020 19:04:47 +0000 (UTC) X-FDA: 77171873814.06.sail46_0e11ed627033 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin06.hostedemail.com (Postfix) with ESMTP id A8DB7100480A8 for ; Thu, 20 Aug 2020 19:04:47 +0000 (UTC) X-HE-Tag: sail46_0e11ed627033 X-Filterd-Recvd-Size: 4835 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf12.hostedemail.com (Postfix) with ESMTP for ; Thu, 20 Aug 2020 19:04:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=oA8uW7xzETbBJpktLvfoWrpUHRp3r6xKbF8SQ19fA74=; b=cT3s3iCAC3O1Z0gjwp8wfy0/UJ 0IlYiuYJqtSKiuOtkFuqtSTJftpvNc0G3FNGHl8++L/N+AgGgrPnmGv6ufwuCy9feDahFCESKaDBp c6l19FIZp0icjVgWPF39E5cmYa2SGBM4saE85Ea4yh99q2kEgazuqHe5BypI3B0h5xvLmIjl3BeL3 zR56Ns2uW9KWR/5tiBoz8mX+MpVMcLe1kkIsrvp9INELNG6xpJfBZ5zeTVFihCnWK4vIxo6WCI15A k2H/LxikQ/83B0Mkd03UggIezRV9DnBaOc8EzNDNw+Q3LxZ/4PRqqogqr7dp1Uotkb+mLPI09xCA/ M3k+d14w==; Received: from willy by casper.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8prg-0003MN-B9; Thu, 20 Aug 2020 19:04:44 +0000 Date: Thu, 20 Aug 2020 20:04:44 +0100 From: Matthew Wilcox To: Mike Rapoport Cc: linux-mm@kvack.org, Andrew Morton , Hugh Dickins , William Kucharski , Johannes Weiner , Jan Kara , linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/7] mm: Rewrite shmem_seek_hole_data Message-ID: <20200820190444.GN17456@casper.infradead.org> References: <20200819150555.31669-1-willy@infradead.org> <20200819150555.31669-3-willy@infradead.org> <20200820164546.GD752365@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200820164546.GD752365@kernel.org> X-Rspamd-Queue-Id: A8DB7100480A8 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam03 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Aug 20, 2020 at 07:45:46PM +0300, Mike Rapoport wrote: > > - * llseek SEEK_DATA or SEEK_HOLE through the page cache. > > + * llseek SEEK_DATA or SEEK_HOLE through the page cache. We don't need > > + * to get a reference on the page because this interface is racy anyway. > > + * The page we find will have had the state at some point. > > For my non-native ear "will have had" is too complex ;-) Fair! How about simply "The page we find did have the state at some point". But now I'm thinking about it some more, and I'm not so sure of it now. What if we put a page in the cache that was !Uptodate, then we do a lookup, find this pointer, then the page is removed from the cache, then it's allocated somewhere else, marked Uptodate, and now we're scheduled back in and find it's Uptodate, when there was never a page at this location that was Uptodate? So maybe I have to retract this patch after all. > > */ > > static pgoff_t shmem_seek_hole_data(struct address_space *mapping, > > pgoff_t index, pgoff_t end, int whence) > > { > > + XA_STATE(xas, &mapping->i_pages, index); > > struct page *page; > > - struct pagevec pvec; > > - pgoff_t indices[PAGEVEC_SIZE]; > > - bool done = false; > > - int i; > > > > - pagevec_init(&pvec); > > - pvec.nr = 1; /* start small: we may be there already */ > > - while (!done) { > > - pvec.nr = find_get_entries(mapping, index, > > - pvec.nr, pvec.pages, indices); > > - if (!pvec.nr) { > > - if (whence == SEEK_DATA) > > - index = end; > > - break; > > + rcu_read_lock(); > > + if (whence == SEEK_DATA) { > > + for (;;) { > > + page = xas_find(&xas, end); > > + if (xas_retry(&xas, page)) > > + continue; > > + if (!page || xa_is_value(page) || PageUptodate(page)) > > + break; > > } > > - for (i = 0; i < pvec.nr; i++, index++) { > > - if (index < indices[i]) { > > - if (whence == SEEK_HOLE) { > > - done = true; > > - break; > > - } > > - index = indices[i]; > > - } > > - page = pvec.pages[i]; > > - if (page && !xa_is_value(page)) { > > - if (!PageUptodate(page)) > > - page = NULL; > > - } > > - if (index >= end || > > - (page && whence == SEEK_DATA) || > > - (!page && whence == SEEK_HOLE)) { > > - done = true; > > + } else /* SEEK_HOLE */ { > > + for (;;) { > > + page = xas_next(&xas); > > + if (xas_retry(&xas, page)) > > + continue; > > + if (!xa_is_value(page) && > > + (!page || !PageUptodate(page))) > > + break; > > + if (xas.xa_index >= end) > > break; > > - } > > } > > - pagevec_remove_exceptionals(&pvec); > > - pagevec_release(&pvec); > > - pvec.nr = PAGEVEC_SIZE; > > - cond_resched(); > > } > > - return index; > > + rcu_read_unlock(); > > + > > + return xas.xa_index; > > } > > > > static loff_t shmem_file_llseek(struct file *file, loff_t offset, int whence) > > -- > > 2.28.0 > > > > > > -- > Sincerely yours, > Mike.