linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* fsnotify: Question on proper use in-kernel
@ 2023-03-17  3:19 Ioannis Zarkadas
  2023-03-17  9:45 ` Amir Goldstein
  0 siblings, 1 reply; 2+ messages in thread
From: Ioannis Zarkadas @ 2023-03-17  3:19 UTC (permalink / raw)
  To: linux-fsdevel

Hi everyone!

I'm developing a kernel module and trying to setup a directory watch with fsnotify.
I mainly copied existing code paths in the kernel that I found, because I couldn't
find any usage documentation.
I am using Linux Kernel version 5.12.0.

My issue is the following:
- Setting up the watch works initially.
- If I remove and reinsert the kernel module, fsnotify_add_inode_mark fails with EEXIST.

So I must be doing something wrong. I am using the put/destroy methods for the mark
and the put method for the group when unloading the module.

Here is how I setup the watch:

> static struct fsnotify_group *group;
> static struct fsnotify_mark mark;
>
> static int setup_sync_dir_watch(char *sync_dir) {
>     int ret;
>     struct fsnotify_mark *old_mark;
>     struct path sync_dir_path;
>
>     pr_info("%s: Syncing extents for files under '%s'\n", MODULE_NAME,
>             sync_dir);
>
>     group = fsnotify_alloc_group(&nvmeof_xrp_fsnotify_ops);
>     if (IS_ERR(group)) {
>         pr_err("%s: Error allocating fsnotify group!\n", MODULE_NAME);
>         return -1;
>     }
>     ret = kern_path(sync_dir, LOOKUP_FOLLOW, &sync_dir_path);
>     if (ret) {
>         pr_err("%s: Error getting kernel path: %d!\n", MODULE_NAME, ret);
>         goto release_group;
>     }
>     fsnotify_init_mark(&mark, group);
>     mark.mask = FS_CREATE | FS_DELETE | FS_MODIFY |
>                                  FS_CLOSE_WRITE | FS_EVENT_ON_CHILD;
>     ret = fsnotify_add_inode_mark(&mark,
>                                   sync_dir_path.dentry->d_inode, 0);
>     path_put(&sync_dir_path);
>     if (ret) {
>         pr_err("%s: Error adding fsnotify mark! Error code: %d\n", MODULE_NAME,
>                ret);
>         goto release_mark;
>     }
>     return 0;
> release_mark:
>     fsnotify_destroy_mark(&mark, group);
>     fsnotify_put_mark(&mark);
> release_group:
>     fsnotify_put_group(group);
>     return ret;
> }

And here is how I clear it when exiting the module:

> static void __exit module_exit(void) {
>     fsnotify_destroy_mark(&mark, group);
>     fsnotify_put_mark(&mark);
>     fsnotify_put_group(group);

I also tried to find the mark and delete it, but it returns NULL even though
fsnotify_add_inode_mark returns EEXISTS:

> mutex_lock(&group->mark_mutex);
> old_mark = fsnotify_find_mark(
>     &sync_dir_path.dentry->d_inode->i_fsnotify_marks,
>     group);
> if (old_mark != NULL) {
>     pr_info("%s: Found old mark, destroying it...\n", MODULE_NAME);
>     fsnotify_destroy_mark(old_mark, group);
>     fsnotify_put_mark(old_mark);
> }
> mutex_unlock(&group->mark_mutex);

Thanks in advance,
Yannis

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: fsnotify: Question on proper use in-kernel
  2023-03-17  3:19 fsnotify: Question on proper use in-kernel Ioannis Zarkadas
@ 2023-03-17  9:45 ` Amir Goldstein
  0 siblings, 0 replies; 2+ messages in thread
From: Amir Goldstein @ 2023-03-17  9:45 UTC (permalink / raw)
  To: Ioannis Zarkadas; +Cc: linux-fsdevel, Jan Kara

