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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id DB70CC433EF for ; Mon, 10 Jan 2022 12:36:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 408FD6B007B; Mon, 10 Jan 2022 07:36:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3928F6B007D; Mon, 10 Jan 2022 07:36:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2592F6B0080; Mon, 10 Jan 2022 07:36:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0252.hostedemail.com [216.40.44.252]) by kanga.kvack.org (Postfix) with ESMTP id 135FC6B007B for ; Mon, 10 Jan 2022 07:36:16 -0500 (EST) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id C9C269526A for ; Mon, 10 Jan 2022 12:36:15 +0000 (UTC) X-FDA: 79014325110.16.A93512F Received: from mail-ua1-f53.google.com (mail-ua1-f53.google.com [209.85.222.53]) by imf23.hostedemail.com (Postfix) with ESMTP id 0AA2614000C for ; Mon, 10 Jan 2022 12:36:14 +0000 (UTC) Received: by mail-ua1-f53.google.com with SMTP id o1so23090169uap.4 for ; Mon, 10 Jan 2022 04:36:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=qFOfaEvokfZV1EFTfIDgkQYCii1N7VmjLFzadFiYr08=; b=OWQBLpWCvBUPWP+FHu4XAALHaLD+dPwxfeKnbhpD4AHdmHJDjhuo97g+3ZspdOwYQm nTYlJ7DEJsS7W1UnkbxqPdomss0M8VD/H8tAzQkhtrcIZRT7uHyEva5ykUFpsiqbUsgK ih+VzNHukOrJQkUjXET57SmdPwz7X7gVOgS3oCUz71Evv5QVX8lSD9YYw79q12YDm5p8 1PaohPR1UhkJcTX2SZ7VneYDxg5YRDhFYBmgHMeUSDixBNbtEub3SFIZF3Tvk2Vy34hD XoFs12/ioEQ9Ohy+R+wPcp2xVurTM7zp+WSGlpyXkWV7dAKnRxzrkrwes99FCLw8UYny L/vA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=qFOfaEvokfZV1EFTfIDgkQYCii1N7VmjLFzadFiYr08=; b=f8JpO/N8i/taPqEKDs198UkepWMuO3srXN5f7M2bZLCqnLCln61CfDbtnUG6I07a/e k9mm5H5UmWtdPz5WqALU6sODhhJs6rfr+R9CXNTo0i9ArHZ9iQf6cxQ+k+1y5ejOIgmB Fd3w1+BSDh7Fnmw4tw27n3AJrG0hiUQ4SoSQhQNkb2GF6mx7Z54Y/ce/TUPCr1/fjsyw iU3YAeJG9nbNj1CyOS/kERQ5OqrFchck53Nlbf24omcSgySaapDMLgCh/CHnnzMTc9YR WNLklVuiE6UZ/a7/+YUyAWaOATTAKRVx296jtYcF7VBVEehPVLwPf0Vcz/a2BNwPZhLM ecoQ== X-Gm-Message-State: AOAM533XNWb6XGv9An0P31ZDOYE0u1YxJY2OfdManz5AiMhV9dNTgNBp cj9XE9Kxl3ks220omYe5+Rct8r9NRwxjm9Q/vrI= X-Google-Smtp-Source: ABdhPJyf8+4K1X40XYLX6CA6YiAMIijTYfrHswTWu60IzHY/qXu3L67uo3anH53hXnEVh7YwelBqF4tj/z+87r06yoI= X-Received: by 2002:a05:6102:3232:: with SMTP id x18mr1997159vsf.11.1641818174323; Mon, 10 Jan 2022 04:36:14 -0800 (PST) MIME-Version: 1.0 References: <1641488717-13865-1-git-send-email-quic_charante@quicinc.com> In-Reply-To: <1641488717-13865-1-git-send-email-quic_charante@quicinc.com> From: Mark Hemment Date: Mon, 10 Jan 2022 12:36:03 +0000 Message-ID: Subject: Re: [PATCH v3 RESEND] mm: shmem: implement POSIX_FADV_[WILL|DONT]NEED for shmem To: Charan Teja Reddy Cc: Hugh Dickins , Andrew Morton , "Matthew Wilcox (Oracle)" , vbabka@suse.cz, rientjes@google.com, mhocko@suse.com, Suren Baghdasaryan , Shakeel Butt , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Charan Teja Reddy Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 0AA2614000C X-Stat-Signature: 89moe3xxnowz6uxtkemma3buokt4amud Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=googlemail.com header.s=20210112 header.b=OWQBLpWC; dmarc=pass (policy=quarantine) header.from=googlemail.com; spf=pass (imf23.hostedemail.com: domain of markhemm@googlemail.com designates 209.85.222.53 as permitted sender) smtp.mailfrom=markhemm@googlemail.com X-HE-Tag: 1641818174-75053 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, 6 Jan 2022 at 17:06, Charan Teja Reddy wrote: > > From: Charan Teja Reddy > > Currently fadvise(2) is supported only for the files that doesn't > associated with noop_backing_dev_info thus for the files, like shmem, > fadvise results into NOP. But then there is file_operations->fadvise() > that lets the file systems to implement their own fadvise > implementation. Use this support to implement some of the POSIX_FADV_XXX > functionality for shmem files. ... > +static void shmem_isolate_pages_range(struct address_space *mapping, loff_t start, > + loff_t end, struct list_head *list) > +{ > + XA_STATE(xas, &mapping->i_pages, start); > + struct page *page; > + > + rcu_read_lock(); > + xas_for_each(&xas, page, end) { > + if (xas_retry(&xas, page)) > + continue; > + if (xa_is_value(page)) > + continue; > + if (!get_page_unless_zero(page)) > + continue; > + if (isolate_lru_page(page)) > + continue; Need to unwind the get_page on failure to isolate. Should PageUnevicitable() pages (SHM_LOCK) be skipped? (That is, does SHM_LOCK override DONTNEED?) ... > +static int shmem_fadvise_dontneed(struct address_space *mapping, loff_t start, > + loff_t end) > +{ > + int ret; > + struct page *page; > + LIST_HEAD(list); > + struct writeback_control wbc = { > + .sync_mode = WB_SYNC_NONE, > + .nr_to_write = LONG_MAX, > + .range_start = 0, > + .range_end = LLONG_MAX, > + .for_reclaim = 1, > + }; > + > + if (!shmem_mapping(mapping)) > + return -EINVAL; > + > + if (!total_swap_pages) > + return 0; > + > + lru_add_drain(); > + shmem_isolate_pages_range(mapping, start, end, &list); > + > + while (!list_empty(&list)) { > + page = lru_to_page(&list); > + list_del(&page->lru); > + if (page_mapped(page)) > + goto keep; > + if (!trylock_page(page)) > + goto keep; > + if (unlikely(PageTransHuge(page))) { > + if (split_huge_page_to_list(page, &list)) > + goto keep; > + } I don't know the shmem code and the lifecycle of a shm-page, so genuine questions; When the try-lock succeeds, should there be a test for PageWriteback() (page skipped if true)? Also, does page->mapping need to be tested for NULL to prevent races with deletion from the page-cache? ... > + > + clear_page_dirty_for_io(page); > + SetPageReclaim(page); > + ret = shmem_writepage(page, &wbc); > + if (ret || PageWriteback(page)) { > + if (ret) > + unlock_page(page); > + goto keep; > + } > + > + if (!PageWriteback(page)) > + ClearPageReclaim(page); > + > + /* > + * shmem_writepage() place the page in the swapcache. > + * Delete the page from the swapcache and release the > + * page. > + */ > + __mod_node_page_state(page_pgdat(page), > + NR_ISOLATED_ANON + page_is_file_lru(page), compound_nr(page)); > + lock_page(page); > + delete_from_swap_cache(page); > + unlock_page(page); > + put_page(page); > + continue; > +keep: > + putback_lru_page(page); > + __mod_node_page_state(page_pgdat(page), > + NR_ISOLATED_ANON + page_is_file_lru(page), compound_nr(page)); > + } The putback_lru_page() drops the last reference hold this code has on 'page'. Is it safe to use 'page' after dropping this reference? Cheers, Mark