Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
From: "zhangxiaoxu (A)" <zhangxiaoxu5@huawei.com>
To: Trond Myklebust <trondmy@hammerspace.com>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"anna.schumaker@netapp.com" <anna.schumaker@netapp.com>
Subject: Re: [PATCH] nfs: Fix nfsi->nrequests count error on nfs_inode_remove_request
Date: Tue, 8 Oct 2019 10:00:07 +0800
Message-ID: <4096d9e4-f6a2-b1e1-4f52-81cf44fc4a1f@huawei.com> (raw)
In-Reply-To: <cb955e718bd3c62f9c23661e95db77efa65d7177.camel@hammerspace.com>

Thanks for your review.

I think PG_REMOVE and PG_INODE_REF can't be both setted except in
'nfs_inode_remove_request' function.

nfs_inode_remove_request
	// maybe set PG_REMOVE here.
	nfs_page_group_sync_on_bit(req, PG_REMOVE)
		nfs_page_group_lock(req);
		nfs_page_group_sync_on_bit_locked
			WARN_ON_ONCE(test_and_set_bit(bit, &req->wb_flags));
		nfs_page_group_unlock(req);
	// But also clear the PG_INODE_REF flag.
	test_and_clear_bit(PG_INODE_REF, &req->wb_flags)

'nfs_lock_and_join_requests' also need the PG_HEADLOCK flag:

nfs_lock_and_join_requests
	nfs_page_group_lock(head);
		test_and_clear_bit(PG_REMOVE, &head->wb_flags)
	nfs_page_group_unlock(head);


在 2019/10/5 22:35, Trond Myklebust 写道:
> However nfs_lock_and_join_requests() looks like it does need to change
> to something like the following:
> 
>          /* Postpone destruction of this request */
> -       if (test_and_clear_bit(PG_REMOVE, &head->wb_flags)) {
> -               set_bit(PG_INODE_REF, &head->wb_flags);
> +       if (test_and_clear_bit(PG_REMOVE, &head->wb_flags) &&
> +           !test_and_set_bit(PG_INODE_REF, &head->wb_flags)) {
>                  kref_get(&head->wb_kref);
>                  atomic_long_inc(&NFS_I(inode)->nrequests);
>          }
> 
> 
> Do you agree?


  reply index

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-26  6:29 ZhangXiaoxu
2019-10-05  1:51 ` zhangxiaoxu (A)
2019-10-05 14:07   ` Trond Myklebust
2019-10-05 14:35     ` Trond Myklebust
2019-10-08  2:00       ` zhangxiaoxu (A) [this message]
2019-10-08 12:31         ` Trond Myklebust

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4096d9e4-f6a2-b1e1-4f52-81cf44fc4a1f@huawei.com \
    --to=zhangxiaoxu5@huawei.com \
    --cc=anna.schumaker@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trondmy@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org
	public-inbox-index linux-nfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git