Hi Andrew,
After merging the akpm-current tree, today's linux-next build (powerpc
allnoconfig) failed like this:
mm/built-in.o: In function `shrink_slab.part.73.constprop.83':
vmscan.c:(.text+0xf760): undefined reference to `__srcu_read_lock'
vmscan.c:(.text+0xf924): undefined reference to `__srcu_read_unlock'
mm/built-in.o: In function `unregister_shrinker':
(.text+0xfa60): undefined reference to `synchronize_srcu'
mm/built-in.o:(.data+0x1e8): undefined reference to `process_srcu'
Caused by commit
dab937da82f9 ("mm: srcu-ify shrinkers")
I have reverted that commit for today.
--
Cheers,
Stephen Rothwell [email protected]
On Fri, 2015-07-24 at 15:33 +1000, Stephen Rothwell wrote:
> Hi Andrew,
>
> After merging the akpm-current tree, today's linux-next build (powerpc
> allnoconfig) failed like this:
>
> mm/built-in.o: In function `shrink_slab.part.73.constprop.83':
> vmscan.c:(.text+0xf760): undefined reference to `__srcu_read_lock'
> vmscan.c:(.text+0xf924): undefined reference to `__srcu_read_unlock'
> mm/built-in.o: In function `unregister_shrinker':
> (.text+0xfa60): undefined reference to `synchronize_srcu'
> mm/built-in.o:(.data+0x1e8): undefined reference to `process_srcu'
>
> Caused by commit
>
> dab937da82f9 ("mm: srcu-ify shrinkers")
>
> I have reverted that commit for today.
Adding paulmck.
I'm not entirely sure what is the fix here. Paul G also reported it
failing for arm. I was able to reproduce with powerpc, and the following
fixes the issue, but I doubt it is the correct way to address this. The
idea was based on how we do it for x86.
Also, having SRCU in mm is, lets say, more than convenient, beyond
MMU_NOTIFIERS which explicitly selects SRCU.
Paul?
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 5ef2711..774207e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -124,6 +124,7 @@ config PPC
select GENERIC_IRQ_SHOW
select GENERIC_IRQ_SHOW_LEVEL
select IRQ_FORCED_THREADING
+ select SRCU
select HAVE_RCU_TABLE_FREE if SMP
select HAVE_SYSCALL_TRACEPOINTS
select HAVE_BPF_JIT
Thanks,
Davidlohr
On Fri, Jul 24, 2015 at 01:16:05PM -0700, Davidlohr Bueso wrote:
> On Fri, 2015-07-24 at 15:33 +1000, Stephen Rothwell wrote:
> > Hi Andrew,
> >
> > After merging the akpm-current tree, today's linux-next build (powerpc
> > allnoconfig) failed like this:
> >
> > mm/built-in.o: In function `shrink_slab.part.73.constprop.83':
> > vmscan.c:(.text+0xf760): undefined reference to `__srcu_read_lock'
> > vmscan.c:(.text+0xf924): undefined reference to `__srcu_read_unlock'
> > mm/built-in.o: In function `unregister_shrinker':
> > (.text+0xfa60): undefined reference to `synchronize_srcu'
> > mm/built-in.o:(.data+0x1e8): undefined reference to `process_srcu'
> >
> > Caused by commit
> >
> > dab937da82f9 ("mm: srcu-ify shrinkers")
> >
> > I have reverted that commit for today.
>
> Adding paulmck.
>
> I'm not entirely sure what is the fix here. Paul G also reported it
> failing for arm. I was able to reproduce with powerpc, and the following
> fixes the issue, but I doubt it is the correct way to address this. The
> idea was based on how we do it for x86.
>
> Also, having SRCU in mm is, lets say, more than convenient, beyond
> MMU_NOTIFIERS which explicitly selects SRCU.
>
> Paul?
SRCU was made optional as part of the kernel tinification project,
so adding Josh on CC. The hope would be that the feature needing SRCU
could add the "select" rather than having major architectures doing so.
I guess if SRCU is needed everywhere, it is needed everywhere, but...
Thanx, Paul
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 5ef2711..774207e 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -124,6 +124,7 @@ config PPC
> select GENERIC_IRQ_SHOW
> select GENERIC_IRQ_SHOW_LEVEL
> select IRQ_FORCED_THREADING
> + select SRCU
> select HAVE_RCU_TABLE_FREE if SMP
> select HAVE_SYSCALL_TRACEPOINTS
> select HAVE_BPF_JIT
>
> Thanks,
> Davidlohr
>
On Fri, Jul 24, 2015 at 04:09:02PM -0700, Paul E. McKenney wrote:
> On Fri, Jul 24, 2015 at 01:16:05PM -0700, Davidlohr Bueso wrote:
> > On Fri, 2015-07-24 at 15:33 +1000, Stephen Rothwell wrote:
> > > Hi Andrew,
> > >
> > > After merging the akpm-current tree, today's linux-next build (powerpc
> > > allnoconfig) failed like this:
> > >
> > > mm/built-in.o: In function `shrink_slab.part.73.constprop.83':
> > > vmscan.c:(.text+0xf760): undefined reference to `__srcu_read_lock'
> > > vmscan.c:(.text+0xf924): undefined reference to `__srcu_read_unlock'
> > > mm/built-in.o: In function `unregister_shrinker':
> > > (.text+0xfa60): undefined reference to `synchronize_srcu'
> > > mm/built-in.o:(.data+0x1e8): undefined reference to `process_srcu'
> > >
> > > Caused by commit
> > >
> > > dab937da82f9 ("mm: srcu-ify shrinkers")
> > >
> > > I have reverted that commit for today.
> >
> > Adding paulmck.
> >
> > I'm not entirely sure what is the fix here. Paul G also reported it
> > failing for arm. I was able to reproduce with powerpc, and the following
> > fixes the issue, but I doubt it is the correct way to address this. The
> > idea was based on how we do it for x86.
> >
> > Also, having SRCU in mm is, lets say, more than convenient, beyond
> > MMU_NOTIFIERS which explicitly selects SRCU.
> >
> > Paul?
>
> SRCU was made optional as part of the kernel tinification project,
> so adding Josh on CC. The hope would be that the feature needing SRCU
> could add the "select" rather than having major architectures doing so.
>
> I guess if SRCU is needed everywhere, it is needed everywhere, but...
I certainly agree that it doesn't make sense to make all architectures
select SRCU, if an unremovable core kernel feature uses SRCU. If
possible, I'd really like to avoid seeing SRCU become mandatory again,
though.
Is there any chance at all of the shrinker mechanism becoming optional?
At first glance, it seems reasonably separate from the rest of mm, in
that if it didn't exist and shrinking didn't happen, the rest of mm
still works. If that happened, MM_SHRINKER could select SRCU.
If that's not possible, then for the moment, I'd suggest making a hidden
symbol MM_SHRINKER that's always y and does "select SRCU", to preserve
SRCU's modularity for the moment while not forcing every architecture to
select it.
- Josh Triplett
On Sat, 2015-07-25 at 12:47 -0700, Josh Triplett wrote:
> I certainly agree that it doesn't make sense to make all architectures
> select SRCU, if an unremovable core kernel feature uses SRCU. If
> possible, I'd really like to avoid seeing SRCU become mandatory again,
> though.
I find it very strange that srcu is not taken for granted like rcu is,
or even regular locking primitives. How much overhead does srcu add?
> Is there any chance at all of the shrinker mechanism becoming optional?
> At first glance, it seems reasonably separate from the rest of mm, in
> that if it didn't exist and shrinking didn't happen, the rest of mm
> still works. If that happened, MM_SHRINKER could select SRCU.
Some mm functionality might very possibly rely on srcu in the future if
we expect any chances of scaling, ie: faults. So I'd rather not take a
short term solution here, as we'll probably be discussing this again
otherwise.
> If that's not possible, then for the moment, I'd suggest making a hidden
> symbol MM_SHRINKER that's always y and does "select SRCU", to preserve
> SRCU's modularity for the moment while not forcing every architecture to
> select it.
This is _very_ hacking. While tinyfication has its uses and
applications, I'd rather not have it in the way of normal kernels.
Thanks,
Davidlohr
On Sat, Jul 25, 2015 at 02:24:02PM -0700, Davidlohr Bueso wrote:
> On Sat, 2015-07-25 at 12:47 -0700, Josh Triplett wrote:
> > I certainly agree that it doesn't make sense to make all architectures
> > select SRCU, if an unremovable core kernel feature uses SRCU. If
> > possible, I'd really like to avoid seeing SRCU become mandatory again,
> > though.
>
> I find it very strange that srcu is not taken for granted like rcu is,
> or even regular locking primitives. How much overhead does srcu add?
About 2k. (For scale, note that a tinyconfig kernel is currently on the
order of 700-800k, so that's an appreciable fraction of the kernel.)
> > Is there any chance at all of the shrinker mechanism becoming optional?
> > At first glance, it seems reasonably separate from the rest of mm, in
> > that if it didn't exist and shrinking didn't happen, the rest of mm
> > still works. If that happened, MM_SHRINKER could select SRCU.
>
> Some mm functionality might very possibly rely on srcu in the future if
> we expect any chances of scaling, ie: faults. So I'd rather not take a
> short term solution here, as we'll probably be discussing this again
> otherwise.
What other mm functionality plans to use SRCU?
Among other things, no-mmu builds might still be able to omit it.
> > If that's not possible, then for the moment, I'd suggest making a hidden
> > symbol MM_SHRINKER that's always y and does "select SRCU", to preserve
> > SRCU's modularity for the moment while not forcing every architecture to
> > select it.
>
> This is _very_ hacking. While tinyfication has its uses and
> applications, I'd rather not have it in the way of normal kernels.
Thinking about it further, the better alternative if SRCU can't be kept
optional CONFIG_SRCU "default y" and hidden, so that it doesn't get
disabled by tinyconfig.
- Josh Triplett
On Sat, 25 Jul 2015 15:35:24 -0700 Josh Triplett <[email protected]> wrote:
> > Some mm functionality might very possibly rely on srcu in the future if
> > we expect any chances of scaling, ie: faults. So I'd rather not take a
> > short term solution here, as we'll probably be discussing this again
> > otherwise.
>
> What other mm functionality plans to use SRCU?
>
> Among other things, no-mmu builds might still be able to omit it.
Yup.
It's pretty trivial to make the shrinker srcuification be a
Kconfigurable thing. A few little helper functions and we're done.
That way, non-SMP kernels can use the plain old rwsem if so desired.
otoh it's better to use the same mechanism on all kernels for reasons
of testing coverage, maintenance cost, etc.
The mm-srcu-ify-shrinkers.patch changelog is suspiciously lacking in
evidence-of-benefit. We could just drop it?
On Mon, 2015-07-27 at 13:03 -0700, Andrew Morton wrote:
> On Sat, 25 Jul 2015 15:35:24 -0700 Josh Triplett <[email protected]> wrote:
>
> > > Some mm functionality might very possibly rely on srcu in the future if
> > > we expect any chances of scaling, ie: faults. So I'd rather not take a
> > > short term solution here, as we'll probably be discussing this again
> > > otherwise.
> >
> > What other mm functionality plans to use SRCU?
Right now I have (unpublished) patches that use srcu as a way to avoid
mmap_sem when faulting across the entire path. Previous alternatives
also use it, as ie, can involve IO and lots of other sleeping
operations. Yes, you can argue that they're not published all you want,
but I'm talking beyond my specific use case. Linux VM is known to scale,
why should we hide a core scalability tool from it?
> > Among other things, no-mmu builds might still be able to omit it.
>
> Yup.
Makes sense.
>
> It's pretty trivial to make the shrinker srcuification be a
> Kconfigurable thing. A few little helper functions and we're done.
> That way, non-SMP kernels can use the plain old rwsem if so desired.
>
> otoh it's better to use the same mechanism on all kernels for reasons
> of testing coverage, maintenance cost, etc.
>
> The mm-srcu-ify-shrinkers.patch changelog is suspiciously lacking in
> evidence-of-benefit. We could just drop it?
That's up to you, but I feel we should have srcu available in mm.
Dropping this particular patch is only a band-aid, imo.
Thanks,
Davidlohr
On Mon, Jul 27, 2015 at 01:03:12PM -0700, Andrew Morton wrote:
> On Sat, 25 Jul 2015 15:35:24 -0700 Josh Triplett <[email protected]> wrote:
>
> > > Some mm functionality might very possibly rely on srcu in the future if
> > > we expect any chances of scaling, ie: faults. So I'd rather not take a
> > > short term solution here, as we'll probably be discussing this again
> > > otherwise.
> >
> > What other mm functionality plans to use SRCU?
> >
> > Among other things, no-mmu builds might still be able to omit it.
>
> Yup.
>
> It's pretty trivial to make the shrinker srcuification be a
> Kconfigurable thing. A few little helper functions and we're done.
> That way, non-SMP kernels can use the plain old rwsem if so desired.
>
> otoh it's better to use the same mechanism on all kernels for reasons
> of testing coverage, maintenance cost, etc.
I agree with that. I'm wondering if, rather than making the
SRCU-ification optional, shrinkers themselves could just be optional.
Unless I'm badly misunderstanding what shrinkers do, they seem like a
perfect example of something that could be omitted with little to no
impact. (Stub them out, make them never called, and if you run out of
memory just be unhappy. Ditto for the oom-killer, which really ought to
be optional.)
- Josh Triplett
On Mon, Jul 27, 2015 at 01:20:02PM -0700, Davidlohr Bueso wrote:
> On Mon, 2015-07-27 at 13:03 -0700, Andrew Morton wrote:
> > On Sat, 25 Jul 2015 15:35:24 -0700 Josh Triplett <[email protected]> wrote:
> >
> > > > Some mm functionality might very possibly rely on srcu in the future if
> > > > we expect any chances of scaling, ie: faults. So I'd rather not take a
> > > > short term solution here, as we'll probably be discussing this again
> > > > otherwise.
> > >
> > > What other mm functionality plans to use SRCU?
>
> Right now I have (unpublished) patches that use srcu as a way to avoid
> mmap_sem when faulting across the entire path. Previous alternatives
> also use it, as ie, can involve IO and lots of other sleeping
> operations.
That sounds interesting! mmap_sem is definitely a performance
bottleneck. How do you handle writes versus reads?
> Yes, you can argue that they're not published all you want,
> but I'm talking beyond my specific use case. Linux VM is known to scale,
> why should we hide a core scalability tool from it?
In the case of mmap_sem, does it help at all if tiny kernels were 1)
non-preemptible and 2) non-SMP? Tiny kernels don't necessarily care
about scaling.
> > > Among other things, no-mmu builds might still be able to omit it.
> >
> > Yup.
>
> Makes sense.
Thanks.
- Josh Triplett
On Mon, 27 Jul 2015 13:27:58 -0700 [email protected] wrote:
> I agree with that. I'm wondering if, rather than making the
> SRCU-ification optional, shrinkers themselves could just be optional.
> Unless I'm badly misunderstanding what shrinkers do, they seem like a
> perfect example of something that could be omitted with little to no
> impact. (Stub them out, make them never called, and if you run out of
> memory just be unhappy. Ditto for the oom-killer, which really ought to
> be optional.)
The shrinkers do important stuff ;) "find /" will consume large amounts
of memory for inode and dentry caches. The shrinkers are how we free
that up again.
On Mon, Jul 27, 2015 at 01:31:33PM -0700, Andrew Morton wrote:
> On Mon, 27 Jul 2015 13:27:58 -0700 [email protected] wrote:
>
> > I agree with that. I'm wondering if, rather than making the
> > SRCU-ification optional, shrinkers themselves could just be optional.
> > Unless I'm badly misunderstanding what shrinkers do, they seem like a
> > perfect example of something that could be omitted with little to no
> > impact. (Stub them out, make them never called, and if you run out of
> > memory just be unhappy. Ditto for the oom-killer, which really ought to
> > be optional.)
>
> The shrinkers do important stuff ;) "find /" will consume large amounts
> of memory for inode and dentry caches. The shrinkers are how we free
> that up again.
*Ah*, I see. I misunderstood their purpose, and I didn't realize that
was one of the cases they covered. While that might be possible to
reduce, it doesn't sound like it can go away entirely. :)
- Josh Triplett
On Mon, 2015-07-27 at 13:31 -0700, [email protected] wrote:
> That sounds interesting! mmap_sem is definitely a performance
> bottleneck. How do you handle writes versus reads?
The idea is to make vmas srcu aware, such that their lookups in the
vmacache are lockless and can survive the entire fault path, among
others we have ->vm_file. We simply handle cases when the
vma/page-tables have changed between when the lookup was done and when
we grab the pte lock with mmap_sem. These invalidations are a pain,
albeit non fatal in some cases.
>
> > Yes, you can argue that they're not published all you want,
> > but I'm talking beyond my specific use case. Linux VM is known to scale,
> > why should we hide a core scalability tool from it?
>
> In the case of mmap_sem, does it help at all if tiny kernels were 1)
> non-preemptible and 2) non-SMP? Tiny kernels don't necessarily care
> about scaling.
Yes, I believe it would! I actually assumed tiny kernels were already
UP. I don't think it makes much sense to have it at that level. Same
with preemption.
Thanks,
Davidlohr
On Tue, Jul 28, 2015 at 11:14:40AM -0700, Davidlohr Bueso wrote:
> On Mon, 2015-07-27 at 13:31 -0700, [email protected] wrote:
> > That sounds interesting! mmap_sem is definitely a performance
> > bottleneck. How do you handle writes versus reads?
>
> The idea is to make vmas srcu aware, such that their lookups in the
> vmacache are lockless and can survive the entire fault path, among
> others we have ->vm_file. We simply handle cases when the
> vma/page-tables have changed between when the lookup was done and when
> we grab the pte lock with mmap_sem. These invalidations are a pain,
> albeit non fatal in some cases.
SOunds promising.
> > > Yes, you can argue that they're not published all you want,
> > > but I'm talking beyond my specific use case. Linux VM is known to scale,
> > > why should we hide a core scalability tool from it?
> >
> > In the case of mmap_sem, does it help at all if tiny kernels were 1)
> > non-preemptible and 2) non-SMP? Tiny kernels don't necessarily care
> > about scaling.
>
> Yes, I believe it would! I actually assumed tiny kernels were already
> UP. I don't think it makes much sense to have it at that level. Same
> with preemption.
OK. So, would you consider making it possible to compile out SRCU in
that specific case, while depending on SRCU if either SMP or PREEMPT?
Because that's also the case that allows tiny RCU rather than full RCU.
- Josh Triplett