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?
