Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 24 Oct 2022 23:08:28 +0530
From:   Gautam Menghani <gautammenghani201@gmail.com>
To:     Zach O'Keefe <zokeefe@google.com>
Cc:     Steven Rostedt <rostedt@goodmis.org>, akpm@linux-foundation.org,
        mhiramat@kernel.org, shy828301@gmail.com, vbabka@suse.cz,
        david@redhat.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/khugepaged: add tracepoint to collapse_file()
Message-ID: <Y1bNlKl1XSCxhaVT@biggie>
References: <20221024150922.129814-1-gautammenghani201@gmail.com>
 <20221024111621.3ba17e2c@gandalf.local.home>
 <CAAa6QmSKtj6T2dW1tkg5_HVj2+rXj5inOLdEzr0MkJzQxxcPXQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAAa6QmSKtj6T2dW1tkg5_HVj2+rXj5inOLdEzr0MkJzQxxcPXQ@mail.gmail.com>
Precedence: bulk

On Mon, Oct 24, 2022 at 09:44:16AM -0700, Zach O'Keefe wrote:
> Thanks for your mail, Gautam.
> 
> > I try to keep dereferences out of the calling path as much as possible
> > (adds to I$ at the call site).
> 
> This was probably due to the way I handled
> trace_mm_khugepaged_scan_file(). Perhaps that can be cleaned up at the
> same time as this patch, for consistency.
>
Yes sure I'll send a patch for cleaning this up.
 
> Also, no qualms about adding this tracepoint; there are a few scan
> result codes that overlap between hpage_collapse_scan_file() and those
> possibly returned in collapse_file() such that, if we only have the
> one tracepoint in hpage_collapse_scan_file(), it could be ambiguous
> what callsite the error path stemmed from. Luckily this hasn't been an
> issue thus far.
> 
> Lastly, a few other items we might care about capturing:
> 
> - is_shmem (perhaps the filename is enough to know this - but I know
> at least once during development I was caught off-guard b/c a mount I
> thought to be file-backed turned out to be tmpfs (and something I
> didn't think to question until I had wasted some time on other
> paths)).
> - index

Yes noted.