* Re: [PATCH] vmalloc: add warning in __vmalloc
2012-05-03 6:30 ` [PATCH] vmalloc: add warning in __vmalloc Nick Piggin
@ 2012-05-03 7:13 ` Artem Bityutskiy
2012-05-03 7:14 ` Nick Piggin
2012-05-03 13:48 ` Steven Whitehouse
1 sibling, 1 reply; 4+ messages in thread
From: Artem Bityutskiy @ 2012-05-03 7:13 UTC (permalink / raw)
To: Nick Piggin
Cc: Sage Weil, Minchan Kim, Andrew Morton, linux-mm, linux-kernel,
kosaki.motohiro, rientjes, Neil Brown, David Woodhouse,
Theodore Ts'o, Adrian Hunter, Steven Whitehouse,
David S. Miller, James Morris, Alexander Viro, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 548 bytes --]
On Thu, 2012-05-03 at 16:30 +1000, Nick Piggin wrote:
> Note that in writeback paths, a "good citizen" filesystem should not require
> any allocations, or at least it should be able to tolerate allocation failures.
> So fixing that would be a good idea anyway.
This is a good point, but UBIFS kmallocs(GFP_NOFS) when doing I/O
because it needs to compress/decompress. But I agree that if kmalloc
fails, we should have a fall-back reserve buffer protected by a mutex
for memory pressure situations.
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] vmalloc: add warning in __vmalloc
2012-05-03 6:30 ` [PATCH] vmalloc: add warning in __vmalloc Nick Piggin
2012-05-03 7:13 ` Artem Bityutskiy
@ 2012-05-03 13:48 ` Steven Whitehouse
1 sibling, 0 replies; 4+ messages in thread
From: Steven Whitehouse @ 2012-05-03 13:48 UTC (permalink / raw)
To: Nick Piggin
Cc: Sage Weil, Minchan Kim, Andrew Morton, linux-mm, linux-kernel,
kosaki.motohiro, rientjes, Neil Brown, Artem Bityutskiy,
David Woodhouse, Theodore Ts'o, Adrian Hunter,
David S. Miller, James Morris, Alexander Viro, linux-fsdevel
Hi,
On Thu, 2012-05-03 at 16:30 +1000, Nick Piggin wrote:
> On 3 May 2012 15:46, Sage Weil <sage@newdream.net> wrote:
> > On Thu, 3 May 2012, Minchan Kim wrote:
> >> On 05/03/2012 04:46 AM, Andrew Morton wrote:
> >> > Well. What are we actually doing here? Causing the kernel to spew a
> >> > warning due to known-buggy callsites, so that users will report the
> >> > warnings, eventually goading maintainers into fixing their stuff.
> >> >
> >> > This isn't very efficient :(
> >>
> >>
> >> Yes. I hope maintainers fix it before merging this.
> >>
> >> >
> >> > It would be better to fix that stuff first, then add the warning to
> >> > prevent reoccurrences. Yes, maintainers are very naughty and probably
> >> > do need cattle prods^W^W warnings to motivate them to fix stuff, but we
> >> > should first make an effort to get these things fixed without
> >> > irritating and alarming our users.
> >> >
> >> > Where are these offending callsites?
> >
> > Okay, maybe this is a stupid question, but: if an fs can't call vmalloc
> > with GFP_NOFS without risking deadlock, calling with GFP_KERNEL instead
> > doesn't fix anything (besides being more honest). This really means that
> > vmalloc is effectively off-limits for file systems in any
> > writeback-related path, right?
>
> Anywhere it cannot reenter the filesystem, yes. GFP_NOFS is effectively
> GFP_KERNEL when calling vmalloc.
>
> Note that in writeback paths, a "good citizen" filesystem should not require
> any allocations, or at least it should be able to tolerate allocation failures.
> So fixing that would be a good idea anyway.
For cluster filesystems, there is an additional issue. When we allocate
memory with GFP_KERNEL we might land up pushing inodes out of cache,
which can also mean deallocating them. That process involves taking
cluster locks, and so it is not valid to do this while holding another
cluster lock (since the locks may be taken in random order).
In the GFS2 use case for vmalloc, this is being done if kmalloc fails
and also if the memory required is too large for kmalloc (very unlikely,
but possible with very large directories). Also, it is being done under
a cluster lock (shared mode).
I recently looked back at the thread which resulted in that particular
vmalloc call being left there:
http://www.redhat.com/archives/cluster-devel/2010-July/msg00021.html
http://www.redhat.com/archives/cluster-devel/2010-July/msg00022.html
http://www.redhat.com/archives/cluster-devel/2010-July/msg00023.html
which reminded me of the problem. So this might not be so easy to
resolve...
Steve.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 4+ messages in thread