From: Jan Kara <jack@suse.cz>
To: Francesco Ruggeri <fruggeri@arista.com>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
amir73il@gmail.com, jack@suse.cz
Subject: Re: soft lockup in fanotify_read
Date: Tue, 14 Jul 2020 15:14:13 +0200 [thread overview]
Message-ID: <20200714131413.GJ23073@quack2.suse.cz> (raw)
In-Reply-To: <20200714025417.A25EB95C0339@us180.sjc.aristanetworks.com>
Hello!
On Mon 13-07-20 19:54:17, Francesco Ruggeri wrote:
> We are getting this soft lockup in fanotify_read.
> The reason is that this code does not seem to scale to cases where there
> are big bursts of events generated by fanotify_handle_event.
> fanotify_read acquires group->notification_lock for each event.
> fanotify_handle_event uses the lock to add one event, which also involves
> fanotify_merge, which scans the whole list trying to find an event to
> merge the new one with.
> In our case fanotify_read is invoked with a buffer big enough for 200
> events, and what happens is that every time fanotify_read dequeues an
> event and releases the lock, fanotify_handle_event adds several more,
> scanning a longer and longer list. This causes fanotify_read to wait
> longer and longer for the lock, and the soft lockup happens before
> fanotify_read can reach 200 events.
> Is it intentional for fanotify_read to acquire the lock for each event,
> rather than batching together a user buffer worth of events?
Thanks for report and the analysis. I agree what you describe is possible.
The locking is actually fine I think but you're correct that the merging
logic isn't ideal and for large amounts of queued events may be too slow.
We were already discussing with Amir how to speed it up but didn't end up
doing anything yet since the issue wasn't really pressing.
WRT fanotify_read() removing events from the list in batches: That's
certainly one possible optimization but (especially with recent extensions
to fanotify interface) it is difficult to tell how many events will
actually fit in the provided buffer so we'd have to provide a way to push
events back to the event queue which may get a bit tricky. And as I wrote
above I think the real problem is actually with fanotify merge logic which
ends up holding the notification_lock for too long...
But we may want to add cond_resched() into the loop in fanotify_read() as
that can currently take time proportinal to user-provided buffer which can
be a lot. That will probably silence the softlockup for you as well
(although it's not really fixing the underlying issue).
We'll have a look what we can do about this :)
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
next prev parent reply other threads:[~2020-07-14 13:14 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-14 2:54 Francesco Ruggeri
2020-07-14 13:10 ` Amir Goldstein
2020-07-14 13:14 ` Jan Kara [this message]
2020-07-14 16:57 ` Francesco Ruggeri
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200714131413.GJ23073@quack2.suse.cz \
--to=jack@suse.cz \
--cc=amir73il@gmail.com \
--cc=fruggeri@arista.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®