From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: Jan Kara <jack@suse.cz>
Cc: linux-ext4@vger.kernel.org, Ted Tso <tytso@mit.edu>,
Ross Zwisler <ross.zwisler@linux.intel.com>,
Dan Williams <dan.j.williams@intel.com>,
linux-fsdevel@vger.kernel.org, linux-nvdimm@lists.01.org
Subject: Re: [PATCH 0/2] ext4: Fix ENOSPC handling for DAX faults
Date: Fri, 15 Dec 2017 12:36:25 -0700 [thread overview]
Message-ID: <20171215193625.GA5454@linux.intel.com> (raw)
In-Reply-To: <20171213091352.23448-1-jack@suse.cz>
On Wed, Dec 13, 2017 at 10:13:50AM +0100, Jan Kara wrote:
> Hello,
>
> these two patches fix handling of ENOSPC during DAX faults. The problem is
> that currently running transaction may be holding lots of already freed
> blocks which can be reallocated only once the transaction commits. Standard
> retry logic in ext4_iomap_end() does not work for DAX page fault handler
> since we hold current transaction open in ext4_dax_huge_fault() and thus
> retry logic cannot force the running transaction and as a result application
> gets SIGBUS due to premature ENOSPC error.
>
> These two patches fix the problem. I'm not too happy about patch 1/2 passing
> additional info (error code) from the fault handler but I don't see an
> easy better option since we want to also pass back special return values
> like VM_FAULT_FALLBACK. Comments are welcome.
I also don't love having two error codes coming back out of the DAX fault
handlers. I'm worried that we'll end up forgetting to set errp in some cases,
and will only set the VM_FAULT_* error code.
I wonder if a better way to solve this would be to define a new
VM_FAULT_NOSPC, just like we have VM_FAULT_OOM for ENOMEM errors? Essentially
what it seems like we are saying is that the very general return of just
VM_FAULT_SIGBUS doesn't provide us enough information, and that being able to
distinguish that from ENOSPC errors would be useful.
With that flag we would have 2 choices:
1) Add VM_FAULT_NOSPC to the VM_FAULT_ERROR mask, and then update things like
mm_fault_error() appropriately so, like the other errors in this class, it
results in SIGBUS, or
2) Just always return (VM_FAULT_NOSPC|VM_FAULT_SIGBUS), which I think would
mean that you wouldn't need to alter mm_fault_error() et al.
Do either of these sound appealing?
next prev parent reply other threads:[~2017-12-15 19:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-13 9:13 [PATCH 0/2] ext4: Fix ENOSPC handling for DAX faults Jan Kara
[not found] ` <20171213091352.23448-1-jack-AlSwsSmVLrQ@public.gmane.org>
2017-12-13 9:13 ` [PATCH 1/2] dax: Pass detailed error code from dax_iomap_fault() Jan Kara
2017-12-13 9:13 ` [PATCH 2/2] ext4: Fix ENOSPC handling in DAX page fault handler Jan Kara
2017-12-15 19:36 ` Ross Zwisler [this message]
2017-12-19 14:30 ` [PATCH 0/2] ext4: Fix ENOSPC handling for DAX faults Jan Kara
[not found] ` <20171219143051.GA25754-4I4JzKEfoa/jFM9bn6wA6Q@public.gmane.org>
2017-12-19 16:42 ` Ross Zwisler
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=20171215193625.GA5454@linux.intel.com \
--to=ross.zwisler@linux.intel.com \
--cc=dan.j.williams@intel.com \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-nvdimm@lists.01.org \
--cc=tytso@mit.edu \
/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;
as well as URLs for NNTP newsgroup(s).