Re: [PATCH] nfs: Fix nfsi->nrequests count error on nfs_inode_remove_request

["zhangxiaoxu (A)" <zhangxiaoxu5@huawei.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?