Re: [LKP] Re: [xfs] a1df10d42b: xfstests.generic.31*.fail

From: Dave Chinner <david@fromorbit.com>
To: Philip Li <philip.li@intel.com>
Cc: Oliver Sang <oliver.sang@intel.com>,
	Guo Xuenan <guoxuenan@huawei.com>,
	lkp@lists.01.org, lkp@intel.com, Hou Tao <houtao1@huawei.com>,
	linux-xfs@vger.kernel.org
Subject: Re: [LKP] Re: [xfs]  a1df10d42b: xfstests.generic.31*.fail
Date: Tue, 11 Oct 2022 07:54:40 +1100	[thread overview]
Message-ID: <20221010205440.GV3600936@dread.disaster.area> (raw)
In-Reply-To: <Y0NoKUei4Xfn/afb@rli9-MOBL1.ccr.corp.intel.com>

On Mon, Oct 10, 2022 at 08:32:41AM +0800, Philip Li wrote:
> On Mon, Oct 10, 2022 at 11:07:40AM +1100, Dave Chinner wrote:
> > On Sun, Oct 09, 2022 at 03:17:55PM +0800, Oliver Sang wrote:
> > > Hi Dave,
> > > 
> > > On Thu, Oct 06, 2022 at 08:35:43AM +1100, Dave Chinner wrote:
> > > > On Wed, Oct 05, 2022 at 09:45:12PM +0800, kernel test robot wrote:
> > > > > 
> > > > > Greeting,
> > > > > 
> > > > > FYI, we noticed the following commit (built with gcc-11):
> > > > > 
> > > > > commit: a1df10d42ba99c946f6a574d4d31951bc0a57e33 ("xfs: fix exception caused by unexpected illegal bestcount in leaf dir")
> > > > > url: https://github.com/intel-lab-lkp/linux/commits/UPDATE-20220929-162751/Guo-Xuenan/xfs-fix-uaf-when-leaf-dir-bestcount-not-match-with-dir-data-blocks/20220831-195920
> > > > > 
[....]

> > commit a1df10d42ba99c946f6a574d4d31951bc0a57e33 *does not exist in
> > the upstream xfs-dev tree*. The URL provided pointing to the commit
> > above resolves to a "404 page not found" error, so I have not idea
> > what code was even being tested here.
> > 
> > AFAICT, the patch being tested is this one (based on the github url
> > matching the patch title:
> > 
> > https://lore.kernel.org/linux-xfs/20220831121639.3060527-1-guoxuenan@huawei.com/
> > 
> > Which I NACKed almost a whole month ago! The latest revision of the
> > patch was posted 2 days ago here:
> > 
> > https://lore.kernel.org/linux-xfs/20221008033624.1237390-1-guoxuenan@huawei.com/
> > 
> > Intel kernel robot maintainers: I've just wasted the best part of 2
> > hours trying to reproduce and track down a corruption bug that this
> > report lead me to beleive was in the upstream XFS tree.
> 
> hi Dave, we are very sorry to waste your time on this report. It's our fault to not
> make it clear that this is testing a review patch in mailing list. And we also
> miss the NACKed information in your review, and send out this meaningless report.

The biggest issue was how it was presented.

Normally I see reports from the kernel robot for specific
uncommitted patches like this as a threaded reply to the specific
patch that was identified as having a problem.  And normally this
sort of standalone test failure report comes from a failure bisected
to a commit already in an upstream tree. 

So my confusion here is largely because a bug in an uncommitted
patch was reported in the same manner as an upstream regression
would be reported - as a standalone bug report...

> > You need to make it very clear that your bug report is for a commit
> > that *hasn't been merged into an upstream tree*. The CI robot
> > noticed a bug in an *old* NACKed patch, not a bug in a new upstream
> > commit. Please make it *VERY CLEAR* where the code the CI robot is
> > testing has come from.
> 
> We will correct our process ASAP to 
> 
> 1) make it clear, what is tested from, a review patch or a patch on upstream tree

Yes, commit ID by itself is not sufficient to identify the issue,
nor is a pointer to the CI tree the robot built. For a patch pulled
from a list, it should not be reported as a "commit that failed".
It should be reported as "uncommitted patch that failed", with:

- a lore link to the patch that was identified as having an issue;
- a pointer to the base tree the patch(es) were applied to (e.g.
  linus-v5.19-rc7, linux-next-2022-25-09, etc)
- a pointer to the CI integration tree (that doesn't) the patch was
  applied to and tested.

For an upstream commit that failed, reporting "<commit id> failed"
is a good start, but it really needs to include the tree as the
robot might be testing dev trees or linux-next rather than Linus's
tree. i.e. report as "<tree, commit id> failed <test>".

> 2) do not send such report, if the patch has already been NACKed

That's not so much a problem. The real problem that needs solving is
ensuring that the recipients of the bug report are able to quickly
and obviously identify what was being tested when the issue was hit.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com