| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CADnza444dTr=JEtqpL5wxHRNkEc7vBz1qq9TL7Z+5h749vNawg@mail.gmail.com>
Date: Mon, 16 Apr 2012 09:10:04 -0400
From: Fred Isaman <iisaman@...app.com>
To: Mel Gorman <mgorman@...e.de>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Linux-MM <linux-mm@...ck.org>,
Linux-Netdev <netdev@...r.kernel.org>,
Linux-NFS <linux-nfs@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
David Miller <davem@...emloft.net>,
Trond Myklebust <Trond.Myklebust@...app.com>,
Neil Brown <neilb@...e.de>,
Christoph Hellwig <hch@...radead.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Mike Christie <michaelc@...wisc.edu>,
Eric B Munson <emunson@...bm.net>
Subject: Re: [PATCH 08/11] nfs: disable data cache revalidation for swapfiles
On Mon, Apr 16, 2012 at 8:17 AM, Mel Gorman <mgorman@...e.de> wrote:
> The VM does not like PG_private set on PG_swapcache pages. As suggested
> by Trond in http://lkml.org/lkml/2006/8/25/348, this patch disables
> NFS data cache revalidation on swap files. as it does not make
> sense to have other clients change the file while it is being used as
> swap. This avoids setting PG_private on swap pages, since there ought
> to be no further races with invalidate_inode_pages2() to deal with.
>
> Since we cannot set PG_private we cannot use page->private which
> is already used by PG_swapcache pages to store the nfs_page. Thus
> augment the new nfs_page_find_request logic.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Signed-off-by: Mel Gorman <mgorman@...e.de>
> ---
> fs/nfs/inode.c | 6 ++++++
> fs/nfs/write.c | 51 +++++++++++++++++++++++++++++++++++++--------------
> 2 files changed, 43 insertions(+), 14 deletions(-)
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index e8bbfa5..af43ef6 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -880,6 +880,12 @@ int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping)
> struct nfs_inode *nfsi = NFS_I(inode);
> int ret = 0;
>
> + /*
> + * swapfiles are not supposed to be shared.
> + */
> + if (IS_SWAPFILE(inode))
> + goto out;
> +
> if ((nfsi->cache_validity & NFS_INO_REVAL_PAGECACHE)
> || nfs_attribute_cache_expired(inode)
> || NFS_STALE(inode)) {
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index 6a891eb..eea4ec0 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -111,15 +111,30 @@ static void nfs_context_set_write_error(struct nfs_open_context *ctx, int error)
> set_bit(NFS_CONTEXT_ERROR_WRITE, &ctx->flags);
> }
>
> -static struct nfs_page *nfs_page_find_request_locked(struct page *page)
> +static struct nfs_page *
> +nfs_page_find_request_locked(struct nfs_inode *nfsi, struct page *page)
> {
> struct nfs_page *req = NULL;
>
> - if (PagePrivate(page)) {
> + if (PagePrivate(page))
> req = (struct nfs_page *)page_private(page);
> - if (req != NULL)
> - kref_get(&req->wb_kref);
> + else if (unlikely(PageSwapCache(page))) {
> + struct nfs_page *freq, *t;
> +
> + /* Linearly search the commit list for the correct req */
> + list_for_each_entry_safe(freq, t, &nfsi->commit_list, wb_list) {
> + if (freq->wb_page == page) {
> + req = freq;
> + break;
> + }
> + }
> +
> + BUG_ON(req == NULL);
I suspect I am missing something, but why is it guaranteed that the
req is on the commit list?
Fred
> }
> +
> + if (req)
> + kref_get(&req->wb_kref);
> +
> return req;
> }
>
> @@ -129,7 +144,7 @@ static struct nfs_page *nfs_page_find_request(struct page *page)
> struct nfs_page *req = NULL;
>
> spin_lock(&inode->i_lock);
> - req = nfs_page_find_request_locked(page);
> + req = nfs_page_find_request_locked(NFS_I(inode), page);
> spin_unlock(&inode->i_lock);
> return req;
> }
> @@ -232,7 +247,7 @@ static struct nfs_page *nfs_find_and_lock_request(struct page *page, bool nonblo
>
> spin_lock(&inode->i_lock);
> for (;;) {
> - req = nfs_page_find_request_locked(page);
> + req = nfs_page_find_request_locked(NFS_I(inode), page);
> if (req == NULL)
> break;
> if (nfs_lock_request_dontget(req))
> @@ -385,9 +400,15 @@ static void nfs_inode_add_request(struct inode *inode, struct nfs_page *req)
> spin_lock(&inode->i_lock);
> if (!nfsi->npages && nfs_have_delegation(inode, FMODE_WRITE))
> inode->i_version++;
> - set_bit(PG_MAPPED, &req->wb_flags);
> - SetPagePrivate(req->wb_page);
> - set_page_private(req->wb_page, (unsigned long)req);
> + /*
> + * Swap-space should not get truncated. Hence no need to plug the race
> + * with invalidate/truncate.
> + */
> + if (likely(!PageSwapCache(req->wb_page))) {
> + set_bit(PG_MAPPED, &req->wb_flags);
> + SetPagePrivate(req->wb_page);
> + set_page_private(req->wb_page, (unsigned long)req);
> + }
> nfsi->npages++;
> kref_get(&req->wb_kref);
> spin_unlock(&inode->i_lock);
> @@ -404,9 +425,11 @@ static void nfs_inode_remove_request(struct nfs_page *req)
> BUG_ON (!NFS_WBACK_BUSY(req));
>
> spin_lock(&inode->i_lock);
> - set_page_private(req->wb_page, 0);
> - ClearPagePrivate(req->wb_page);
> - clear_bit(PG_MAPPED, &req->wb_flags);
> + if (likely(!PageSwapCache(req->wb_page))) {
> + set_page_private(req->wb_page, 0);
> + ClearPagePrivate(req->wb_page);
> + clear_bit(PG_MAPPED, &req->wb_flags);
> + }
> nfsi->npages--;
> spin_unlock(&inode->i_lock);
> nfs_release_request(req);
> @@ -646,7 +669,7 @@ static struct nfs_page *nfs_try_to_update_request(struct inode *inode,
> spin_lock(&inode->i_lock);
>
> for (;;) {
> - req = nfs_page_find_request_locked(page);
> + req = nfs_page_find_request_locked(NFS_I(inode), page);
> if (req == NULL)
> goto out_unlock;
>
> @@ -1690,7 +1713,7 @@ int nfs_wb_page_cancel(struct inode *inode, struct page *page)
> */
> int nfs_wb_page(struct inode *inode, struct page *page)
> {
> - loff_t range_start = page_offset(page);
> + loff_t range_start = page_file_offset(page);
> loff_t range_end = range_start + (loff_t)(PAGE_CACHE_SIZE - 1);
> struct writeback_control wbc = {
> .sync_mode = WB_SYNC_ALL,
> --
> 1.7.9.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists