mbox series

[v3,0/7] fanotify events on child with name info

Message ID 20200505162014.10352-1-amir73il@gmail.com (mailing list archive)
Headers show
Series fanotify events on child with name info | expand

Message

Amir Goldstein May 5, 2020, 4:20 p.m. UTC
Jan,

In the v3 posting of the name info patches [1] I dropped the
FAN_REPORT_NAME patches as agreed to defer them to next cycle.

Following is remainder of the series to complement the FAN_DIR_MODIFY
patches that were merged to v5.7-rc1.

The v3 patches are available on my github branch fanotify_name [2].
Same branch names for LTP tests [3], man page draft [4] and a demo [5].

Patches 1-4 are cleanup and minor re-factoring in prep for the name
info patches.

Patch 5 adds the FAN_REPORT_NAME flag and the new event reporting format
combined of FAN_EVENT_INFO_TYPE_DFID_NAME and FAN_EVENT_INFO_TYPE_FID
info records, but provides not much added value beyond inotify.

Patches 6-7 add the new capability of filesystem/mount watch with events
including name info.

I have made an API decision that stems from consolidating the
implementation with fsnotify_parent() that requires your approval -
A filesystem/mount mark with FAN_REPORT_NAME behaves as if all the
directories and inodes are marked.  This results in user getting all
relevant events in two flavors - one with both info records and one with just
FAN_EVENT_INFO_TYPE_FID.  I have tries several approaches to work around this
bizarrity, but in the end I decided that would be the lesser evil and that
bizarre behavior is at least easy to document.

Let me know what you think.
Thanks,
Amir.

Main changes since v2:
- FAN_DIR_MODIFY patches have been merged
- A few more clean patches
- More text about the motivation (in "report parent fid + name" patch)
- Reduce code duplication with fsnotify_parent()

[1] https://lore.kernel.org/linux-fsdevel/20200319151022.31456-1-amir73il@gmail.com/
[2] https://github.com/amir73il/linux/commits/fanotify_name
[3] https://github.com/amir73il/ltp/commits/fanotify_name
[4] https://github.com/amir73il/man-pages/commits/fanotify_name
[5] https://github.com/amir73il/inotify-tools/commits/fanotify_name

Amir Goldstein (7):
  fanotify: create overflow event type
  fanotify: break up fanotify_alloc_event()
  fanotify: generalize the handling of extra event flags
  fanotify: distinguish between fid encode error and null fid
  fanotify: report parent fid + name for events on children
  fsnotify: send event "on child" to sb/mount marks
  fanotify: report events "on child" with name info to sb/mount marks

 fs/notify/fanotify/fanotify.c      | 213 +++++++++++++++++------------
 fs/notify/fanotify/fanotify.h      |  18 ++-
 fs/notify/fanotify/fanotify_user.c |  46 +++++--
 fs/notify/fsnotify.c               |  38 ++++-
 include/linux/fanotify.h           |   2 +-
 include/linux/fsnotify_backend.h   |  23 +++-
 include/uapi/linux/fanotify.h      |   4 +
 7 files changed, 231 insertions(+), 113 deletions(-)

Comments

Amir Goldstein May 18, 2020, 9:40 a.m. UTC | #1
On Tue, May 5, 2020 at 7:20 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> Jan,
>
> In the v3 posting of the name info patches [1] I dropped the
> FAN_REPORT_NAME patches as agreed to defer them to next cycle.
>
> Following is remainder of the series to complement the FAN_DIR_MODIFY
> patches that were merged to v5.7-rc1.
>
> The v3 patches are available on my github branch fanotify_name [2].
> Same branch names for LTP tests [3], man page draft [4] and a demo [5].
>
> Patches 1-4 are cleanup and minor re-factoring in prep for the name
> info patches.
>
> Patch 5 adds the FAN_REPORT_NAME flag and the new event reporting format
> combined of FAN_EVENT_INFO_TYPE_DFID_NAME and FAN_EVENT_INFO_TYPE_FID
> info records, but provides not much added value beyond inotify.
>
> Patches 6-7 add the new capability of filesystem/mount watch with events
> including name info.
>
> I have made an API decision that stems from consolidating the
> implementation with fsnotify_parent() that requires your approval -
> A filesystem/mount mark with FAN_REPORT_NAME behaves as if all the
> directories and inodes are marked.  This results in user getting all
> relevant events in two flavors - one with both info records and one with just
> FAN_EVENT_INFO_TYPE_FID.  I have tries several approaches to work around this
> bizarrity, but in the end I decided that would be the lesser evil and that
> bizarre behavior is at least easy to document.
>

Hi Jan,

Were you able to give some thought to the API question above?

I would really like to be able to finalize the design of the API, so
that I will be able to continue working on the man page patches.

Re-phrasing the API question that needs addressing:

With FAN_REPORT_NAME, filesystem/mount watches get -
1. ONLY events with name (no events on root and no SELF events)
2. Each event in one flavor (with name info when available)
3. ALL events in both flavors (where name info is available)
4. Something else?

The current v3 patches implement API choice #3, which is derived
from the implementation choice to emit two events via fsnotify_parent().

My v2 patches implemented API choice #2, which was the reason
for duplicating name snapshot code inside handle_event().
I considered implementing merge of event with and without name,
but it seemed too ugly to live.

We could also go for an API that allows any combination of
FAN_REPORT_NAME and FAN_REPORT_FID:
FAN_REPORT_FID - current upstream behavior
FAN_REPORT_FID_NAME - choice #3 above as implemented in v3
FAN_REPORT_NAME - choice #1 above

At one point, I also considered that user needs to opt-in
for named events per filesystem/mount mark with
FAN_EVENT_ON_CHILD. This flag is implied in v3 for
all filesystem/mount marks by FAN_REPORT_NAME, while
for directory marks it is opt-in as it has always been.

At the moment I went with choice #3 in v3 because I felt it
would be the simplest of all choices to document.

I didn't want to invest time in documenting behavior if you find it
unacceptable. If you are swaying between more than one option,
then I am willing to try documenting more than one option, so we
can see what the result looks like.

Thanks,
Amir.