* [PATCH v2] fanotify reports the thread id of the event trigger
@ 2018-09-17 14:19 nixiaoming
2018-09-17 15:51 ` Amir Goldstein
0 siblings, 1 reply; 6+ messages in thread
From: nixiaoming @ 2018-09-17 14:19 UTC (permalink / raw)
To: jack, amir73il; +Cc: nixiaoming, linux-fsdevel, linux-kernel
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 <nixiaoming@huawei.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] fanotify reports the thread id of the event trigger
2018-09-17 14:19 [PATCH v2] fanotify reports the thread id of the event trigger nixiaoming
@ 2018-09-17 15:51 ` Amir Goldstein
2018-09-18 3:01 ` Nixiaoming
0 siblings, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2018-09-17 15:51 UTC (permalink / raw)
To: nixiaoming; +Cc: Jan Kara, linux-fsdevel, linux-kernel
On Mon, Sep 17, 2018 at 6:05 PM nixiaoming <nixiaoming@huawei.com> 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 <nixiaoming@huawei.com>
> ---
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH v2] fanotify reports the thread id of the event trigger
2018-09-17 15:51 ` Amir Goldstein
@ 2018-09-18 3:01 ` Nixiaoming
2018-09-18 7:06 ` Amir Goldstein
0 siblings, 1 reply; 6+ messages in thread
From: Nixiaoming @ 2018-09-18 3:01 UTC (permalink / raw)
To: Amir Goldstein, Steve Grubb; +Cc: Jan Kara, linux-fsdevel, linux-kernel
On Mon, Sep 17, 2018 11:51 PM Amir Goldstein <amir73il@gmail.com> wrote:
>On Mon, Sep 17, 2018 at 6:05 PM nixiaoming <nixiaoming@huawei.com> 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 <nixiaoming@huawei.com>
.......
>> @@ -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.
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] fanotify reports the thread id of the event trigger
2018-09-18 3:01 ` Nixiaoming
@ 2018-09-18 7:06 ` Amir Goldstein
2018-09-18 9:50 ` Nixiaoming
0 siblings, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2018-09-18 7:06 UTC (permalink / raw)
To: nixiaoming; +Cc: Steve Grubb, Jan Kara, linux-fsdevel, linux-kernel
On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming <nixiaoming@huawei.com> wrote:
>
> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming <nixiaoming@huawei.com> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH v2] fanotify reports the thread id of the event trigger
2018-09-18 7:06 ` Amir Goldstein
@ 2018-09-18 9:50 ` Nixiaoming
2018-09-18 12:02 ` Amir Goldstein
0 siblings, 1 reply; 6+ messages in thread
From: Nixiaoming @ 2018-09-18 9:50 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Steve Grubb, Jan Kara, linux-fsdevel, linux-kernel
On Tue, Sep 18, 2018 3:07 PM Amir Goldstein <amir73il@gmail.com>
>On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming <nixiaoming@huawei.com> wrote:
>>
>> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein <amir73il@gmail.com> wrote:
>> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming <nixiaoming@huawei.com> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] fanotify reports the thread id of the event trigger
2018-09-18 9:50 ` Nixiaoming
@ 2018-09-18 12:02 ` Amir Goldstein
0 siblings, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2018-09-18 12:02 UTC (permalink / raw)
To: nixiaoming; +Cc: Steve Grubb, Jan Kara, linux-fsdevel, linux-kernel
On Tue, Sep 18, 2018 at 12:50 PM Nixiaoming <nixiaoming@huawei.com> wrote:
>
> On Tue, Sep 18, 2018 3:07 PM Amir Goldstein <amir73il@gmail.com>
> >On Tue, Sep 18, 2018 at 6:01 AM Nixiaoming <nixiaoming@huawei.com> wrote:
> >>
> >> On Mon, Sep 17, 2018 11:51 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >> >On Mon, Sep 17, 2018 at 6:05 PM nixiaoming <nixiaoming@huawei.com> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-18 12:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-17 14:19 [PATCH v2] fanotify reports the thread id of the event trigger nixiaoming
2018-09-17 15:51 ` Amir Goldstein
2018-09-18 3:01 ` Nixiaoming
2018-09-18 7:06 ` Amir Goldstein
2018-09-18 9:50 ` Nixiaoming
2018-09-18 12:02 ` Amir Goldstein
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).