Messages in this thread | | | Date | Sat, 31 Aug 2019 08:54:46 +0800 | From | Gao Xiang <> | Subject | Re: [PATCH v6 03/24] erofs: add super block operations |
| |
Hi Christoph,
On Sat, Aug 31, 2019 at 01:15:10AM +0800, Gao Xiang wrote:
[]
> > > > > > > + /* be careful RCU symlink path (see ext4_inode_info->i_data)! */ > > > > > + if (is_inode_fast_symlink(inode)) > > > > > + kfree(inode->i_link); > > > > > > > > is_inode_fast_symlink only shows up in a later patch. And really > > > > obsfucates the check here in the only caller as you can just do an > > > > unconditional kfree here - i_link will be NULL except for the case > > > > where you explicitly set it. > > > > > > I cannot fully understand your point (sorry about my English), > > > I will reply you about this later. > > > > With that I mean that you should: > > > > 1) remove is_inode_fast_symlink and just opencode it in the few places > > using it > > 2) remove the check in this place entirely as it is not needed
Add some words about this suggestion since I'm addressing this place, it seems it could not (or I am not sure at least) be freed unconditionally
union { struct pipe_inode_info *i_pipe; struct block_device *i_bdev; struct cdev *i_cdev; char *i_link; unsigned i_dir_seq; };
while I saw what shmem did, it seems that they handle as follows: 3636 static void shmem_free_in_core_inode(struct inode *inode) 3637 { 3638 if (S_ISLNK(inode->i_mode)) 3639 kfree(inode->i_link); 3640 kmem_cache_free(shmem_inode_cachep, SHMEM_I(inode)); 3641 }
I think that would be some check on it to get it is a symlink (for i_dir_seq it seems unsafe).... I think the original check is ok but I will opencode it instead.
Thanks, Gao Xiang
| |