Amir Goldstein <amir73il@xxxxxxxxx> writes:
> On Fri, Aug 13, 2021 at 12:41 AM Gabriel Krisman Bertazi
> <krisman@xxxxxxxxxxxxx> wrote:
>>
>> Some file system events (i.e. FS_ERROR) might not be associated with an
>> inode. For these, it makes sense to associate them directly with the
>> super block of the file system they apply to. This patch allows the
>> event to be reported with a NULL inode, by recovering the superblock
>> directly from the data field, if needed.
>>
>> Signed-off-by: Gabriel Krisman Bertazi <krisman@xxxxxxxxxxxxx>
>>
>> --
>> Changes since v5:
>> - add fsnotify_data_sb handle to retrieve sb from the data field. (jan)
>> ---
>> fs/notify/fsnotify.c | 16 +++++++++++++---
>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
>> index 30d422b8c0fc..536db02cb26e 100644
>> --- a/fs/notify/fsnotify.c
>> +++ b/fs/notify/fsnotify.c
>> @@ -98,6 +98,14 @@ void fsnotify_sb_delete(struct super_block *sb)
>> fsnotify_clear_marks_by_sb(sb);
>> }
>>
>> +static struct super_block *fsnotify_data_sb(const void *data, int data_type)
>> +{
>> + struct inode *inode = fsnotify_data_inode(data, data_type);
>> + struct super_block *sb = inode ? inode->i_sb : NULL;
>> +
>> + return sb;
>> +}
>> +
>> /*
>> * Given an inode, first check if we care what happens to our children. Inotify
>> * and dnotify both tell their parents about events. If we care about any event
>> @@ -455,8 +463,10 @@ static void fsnotify_iter_next(struct fsnotify_iter_info *iter_info)
>> * @file_name is relative to
>> * @file_name: optional file name associated with event
>> * @inode: optional inode associated with event -
>> - * either @dir or @inode must be non-NULL.
>> - * if both are non-NULL event may be reported to both.
>> + * If @dir and @inode are NULL, @data must have a type that
>> + * allows retrieving the file system associated with this
>
> Irrelevant comment. sb must always be available from @data.
>
>> + * event. if both are non-NULL event may be reported to
>> + * both.
>> * @cookie: inotify rename cookie
>> */
>> int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
>> @@ -483,7 +493,7 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
>> */
>> parent = dir;
>> }
>> - sb = inode->i_sb;
>> + sb = inode ? inode->i_sb : fsnotify_data_sb(data, data_type);
>
> const struct path *path = fsnotify_data_path(data, data_type);
> + const struct super_block *sb = fsnotify_data_sb(data, data_type);
>
> All the games with @data @inode and @dir args are irrelevant to this.
> sb should always be available from @data and it does not matter
> if fsnotify_data_inode() is the same as @inode, @dir or neither.
> All those inodes are anyway on the same sb.
Hi Amir,
I think this is actually necessary. I could identify at least one event
(FS_CREATE | FS_ISDIR) where fsnotify is invoked with a NULL data field.
In that case, fsnotify_dirent is called with a negative dentry from
vfs_mkdir(). I'm not sure why exactly the dentry is negative after the
mkdir, but it would be possible we are racing with the file removal, I
guess? It might be a bug in fsnotify that this case even happen, but
I'm not sure yet.
The easiest way around it is what I proposed: just use inode->i_sb if
data is NULL. Since, as you said, data, inode and dir are all on the
same superblock, it should work, I think.
--
Gabriel Krisman Bertazi
