Re: [PATCH v2] fanotify reports the thread id of the event trigger

2018-09-18 Thread Amir Goldstein
On Tue, Sep 18, 2018 at 12:50 PM Nixiaoming  wrote:
>
> On Tue, Sep 18, 2018 3:07 PM Amir Goldstein 
> >On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming  wrote:
> >>
> >> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein  wrote:
> >> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming  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.


Re: [PATCH v2] fanotify reports the thread id of the event trigger

2018-09-18 Thread Amir Goldstein
On Tue, Sep 18, 2018 at 12:50 PM Nixiaoming  wrote:
>
> On Tue, Sep 18, 2018 3:07 PM Amir Goldstein 
> >On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming  wrote:
> >>
> >> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein  wrote:
> >> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming  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.


RE: [PATCH v2] fanotify reports the thread id of the event trigger

2018-09-18 Thread Nixiaoming
On Tue, Sep 18, 2018 3:07 PM Amir Goldstein 
>On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming  wrote:
>>
>> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein  wrote:
>> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming  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


RE: [PATCH v2] fanotify reports the thread id of the event trigger

2018-09-18 Thread Nixiaoming
On Tue, Sep 18, 2018 3:07 PM Amir Goldstein 
>On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming  wrote:
>>
>> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein  wrote:
>> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming  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


Re: [PATCH v2] fanotify reports the thread id of the event trigger

2018-09-18 Thread Amir Goldstein
On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming  wrote:
>
> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein  wrote:
> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming  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.


Re: [PATCH v2] fanotify reports the thread id of the event trigger

2018-09-18 Thread Amir Goldstein
On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming  wrote:
>
> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein  wrote:
> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming  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.


RE: [PATCH v2] fanotify reports the thread id of the event trigger

2018-09-17 Thread Nixiaoming
On Mon, Sep 17, 2018 11:51 PM Amir Goldstein  wrote:
>On Mon, Sep 17, 2018 at 6:05 PM nixiaoming  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 
...
>> @@ -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(>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  0x4000  /* event occurred against 
>> dir */
>>
>> +#define FAN_EVENT_INFO_TID 0x0200  /* 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 0x0010
>#define FAN_UNLIMITED_MARKS 0x0020
>#define FAN_ENABLE_AUDIT0x0040
>+#define FAN_EVENT_INFO_TID 0x0080
>
>Thanks,
>Amir.
>


RE: [PATCH v2] fanotify reports the thread id of the event trigger

2018-09-17 Thread Nixiaoming
On Mon, Sep 17, 2018 11:51 PM Amir Goldstein  wrote:
>On Mon, Sep 17, 2018 at 6:05 PM nixiaoming  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 
...
>> @@ -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(>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  0x4000  /* event occurred against 
>> dir */
>>
>> +#define FAN_EVENT_INFO_TID 0x0200  /* 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 0x0010
>#define FAN_UNLIMITED_MARKS 0x0020
>#define FAN_ENABLE_AUDIT0x0040
>+#define FAN_EVENT_INFO_TID 0x0080
>
>Thanks,
>Amir.
>


Re: [PATCH v2] fanotify reports the thread id of the event trigger

2018-09-17 Thread Amir Goldstein
On Mon, Sep 17, 2018 at 6:05 PM nixiaoming  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 
> ---
>  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(>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(>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(>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  0x4000  /* event occurred against dir 
> */
>
> +#define FAN_EVENT_INFO_TID 0x0200  /* 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 0x0010
#define FAN_UNLIMITED_MARKS 0x0020
#define FAN_ENABLE_AUDIT0x0040
+#define FAN_EVENT_INFO_TID 0x0080

Thanks,
Amir.


Re: [PATCH v2] fanotify reports the thread id of the event trigger

2018-09-17 Thread Amir Goldstein
On Mon, Sep 17, 2018 at 6:05 PM nixiaoming  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 
> ---
>  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(>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(>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(>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  0x4000  /* event occurred against dir 
> */
>
> +#define FAN_EVENT_INFO_TID 0x0200  /* 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 0x0010
#define FAN_UNLIMITED_MARKS 0x0020
#define FAN_ENABLE_AUDIT0x0040
+#define FAN_EVENT_INFO_TID 0x0080

Thanks,
Amir.