Re: [PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Michal Hocko]

On Thu 02-04-20 11:29:09, Konstantin Khlebnikov wrote:
> 
> 
> On 02/04/2020 11.21, Michal Hocko wrote:
> > On Thu 02-04-20 16:10:29, Huang, Ying wrote:
> > > Michal Hocko <mhocko@xxxxxxxxxx> writes:
> > > 
> > > > On Thu 02-04-20 15:03:23, Huang, Ying wrote:
> > [...]
> > > > > > Could you explain why do we need this WARN_ON? I haven't really checked
> > > > > > the swap support for THP but cannot we have normal swap pmd entries?
> > > > > 
> > > > > I have some patches to add the swap pmd entry support, but they haven't
> > > > > been merged yet.
> > > > > 
> > > > > Similar checks are for all THP migration code paths, so I follow the
> > > > > same style.
> > > > 
> > > > I haven't checked other migration code paths but what is the reason to
> > > > add the warning here? Even if this shouldn't happen, smaps is perfectly
> > > > fine to ignore that situation, no?
> > > 
> > > Yes. smaps itself is perfectly fine to ignore it.  I think this is used
> > > to find bugs in other code paths such as THP migration related.
> > 
> > Please do not add new warnings without a good an strong reasons. As a
> > matter of fact there are people running with panic_on_warn and each
> > warning is fatal for them. Please also note that this is a user trigable
> > path and that requires even more care.
> > 
> 
> But this should not happen and if it does we'll never know without debug.

The migration path which already deals with this will notice, right?
Those are paths which really care about consistency.

> VM_WARN_ON checks something only if build with CONFIG_DEBUG_VM=y.
> 
> Anybody who runs debug kernels with panic_on_warn shouldn't expect much stability =)

That doesn't mean we should be adding warnings here and there nilly
willy.

-- 
Michal Hocko
SUSE Labs