| Message ID | 20181003212539.2384-8-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | New fanotify event info API | expand |
On Thu 04-10-18 00:25:38, Amir Goldstein wrote: > In order to identify which thread triggered the event in a > multi-threaded program, add the FAN_EVENT_INFO_TID flag in fanotify_init > to opt-in for reporting the event creator's thread id information. > > Signed-off-by: nixiaoming <nixiaoming@huawei.com> > Signed-off-by: Amir Goldstein <amir73il@gmail.com> ... > diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h > index afddd7e0d5a1..05b696b4856b 100644 > --- a/include/linux/fanotify.h > +++ b/include/linux/fanotify.h > @@ -18,7 +18,10 @@ > #define FANOTIFY_CLASS_BITS (FAN_CLASS_NOTIF | FAN_CLASS_CONTENT | \ > FAN_CLASS_PRE_CONTENT) > > +#define FANOTIFY_EVENT_INFO_FLAGS (FAN_EVENT_INFO_TID) > + > #define FANOTIFY_INIT_FLAGS (FANOTIFY_CLASS_BITS | \ > + FANOTIFY_EVENT_INFO_FLAGS | \ > FAN_CLOEXEC | FAN_NONBLOCK | \ > FAN_UNLIMITED_QUEUE | FAN_UNLIMITED_MARKS) Is there reason to define FANOTIFY_EVENT_INFO_FLAGS? But I guess you prepare with this for further changes you want to do? Just that at this point it looks a bit silly and I cannot really think of a reason why we'd like to distinguish flags in FANOTIFY_EVENT_INFO_FLAGS from other flags in FANOTIFY_INIT_FLAGS... Honza
On Thu, Oct 4, 2018 at 11:46 AM Jan Kara <jack@suse.cz> wrote: > > On Thu 04-10-18 00:25:38, Amir Goldstein wrote: > > In order to identify which thread triggered the event in a > > multi-threaded program, add the FAN_EVENT_INFO_TID flag in fanotify_init > > to opt-in for reporting the event creator's thread id information. > > > > Signed-off-by: nixiaoming <nixiaoming@huawei.com> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > ... > > diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h > > index afddd7e0d5a1..05b696b4856b 100644 > > --- a/include/linux/fanotify.h > > +++ b/include/linux/fanotify.h > > @@ -18,7 +18,10 @@ > > #define FANOTIFY_CLASS_BITS (FAN_CLASS_NOTIF | FAN_CLASS_CONTENT | \ > > FAN_CLASS_PRE_CONTENT) > > > > +#define FANOTIFY_EVENT_INFO_FLAGS (FAN_EVENT_INFO_TID) > > + > > #define FANOTIFY_INIT_FLAGS (FANOTIFY_CLASS_BITS | \ > > + FANOTIFY_EVENT_INFO_FLAGS | \ > > FAN_CLOEXEC | FAN_NONBLOCK | \ > > FAN_UNLIMITED_QUEUE | FAN_UNLIMITED_MARKS) > > Is there reason to define FANOTIFY_EVENT_INFO_FLAGS? But I guess you > prepare with this for further changes you want to do? Just that at this > point it looks a bit silly and I cannot really think of a reason why we'd > like to distinguish flags in FANOTIFY_EVENT_INFO_FLAGS from other flags in > FANOTIFY_INIT_FLAGS... > No functional reason. Feel free to drop FANOTIFY_EVENT_INFO_FLAGS. Thanks, Amir.
On Thu 04-10-18 00:25:38, Amir Goldstein wrote: > In order to identify which thread triggered the event in a > multi-threaded program, add the FAN_EVENT_INFO_TID flag in fanotify_init > to opt-in for reporting the event creator's thread id information. > > Signed-off-by: nixiaoming <nixiaoming@huawei.com> > Signed-off-by: Amir Goldstein <amir73il@gmail.com> Just one question occurred to me (after discussion with a colleague about this feature): Ming, why do you actually need to know thread ID and thread-group ID is not enough? Also note that standard glibc threading functions are *not* going to be compatible with the ID returned from fanotify (e.g. it will be different from POSIX thread ID as returned by pthread_self()). So the feature is somewhat difficult to use from userspace... (at least you could use gettid() systemcall to get the ID of current thread but there's not glibc wrapper for it). Honza
On Thu, Oct 11, 2018 at 6:16 PM Jan Kara <mailto:jack@suse.cz> wrote: >On Thu 04-10-18 00:25:38, Amir Goldstein wrote: >> In order to identify which thread triggered the event in a >> multi-threaded program, add the FAN_EVENT_INFO_TID flag in fanotify_init >> to opt-in for reporting the event creator's thread id information. >> >> Signed-off-by: nixiaoming <nixiaoming@huawei.com> >> Signed-off-by: Amir Goldstein <amir73il@gmail.com> > >Just one question occurred to me (after discussion with a colleague about >this feature): Ming, why do you actually need to know thread ID and >thread-group ID is not enough? Also note that standard glibc threading >functions are *not* going to be compatible with the ID returned from >fanotify (e.g. it will be different from POSIX thread ID as returned by >pthread_self()). So the feature is somewhat difficult to use from >userspace... (at least you could use gettid() systemcall to get the ID of >current thread but there's not glibc wrapper for it). > When using fanotify to monitor system critical directories or files, the monitor and event trigger are not the same process. The monitoring task wants to know which task triggers the event. If the event is triggered by multiple threads, I hope to know which thread is triggered by the task, can't see the pthread id here, but I can see the tid. Thanks
On Fri 12-10-18 02:43:02, Nixiaoming wrote: > On Thu, Oct 11, 2018 at 6:16 PM Jan Kara <mailto:jack@suse.cz> wrote: > >On Thu 04-10-18 00:25:38, Amir Goldstein wrote: > >> In order to identify which thread triggered the event in a > >> multi-threaded program, add the FAN_EVENT_INFO_TID flag in fanotify_init > >> to opt-in for reporting the event creator's thread id information. > >> > >> Signed-off-by: nixiaoming <nixiaoming@huawei.com> > >> Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > > >Just one question occurred to me (after discussion with a colleague about > >this feature): Ming, why do you actually need to know thread ID and > >thread-group ID is not enough? Also note that standard glibc threading > >functions are *not* going to be compatible with the ID returned from > >fanotify (e.g. it will be different from POSIX thread ID as returned by > >pthread_self()). So the feature is somewhat difficult to use from > >userspace... (at least you could use gettid() systemcall to get the ID of > >current thread but there's not glibc wrapper for it). > > > > When using fanotify to monitor system critical directories or files, > the monitor and event trigger are not the same process. > The monitoring task wants to know which task triggers the event. > If the event is triggered by multiple threads, > I hope to know which thread is triggered by the task, > can't see the pthread id here, but I can see the tid. OK, thanks for clarification. Honza
diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index 361e3a0a445c..2c57186caa2e 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -25,7 +25,7 @@ static bool should_merge(struct fsnotify_event *old_fsn, old = FANOTIFY_E(old_fsn); new = FANOTIFY_E(new_fsn); - if (old_fsn->inode == new_fsn->inode && old->tgid == new->tgid && + if (old_fsn->inode == new_fsn->inode && old->pid == new->pid && old->path.mnt == new->path.mnt && old->path.dentry == new->path.dentry) return true; @@ -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 (FAN_GROUP_FLAG(group, FAN_EVENT_INFO_TID)) + event->pid = get_pid(task_pid(current)); + else + event->pid = get_pid(task_tgid(current)); if (path) { event->path = *path; path_get(&event->path); @@ -270,7 +273,7 @@ static void fanotify_free_event(struct fsnotify_event *fsn_event) event = FANOTIFY_E(fsn_event); path_put(&event->path); - put_pid(event->tgid); + put_pid(event->pid); if (fanotify_is_perm_event(fsn_event->mask)) { kmem_cache_free(fanotify_perm_event_cachep, FANOTIFY_PE(fsn_event)); diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h index 88a8290a61cb..ea05b8a401e7 100644 --- a/fs/notify/fanotify/fanotify.h +++ b/fs/notify/fanotify/fanotify.h @@ -19,7 +19,7 @@ struct fanotify_event_info { * during this object's lifetime */ struct path path; - struct pid *tgid; + struct pid *pid; }; /* diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 14594e491d2b..e03be5071362 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -132,7 +132,7 @@ static int fill_event_metadata(struct fsnotify_group *group, metadata->vers = FANOTIFY_METADATA_VERSION; metadata->reserved = 0; metadata->mask = fsn_event->mask & FANOTIFY_OUTGOING_EVENTS; - metadata->pid = pid_vnr(event->tgid); + metadata->pid = pid_vnr(event->pid); if (unlikely(fsn_event->mask & FAN_Q_OVERFLOW)) metadata->fd = FAN_NOFD; else { @@ -944,7 +944,7 @@ COMPAT_SYSCALL_DEFINE6(fanotify_mark, */ static int __init fanotify_user_setup(void) { - BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 6); + BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 7); BUILD_BUG_ON(HWEIGHT32(FANOTIFY_MARK_FLAGS) != 9); fanotify_mark_cache = KMEM_CACHE(fsnotify_mark, diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h index afddd7e0d5a1..05b696b4856b 100644 --- a/include/linux/fanotify.h +++ b/include/linux/fanotify.h @@ -18,7 +18,10 @@ #define FANOTIFY_CLASS_BITS (FAN_CLASS_NOTIF | FAN_CLASS_CONTENT | \ FAN_CLASS_PRE_CONTENT) +#define FANOTIFY_EVENT_INFO_FLAGS (FAN_EVENT_INFO_TID) + #define FANOTIFY_INIT_FLAGS (FANOTIFY_CLASS_BITS | \ + FANOTIFY_EVENT_INFO_FLAGS | \ FAN_CLOEXEC | FAN_NONBLOCK | \ FAN_UNLIMITED_QUEUE | FAN_UNLIMITED_MARKS) diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h index d0c05de670ef..00b2304ed124 100644 --- a/include/uapi/linux/fanotify.h +++ b/include/uapi/linux/fanotify.h @@ -40,6 +40,9 @@ #define FAN_UNLIMITED_MARKS 0x00000020 #define FAN_ENABLE_AUDIT 0x00000040 +/* Flags to determine fanotify event format */ +#define FAN_EVENT_INFO_TID 0x00000100 /* event->pid is thread id */ + /* Deprecated - do not use this in programs and do not add new flags here! */ #define FAN_ALL_INIT_FLAGS (FAN_CLOEXEC | FAN_NONBLOCK | \ FAN_ALL_CLASS_BITS | FAN_UNLIMITED_QUEUE |\