On Tue, 2019-10-08 at 10:00 +0800, zhangxiaoxu (A) wrote: > 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); Fair enough, and the race between nfs_inode_remove_request() and nfs_lock_and_join_requests() with the setting of PG_REMOVE and the clearing on PG_INODE_REF is prevented by the presence of the PG_LOCK. OK. I agree with that argument. Thanks for the explanation! Cheers Trond -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
