On Tue 03-05-22 03:03:07, Jchao Sun wrote:
> Cause of patch b35250c, inode->i_io_list will not only be protected by
> wb->list_lock, but also inode->i_lock. And in that patch, Added some assert
> for inode->i_lock in some functions except inode_io_list_move_locked.
> Should complete it to describe the semantics more clearly. Modified comment
> correspondingly.
I'd slightly rephrase the commit message and reference commit properly. Like:
Commit b35250c0816c ("writeback: Protect inode->i_io_list with inode->i_lock")
made inode->i_io_list not only protected by wb->list_lock but also
inode->i_lock. The commit also added some asserts for inode->i_lock but
inode_io_list_move_locked() was missed. Add assert there and also update
comment describing things protected by inode->i_lock.
Also please add
Fixes: b35250c0816c ("writeback: Protect inode->i_io_list with inode->i_lock")
tag.
> Signed-off-by: Jchao Sun <sunjunchao2870@xxxxxxxxx>
Otherwise a good catch. Thanks. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/fs-writeback.c | 1 +
> fs/inode.c | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 591fe9cf1659..5a761b39f36c 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -120,6 +120,7 @@ static bool inode_io_list_move_locked(struct inode *inode,
> struct list_head *head)
> {
> assert_spin_locked(&wb->list_lock);
> + assert_spin_locked(&inode->i_lock);
>
> list_move(&inode->i_io_list, head);
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 9d9b422504d1..bd4da9c5207e 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -27,7 +27,7 @@
> * Inode locking rules:
> *
> * inode->i_lock protects:
> - * inode->i_state, inode->i_hash, __iget()
> + * inode->i_state, inode->i_hash, __iget(), inode->i_io_list
> * Inode LRU list locks protect:
> * inode->i_sb->s_inode_lru, inode->i_lru
> * inode->i_sb->s_inode_list_lock protects:
> --
> 2.17.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
