At 05/18/2017 09:45 PM, David Sterba wrote:
On Thu, May 18, 2017 at 08:24:26AM +0800, Qu Wenruo wrote:
+static inline void extent_changeset_init(struct extent_changeset *changeset)
+{
+ changeset->bytes_changed = 0;
+ ulist_init(&changeset->range_changed);
+}
+
+static inline struct extent_changeset *extent_changeset_alloc(void)
+{
+ struct extent_changeset *ret;
+
+ ret = kmalloc(sizeof(*ret), GFP_KERNEL);
I don't remember if we'd discussed this before, but have you evaluated
if GFP_KERNEL is ok to use in this context?
IIRC you have informed me that I shouldn't abuse GFP_NOFS.
Use of GFP_NOFS or _KERNEL has to be evaluated case by case. So if it's
"let's use NOFS because everybody else does" or "he said I should not
use NOFS, then I'll use KERNEL", then it's wrong and I'll complain.
A short notice in the changelog or a comment above the allocation would
better signify that the patch author spent some time thinking about the
consequences.
Sometimes it can become pretty hard to find the potential deadlock
scenarios. Using GFP_NOFS in such case is a matter of precaution, but at
least would be nice to be explictly stated somewhere.
Yes it's hard to find such deadlock especially when lockdep will not
detect it.
And this makes the advantage of using stack memory in v3 patch more obvious.
I didn't realize the extra possible deadlock when memory pressure is
high, and to make completely correct usage of GFP_ flags we should let
caller to choose its GFP_ flag, which will introduce more modification
and more possibility to cause problem.
So now I prefer the stack version a little more.
Thanks,
Qu
The hard cases help to understand the callchain patterns and it's easier
to detect them in the future. For example, in your patch I already knew
that it's a problem when I saw lock_extent_bits, because I had seen this
pattern in a patch doing allocation in FIEMAP. Commit
afce772e87c36c7f07f230a76d525025aaf09e41, discussion in thread
http://lkml.kernel.org/r/1465362783-27078-1-git-send-email-lufq.f...@cn.fujitsu.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