On Fri, Mar 17, 2023 at 5:23 AM Ioannis Zarkadas <iz2175@columbia.edu> wrote:
>
> Hi everyone!
>
> I'm developing a kernel module and trying to setup a directory watch with fsnotify.
> I mainly copied existing code paths in the kernel that I found, because I couldn't
> find any usage documentation.
> I am using Linux Kernel version 5.12.0.

I guess the API should be similar to upstream, but I did not check.

>
> My issue is the following:
> - Setting up the watch works initially.
> - If I remove and reinsert the kernel module, fsnotify_add_inode_mark fails with EEXIST.
>
> So I must be doing something wrong. I am using the put/destroy methods for the mark
> and the put method for the group when unloading the module.
>
> Here is how I setup the watch:
>
> > static struct fsnotify_group *group;
> > static struct fsnotify_mark mark;
> >
> > static int setup_sync_dir_watch(char *sync_dir) {
> >     int ret;
> >     struct fsnotify_mark *old_mark;
> >     struct path sync_dir_path;
> >
> >     pr_info("%s: Syncing extents for files under '%s'\n", MODULE_NAME,
> >             sync_dir);
> >
> >     group = fsnotify_alloc_group(&nvmeof_xrp_fsnotify_ops);

You did not mention what your fsnotify_ops contain.

> >     if (IS_ERR(group)) {
> >         pr_err("%s: Error allocating fsnotify group!\n", MODULE_NAME);
> >         return -1;
> >     }
> >     ret = kern_path(sync_dir, LOOKUP_FOLLOW, &sync_dir_path);
> >     if (ret) {
> >         pr_err("%s: Error getting kernel path: %d!\n", MODULE_NAME, ret);
> >         goto release_group;
> >     }
> >     fsnotify_init_mark(&mark, group);
> >     mark.mask = FS_CREATE | FS_DELETE | FS_MODIFY |
> >                                  FS_CLOSE_WRITE | FS_EVENT_ON_CHILD;
> >     ret = fsnotify_add_inode_mark(&mark,
> >                                   sync_dir_path.dentry->d_inode, 0);
> >     path_put(&sync_dir_path);
> >     if (ret) {
> >         pr_err("%s: Error adding fsnotify mark! Error code: %d\n", MODULE_NAME,
> >                ret);
> >         goto release_mark;
> >     }
> >     return 0;
> > release_mark:
> >     fsnotify_destroy_mark(&mark, group);
> >     fsnotify_put_mark(&mark);

Don't think you need that put.

> > release_group:
> >     fsnotify_put_group(group);

fsnotify_destroy_group()

> >     return ret;
> > }
>
> And here is how I clear it when exiting the module:
>
> > static void __exit module_exit(void) {
> >     fsnotify_destroy_mark(&mark, group);
> >     fsnotify_put_mark(&mark);
> >     fsnotify_put_group(group);
>

I think you only need fsnotify_destroy_group().
Internally, it will detach the mark, drop its reference and
fsnotify_wait_marks_destroyed(), because if there are live
reference to mark, you cannot remove the module.

> I also tried to find the mark and delete it, but it returns NULL even though
> fsnotify_add_inode_mark returns EEXISTS:
>
> > mutex_lock(&group->mark_mutex);
> > old_mark = fsnotify_find_mark(
> >     &sync_dir_path.dentry->d_inode->i_fsnotify_marks,
> >     group);
> > if (old_mark != NULL) {
> >     pr_info("%s: Found old mark, destroying it...\n", MODULE_NAME);
> >     fsnotify_destroy_mark(old_mark, group);
> >     fsnotify_put_mark(old_mark);
> > }
> > mutex_unlock(&group->mark_mutex);
>

Not really sure what happened here.
If this behavior persists after making the changes above, better add debug
prints to fsnotify_add_mark_list() to understand what is happening.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-03-17  9:45 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-17  3:19 fsnotify: Question on proper use in-kernel Ioannis Zarkadas
2023-03-17  9:45 ` 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).