linux-kernel - Re: [RFC 1/2] fanotify: new event FAN_MODIFY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOQ4uxhOcvmE+-HKQP0kyH5Jsct8nBMaz60KHhKpDYnvOV8OPQ@mail.gmail.com>
Date:   Wed, 15 Mar 2017 00:30:00 +0200
From:   Amir Goldstein <amir73il@...il.com>
To:     Filip Štědronský <r.lkml@...narg.cz>
Cc:     linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Jan Kara <jack@...e.cz>,
        Alexander Viro <viro@...iv.linux.org.uk>
Subject: Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR

On Tue, Mar 14, 2017 at 4:48 PM, Filip Štědronský <r.lkml@...narg.cz> wrote:
> Hi,
>
> On Tue, Mar 14, 2017 at 03:55:19PM +0200, Amir Goldstein wrote:
>> Please let me know if that is sufficient for your needs
>> or if you need me to prepare a version that delivers filename events
>> without filename info, therefore allowing to merge filename events.
>>
>> Sounds to me like if you get an event FAN_DELETE, "aaa",
>> your implementation can update the db directly without having
>> to scan the directory, so it should be useful.
>> For your consideration.
>
> both approaches might be useful and there are tradeoffs. Direct updates
> save us from rescanning but give more events and more context switches.
>
> On the other hand, with an unlimited queue and well-mergable events I
> could e.g. sleep for five seconds each time after emptying the queue,
> thus giving the events more potential to merge and reducing context
> switches.
>
> But one risks blowing up the queue. However, I have some ideas.
> Basically we could implement some kind of "bulk read" mode for fanotify
> that would pass events to userspace only when
> (a) a given event count thresold is hit (e.g. half the queue limit
>     if queue is limited), or
> (b) the oldest event is older than a specified maximum latency. That
>     might vary from 1 second to 5 minutes depending on specific
>     application needs (e.g. background backup does not need to be
>     low-latency).
>
> This would greatly reduce extra context switches when watching
> a large portion (or whole) of the file system. All of this presumably
> could be implemented on top of your suggested patches.
>
> Either way, I suggest that implementing the nameless filename events
> is a good idea. I do not know whether they will be the best choice
> for my application but they probably will be useful for some
> applications.
>

Done and pushed all branches.
Added 2 new patches to the fanotify_dentry set:
ce0c223 fanotify: allow merge of filename events
cd872d1 fanotify: make filename info optional for filename events

You should test with branch fanotify_sb, using super block watch,
because filename events are not supported in mount scope.
You can follow user API used by my fanotify_demo.c program.

Tested with my fanotify_demo test
both without FAN_EVENT_INFO_NAME and without FAN_EVENT_INFO_FH.

Without FAN_EVENT_INFO_NAME more filename events are merged,
but in my test that runs fs ops by a script, most events do not get merged
because of different pid value.

I guess that for the filesync/indexer use case, we would need to add a new
FAN_EVENT_INFO_NOPID flag to loose the irrelevant pid information
in order to be able to merge a lot more filename event on a directory.

I also plan to add FAN_EVENT_INFO_NOFD, to get rid of redundant
fd (when INFO_FH is available) and then be able to drop the refcount
on the victim object.

Cheers,
Amir.