In order to identify which thread triggered the event in the
multi-threaded program, add the FAN_EVENT_INFO_TID tag in fanotify_init
to select whether to report the event creator's thread id information.
Signed-off-by: nixiaoming <[email protected]>
---
fs/notify/fanotify/fanotify.c | 5 ++++-
fs/notify/fanotify/fanotify_user.c | 5 +++--
include/linux/fsnotify_backend.h | 1 +
include/uapi/linux/fanotify.h | 1 +
4 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 94b5215..5a3af87 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -171,7 +171,10 @@ struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group,
goto out;
init: __maybe_unused
fsnotify_init_event(&event->fse, inode, mask);
- event->tgid = get_pid(task_tgid(current));
+ if (group->fanotify_data.should_report_tid)
+ event->tgid = get_pid(task_pid(current));
+ else
+ event->tgid = get_pid(task_tgid(current));
if (path) {
event->path = *path;
path_get(&event->path);
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 6905488..3aa425b 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -693,9 +693,9 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
return -EPERM;
#ifdef CONFIG_AUDITSYSCALL
- if (flags & ~(FAN_ALL_INIT_FLAGS | FAN_ENABLE_AUDIT))
+ if (flags & ~(FAN_ALL_INIT_FLAGS | FAN_ENABLE_AUDIT | FAN_EVENT_INFO_TID))
#else
- if (flags & ~FAN_ALL_INIT_FLAGS)
+ if (flags & ~(FAN_ALL_INIT_FLAGS | FAN_EVENT_INFO_TID))
#endif
return -EINVAL;
@@ -731,6 +731,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
}
group->fanotify_data.user = user;
+ group->fanotify_data.should_report_tid = (flags & FAN_EVENT_INFO_TID) ? true : false;
atomic_inc(&user->fanotify_listeners);
group->memcg = get_mem_cgroup_from_mm(current->mm);
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index b8f4182..44c659f 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -193,6 +193,7 @@ struct fsnotify_group {
unsigned int max_marks;
struct user_struct *user;
bool audit;
+ bool should_report_tid;
} fanotify_data;
#endif /* CONFIG_FANOTIFY */
};
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index 7424791..4eba41c 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -18,6 +18,7 @@
#define FAN_ONDIR 0x40000000 /* event occurred against dir */
+#define FAN_EVENT_INFO_TID 0x02000000 /* reports the thread id of the event trigger */
#define FAN_EVENT_ON_CHILD 0x08000000 /* interested in child events */
/* helper events */
--
2.10.1
On Mon, Sep 17, 2018 at 6:05 PM nixiaoming <[email protected]> wrote:
>
> In order to identify which thread triggered the event in the
> multi-threaded program, add the FAN_EVENT_INFO_TID tag in fanotify_init
According to code and man page this is a 'flag' not a 'tag'
Please stick to existing terminology.
> to select whether to report the event creator's thread id information.
>
> Signed-off-by: nixiaoming <[email protected]>
> ---
> fs/notify/fanotify/fanotify.c | 5 ++++-
> fs/notify/fanotify/fanotify_user.c | 5 +++--
> include/linux/fsnotify_backend.h | 1 +
> include/uapi/linux/fanotify.h | 1 +
> 4 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 94b5215..5a3af87 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -171,7 +171,10 @@ struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group,
> goto out;
> init: __maybe_unused
> fsnotify_init_event(&event->fse, inode, mask);
> - event->tgid = get_pid(task_tgid(current));
> + if (group->fanotify_data.should_report_tid)
> + event->tgid = get_pid(task_pid(current));
> + else
> + event->tgid = get_pid(task_tgid(current));
> if (path) {
> event->path = *path;
> path_get(&event->path);
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 6905488..3aa425b 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -693,9 +693,9 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
> return -EPERM;
>
> #ifdef CONFIG_AUDITSYSCALL
> - if (flags & ~(FAN_ALL_INIT_FLAGS | FAN_ENABLE_AUDIT))
> + if (flags & ~(FAN_ALL_INIT_FLAGS | FAN_ENABLE_AUDIT | FAN_EVENT_INFO_TID))
> #else
> - if (flags & ~FAN_ALL_INIT_FLAGS)
> + if (flags & ~(FAN_ALL_INIT_FLAGS | FAN_EVENT_INFO_TID))
Not this way. You need to add FAN_EVENT_INFO_TID to FAN_ALL_INIT_FLAGS.
> #endif
> return -EINVAL;
>
> @@ -731,6 +731,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
> }
>
> group->fanotify_data.user = user;
> + group->fanotify_data.should_report_tid = (flags & FAN_EVENT_INFO_TID) ? true : false;
Please drop "? true : false" its not needed.
> atomic_inc(&user->fanotify_listeners);
> group->memcg = get_mem_cgroup_from_mm(current->mm);
>
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index b8f4182..44c659f 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -193,6 +193,7 @@ struct fsnotify_group {
> unsigned int max_marks;
> struct user_struct *user;
> bool audit;
> + bool should_report_tid;
For brevity I would call that report_tid, but not insisting.
> } fanotify_data;
> #endif /* CONFIG_FANOTIFY */
> };
> diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
> index 7424791..4eba41c 100644
> --- a/include/uapi/linux/fanotify.h
> +++ b/include/uapi/linux/fanotify.h
> @@ -18,6 +18,7 @@
>
> #define FAN_ONDIR 0x40000000 /* event occurred against dir */
>
> +#define FAN_EVENT_INFO_TID 0x02000000 /* reports the thread id of the event trigger */
That is not the right place nor the sensible value for this flag, but this:
#define FAN_UNLIMITED_QUEUE 0x00000010
#define FAN_UNLIMITED_MARKS 0x00000020
#define FAN_ENABLE_AUDIT 0x00000040
+#define FAN_EVENT_INFO_TID 0x00000080
Thanks,
Amir.
On Mon, Sep 17, 2018 11:51 PM Amir Goldstein <[email protected]> wrote:
>On Mon, Sep 17, 2018 at 6:05 PM nixiaoming <[email protected]> wrote:
>>
>> In order to identify which thread triggered the event in the
>> multi-threaded program, add the FAN_EVENT_INFO_TID tag in fanotify_init
>
>According to code and man page this is a 'flag' not a 'tag'
>Please stick to existing terminology.
>
Thank you for your guidance, I will correct it in a later patch.
>> to select whether to report the event creator's thread id information.
>>
>> Signed-off-by: nixiaoming <[email protected]>
.......
>> @@ -693,9 +693,9 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>> return -EPERM;
>>
>> #ifdef CONFIG_AUDITSYSCALL
>> - if (flags & ~(FAN_ALL_INIT_FLAGS | FAN_ENABLE_AUDIT))
>> + if (flags & ~(FAN_ALL_INIT_FLAGS | FAN_ENABLE_AUDIT | FAN_EVENT_INFO_TID))
>> #else
>> - if (flags & ~FAN_ALL_INIT_FLAGS)
>> + if (flags & ~(FAN_ALL_INIT_FLAGS | FAN_EVENT_INFO_TID))
>
>Not this way. You need to add FAN_EVENT_INFO_TID to FAN_ALL_INIT_FLAGS.
Thank you for your guidance, I will correct it in a later patch.
>
>> #endif
>> return -EINVAL;
>>
>> @@ -731,6 +731,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>> }
>>
>> group->fanotify_data.user = user;
>> + group->fanotify_data.should_report_tid = (flags & FAN_EVENT_INFO_TID) ? true : false;
>
>Please drop "? true : false" its not needed.
>
>> atomic_inc(&user->fanotify_listeners);
>> group->memcg = get_mem_cgroup_from_mm(current->mm);
>>
>> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
>> index b8f4182..44c659f 100644
>> --- a/include/linux/fsnotify_backend.h
>> +++ b/include/linux/fsnotify_backend.h
>> @@ -193,6 +193,7 @@ struct fsnotify_group {
>> unsigned int max_marks;
>> struct user_struct *user;
>> bool audit;
>> + bool should_report_tid;
>
>For brevity I would call that report_tid, but not insisting.
>
Whether it is better to change to "unsigned int flags"
Save "group->fanotify_data.flags=flags;" in "fanotify_init"
Determine whether "group->fanotify_data.flags" contains "FAN_EVENT_INFO_TID" in "fanotify_alloc_event";
At the same time, if there are other flags that need to be used later, there is no need to add new members.
By the way, whether or not "bool audit" can also be included by flags
thanks
>> } fanotify_data;
>> #endif /* CONFIG_FANOTIFY */
>> };
>> diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
>> index 7424791..4eba41c 100644
>> --- a/include/uapi/linux/fanotify.h
>> +++ b/include/uapi/linux/fanotify.h
>> @@ -18,6 +18,7 @@
>>
>> #define FAN_ONDIR 0x40000000 /* event occurred against dir */
>>
>> +#define FAN_EVENT_INFO_TID 0x02000000 /* reports the thread id of the event trigger */
>
>That is not the right place nor the sensible value for this flag, but this:
>
>#define FAN_UNLIMITED_QUEUE 0x00000010
>#define FAN_UNLIMITED_MARKS 0x00000020
>#define FAN_ENABLE_AUDIT 0x00000040
>+#define FAN_EVENT_INFO_TID 0x00000080
>
>Thanks,
>Amir.
>
On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming <[email protected]> wrote:
>
> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein <[email protected]> wrote:
> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming <[email protected]> wrote:
...
> >> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> >> index b8f4182..44c659f 100644
> >> --- a/include/linux/fsnotify_backend.h
> >> +++ b/include/linux/fsnotify_backend.h
> >> @@ -193,6 +193,7 @@ struct fsnotify_group {
> >> unsigned int max_marks;
> >> struct user_struct *user;
> >> bool audit;
> >> + bool should_report_tid;
> >
> >For brevity I would call that report_tid, but not insisting.
> >
>
> Whether it is better to change to "unsigned int flags"
> Save "group->fanotify_data.flags=flags;" in "fanotify_init"
> Determine whether "group->fanotify_data.flags" contains "FAN_EVENT_INFO_TID" in "fanotify_alloc_event";
>
> At the same time, if there are other flags that need to be used later, there is no need to add new members.
>
> By the way, whether or not "bool audit" can also be included by flags
>
I strongly agree. Didn't want to impose this change on you, but in
fact, I already did
that in my own patch set, so you can use my patches as reference:
https://github.com/amir73il/linux/commit/5225fe1e19c74f4d7a4a4cc98ff6ef5872c8e620
https://github.com/amir73il/linux/commit/0d1a226f5c18012f19ed6eabfab46f0f125ec190
If you do this you need to separate your change into 2 patches.
First make the fanotify_data.flags change collecting the pieces from both
my patches.
Then make your TID change using fanotify_data.flags.
Thanks,
Amir.
On Tue, Sep 18, 2018 3:07 PM Amir Goldstein <[email protected]>
>On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming <[email protected]> wrote:
>>
>> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein <[email protected]> wrote:
>> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming <[email protected]> wrote:
>...
>> >> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
>> >> index b8f4182..44c659f 100644
>> >> --- a/include/linux/fsnotify_backend.h
>> >> +++ b/include/linux/fsnotify_backend.h
>> >> @@ -193,6 +193,7 @@ struct fsnotify_group {
>> >> unsigned int max_marks;
>> >> struct user_struct *user;
>> >> bool audit;
>> >> + bool should_report_tid;
>> >
>> >For brevity I would call that report_tid, but not insisting.
>> >
>>
>> Whether it is better to change to "unsigned int flags"
>> Save "group->fanotify_data.flags=flags;" in "fanotify_init"
>> Determine whether "group->fanotify_data.flags" contains "FAN_EVENT_INFO_TID" in "fanotify_alloc_event";
>>
>> At the same time, if there are other flags that need to be used later, there is no need to add new members.
>>
>> By the way, whether or not "bool audit" can also be included by flags
>>
>
>I strongly agree. Didn't want to impose this change on you, but in
>fact, I already did
>that in my own patch set, so you can use my patches as reference:
>https://github.com/amir73il/linux/commit/5225fe1e19c74f4d7a4a4cc98ff6ef5872c8e620
>https://github.com/amir73il/linux/commit/0d1a226f5c18012f19ed6eabfab46f0f125ec190
>
>If you do this you need to separate your change into 2 patches.
>First make the fanotify_data.flags change collecting the pieces from both
>my patches.
>Then make your TID change using fanotify_data.flags.
>
>Thanks,
>Amir.
>
Should I do this:
1 git cherry-pick 5225fe1e19c74f4d7a4a4cc98ff6ef5872c8e620
Git cherry-pick 0d1a226f5c18012f19ed6eabfab46f0f125ec190
Manually handle patch conflicts
2 Add a new flag "FAN_EVENT_INFO_TID" into fanotify_data.flags
3 Incorporate "bool audit" into fanotify_data.flags
4 Submit 4 patches
thanks
On Tue, Sep 18, 2018 at 12:50 PM Nixiaoming <[email protected]> wrote:
>
> On Tue, Sep 18, 2018 3:07 PM Amir Goldstein <[email protected]>
> >On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming <[email protected]> wrote:
> >>
> >> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein <[email protected]> wrote:
> >> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming <[email protected]> wrote:
> >...
> >> >> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> >> >> index b8f4182..44c659f 100644
> >> >> --- a/include/linux/fsnotify_backend.h
> >> >> +++ b/include/linux/fsnotify_backend.h
> >> >> @@ -193,6 +193,7 @@ struct fsnotify_group {
> >> >> unsigned int max_marks;
> >> >> struct user_struct *user;
> >> >> bool audit;
> >> >> + bool should_report_tid;
> >> >
> >> >For brevity I would call that report_tid, but not insisting.
> >> >
> >>
> >> Whether it is better to change to "unsigned int flags"
> >> Save "group->fanotify_data.flags=flags;" in "fanotify_init"
> >> Determine whether "group->fanotify_data.flags" contains "FAN_EVENT_INFO_TID" in "fanotify_alloc_event";
> >>
> >> At the same time, if there are other flags that need to be used later, there is no need to add new members.
> >>
> >> By the way, whether or not "bool audit" can also be included by flags
> >>
> >
> >I strongly agree. Didn't want to impose this change on you, but in
> >fact, I already did
> >that in my own patch set, so you can use my patches as reference:
> >https://github.com/amir73il/linux/commit/5225fe1e19c74f4d7a4a4cc98ff6ef5872c8e620
> >https://github.com/amir73il/linux/commit/0d1a226f5c18012f19ed6eabfab46f0f125ec190
> >
> >If you do this you need to separate your change into 2 patches.
> >First make the fanotify_data.flags change collecting the pieces from both
> >my patches.
> >Then make your TID change using fanotify_data.flags.
> >
> >Thanks,
> >Amir.
> >
> Should I do this:
> 1 git cherry-pick 5225fe1e19c74f4d7a4a4cc98ff6ef5872c8e620
> Git cherry-pick 0d1a226f5c18012f19ed6eabfab46f0f125ec190
> Manually handle patch conflicts
> 2 Add a new flag "FAN_EVENT_INFO_TID" into fanotify_data.flags
> 3 Incorporate "bool audit" into fanotify_data.flags
> 4 Submit 4 patches
>
No there is stuff in my patches that is irrelevant.
Let me prepare a clean patch for fanotify_data.flags and you can work
on top of that.
I'll try to post it later today.
Thanks,
Amir.