linux-kernel - Re: [PATCH 0/3] add fallback reason strings to DAX PMD path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170524090332.GC10604@quack2.suse.cz>
Date:   Wed, 24 May 2017 11:03:32 +0200
From:   Jan Kara <jack@...e.cz>
To:     Ross Zwisler <ross.zwisler@...ux.intel.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christoph Hellwig <hch@....de>,
        Dan Williams <dan.j.williams@...el.com>,
        Ingo Molnar <mingo@...hat.com>, Jan Kara <jack@...e.cz>,
        Matthew Wilcox <mawilcox@...rosoft.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-fsdevel@...r.kernel.org, linux-nvdimm@...ts.01.org
Subject: Re: [PATCH 0/3] add fallback reason strings to DAX PMD path

On Tue 23-05-17 15:25:57, Ross Zwisler wrote:
> One of the primary motivations for adding tracepoints to the DAX PMD path
> was to allow the user to diagnose whether their system was actually using
> PMDs, and if not to help them understand why.  For me at least this has
> worked okay in some situations, but many times I find myself adding more
> debugging to diagnose fallback reasons that aren't immediately obvious, or
> situations where the current tracepoints are simply insufficient because
> they don't give you enough information.
> 
> This series adds short fallback reason strings to the tracepoints in the
> PMD path with the intention of giving the user better information about why
> their system is falling back to PTEs.

So I don't like adding strings into tracepoints. It just seems too close to
debug printk. After all if you need detailed information for debugging some
particular bug, you can always add kprobe tracepoint at particular
instruction in the function.

Probably my objection boils down to the fact that I don't think tracepoints
in DAX code are for a normal user. They expose lot of details about the
implementation and once users start depending on a particular tracepoint,
it becomes part of the kernel API and cannot be changed which will be
very inconvenient for DAX implementation. I think these tracepoints are for
kernel developers to be able to diagnose what's going on and I'm willing to
impose some thinking burden on those ;).

Also looking at the reasons you've added most of them can already be
detected from the information output by the tracepoint so it seems mostly
like a duplicit information. So my opinion is: If you need more information
like whether pfn is from devmap, make that visible in the tracepoint (like
add pfn flags to the output). Sure the reason for fallback isn't
immediately visible but if you really wonder, you go through conditions in
the code, verify them against the data from the tracepoint and you should
be able to find the reason... If there are some common fallback paths that
you'd think are worth detecting and it's not easy to add information to the
final tracepoint, then we can add a specific tracepoint to that branch.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR