Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kmem rework #2411

Closed
wants to merge 3 commits into from
Closed

Kmem rework #2411

wants to merge 3 commits into from

Conversation

ryao
Copy link
Contributor

@ryao ryao commented Jun 20, 2014

This is a companion for openzfs/spl#369.

@ryao ryao changed the title Use Linux kzalloc()/kfree() for superblocks kmem rework Jul 14, 2014
@ryao
Copy link
Contributor Author

ryao commented Jul 14, 2014

I have just refreshed this and openzfs/spl#369 with the changes that I described in #2390 as well as the DMU transaction processing fix that I described on the Open ZFS mailing list. The latter of the two have some trivial merge conflicts with #2484.

ryao added 3 commits July 14, 2014 17:57
We should have included sys/taskq.h directly because we use the taskq
code here, but we instead had files that included sys/taskq.h also
include sys/kmem.h, which happened to include sys/taskq.h. sys/kmem.h no
longer does this, so we must define the include as we should
have done in the first place.

Signed-off-by: Richard Yao <ryao@gentoo.org>
We store the bdi in the superblock. This is accessed using virt_to_page
in bit_waitqueue(), which does not work on kernel virtual memory. We
switch to kzalloc()/kfree() to fix this.

Signed-off-by: Richard Yao <ryao@gentoo.org>
It was thought that we could ensure forward progress by avoiding memory
allocations during transaction processing and avoiding KM_SLEEP in the
few instances in which we could not.

Recent tests involving L2ARC on systems without swap have shown that
this is not enough. VMWare's bencharmk on a system with 8GB of RAM and
200GB of L2ARC will reliably deadlock when atime is enabled, but will
run fine when atime is not enabled. This was initially thought to be a
mix of an extreme case of ARC contention and atime updates somehow
blocking on transaction group commit, but further examination showed
that only 137MB of RAM were in use by L2ARC's headers and additional
testing showed no deadlocks occurred with 24GB of RAM when atime was
enabled.

Subsequent review of the code revealed that the only change was that
direct reclaim no longer started transactions to perform atime updates.
The logical conclusion is that direct reclaim can occur in locations
that hold locks that block an open transaction through some unforeseen
dependency chain. This patch attempts to address that by modifying the
KM_SLEEP avoidance mechanism to be used in all transaction processing,
rather than just operations on zvols. We also take the opportunity to do
some cleanup by replacing PF_NOIO with PF_FSTRANS, which is the proper
Linux flag for this.

Signed-off-by: Richard Yao <ryao@gentoo.org>
@ryao ryao changed the title kmem rework Kmem rework Jul 30, 2014
@behlendorf
Copy link
Contributor

@ryao I've pulled the following commit in to master and closed this issue. When you get a chance go ahead and open a new issue with the refreshed PF_FSTRANS patch.

194e562 Include sys/taskq.h in linux/vfs_compat.h

@behlendorf behlendorf added this to the 0.6.4 milestone Aug 14, 2014
@ryao ryao mentioned this pull request Sep 15, 2014
ryao added a commit to ryao/zfs that referenced this pull request Nov 29, 2014
We should have included sys/taskq.h directly because we use the taskq
code here, but we instead had files that included sys/taskq.h also
include sys/kmem.h, which happened to include sys/taskq.h. sys/kmem.h no
longer does this, so we must define the include as we should
have done in the first place.

Signed-off-by: Richard Yao <ryao@gentoo.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#2411
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Building Indicates an issue related to building binaries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants