Re: BUG_ON() in workingset_node_shadows_dec() triggers

From: Linus Torvalds
Date: Wed Oct 05 2016 - 12:11:17 EST


On Wed, Oct 5, 2016 at 2:25 AM, Johannes Weiner <hannes@xxxxxxxxxxx> wrote:
>
> Here is a reproducer that triggers the warning instantly for me:

Yup, confirmed.With the VM_WARN_ON_ONCE() it just gets a big nice
splat and the machine happily stays up.

> That radix tree node management needs some cleaning up. It probably
> makes sense to split node->count into actually separate members for
> clarity, and then add a root tag to distinguish shadows from regular
> entries in root->rnode. I have to think about this more, the current
> situation is too fragile and ugly.

Ugh. I even looked at the "node->count = 1" initialization in
radix_tree_extend(), and didn't react to it at all, it looked
obviously correct.

This code is too subtle.

> But in the meantime, there is an obvious fix: don't ever store shadow
> entries in root->rnode, seeing as we need nodes for proper accounting.
>
> It means we temporarily lose the ability to detect refaults from
> single-page files, but it's probably better to keep the stable fix
> small and restore that functionality in a new release.
>
> Patch below. NOTE: I'm traveling without access to my test rig right
> now and so I have only lightly tested this on my laptop. I'm also
> jetlagged like crazy, so please triple check my thinking. The patch
> does fix the reproducer case and has otherwise been stable here.

Hmm. I'm inclined to just apply it and mark it for stable, along with
your other patch. But yes, this needs more thinking about (and
obviously testing). The interactions with the radix tree are too
subtle.

Linus