On Jan 10, 2017, at 5:27 AM, Jan Kara <jack@xxxxxxx> wrote:
>
> evict_inodes() and invalidate_inodes() use list_for_each_entry_safe()
> to iterate sb->s_inodes list. However, since we use i_lru list entry for
> our local temporary list of inodes to destroy, the inode is guaranteed
> to stay in sb->s_inodes list while we hold sb->s_inode_list_lock. So
> there is no real need for safe iteration variant and we can use
> list_for_each_entry() just fine.
This is a pretty "subtle" change, IMHO, with little benefit. IMHO, using
the "_safe" variant makes it more clear to the reader that the inode is
being deleted from the list. At a minimum, I'd think there should be a
comment at list_for_each_entry() to the effect that the inode is not going
to be deleted, so list_for_each_entry_safe() is not needed. Otherwise, if
the inode lifetime changes in some manner in the future this may introduce
hard-to-find corruption of freed memory.
Cheers, Andreas
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> fs/inode.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 88110fd0b282..bd5a47ff2f03 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -601,12 +601,12 @@ static void dispose_list(struct list_head *head)
> */
> void evict_inodes(struct super_block *sb)
> {
> - struct inode *inode, *next;
> + struct inode *inode;
> LIST_HEAD(dispose);
>
> again:
> spin_lock(&sb->s_inode_list_lock);
> - list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) {
> + list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> if (atomic_read(&inode->i_count))
> continue;
>
> @@ -651,11 +651,11 @@ void evict_inodes(struct super_block *sb)
> int invalidate_inodes(struct super_block *sb, bool kill_dirty)
> {
> int busy = 0;
> - struct inode *inode, *next;
> + struct inode *inode;
> LIST_HEAD(dispose);
>
> spin_lock(&sb->s_inode_list_lock);
> - list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) {
> + list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> spin_lock(&inode->i_lock);
> if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) {
> spin_unlock(&inode->i_lock);
> --
> 2.10.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail
