Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
From:   Benjamin Coddington <bcodding@redhat.com>
To:     Anna Schumaker <anna@kernel.org>
Cc:     trond.myklebust@hammerspace.com, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 2/2] NFS: drop unused nfs_direct_req bytes_left
Date:   Fri, 17 Nov 2023 07:06:15 -0500
Message-ID: <9F809D5D-9BA6-47A8-A3CA-AE783F612234@redhat.com>
In-Reply-To: <9AF594B0-9477-4EB9-81F5-85A5F9BBAF87@redhat.com>
References: <21a1f2a6155398965f79ed64f0bd23bf38a50367.1700083991.git.bcodding@redhat.com>
 <952eea7e97246870f83e7a4592e9338007dfe625.1700083991.git.bcodding@redhat.com>
 <CAFX2Jfkc7p+22aK0KvN4yUerS1HuKDC+Njo_AJV1=5pWW0sUYQ@mail.gmail.com>
 <CCFAA747-7BC9-40AB-8E37-A18B33A01511@redhat.com>
 <CAFX2Jf=bDFEx2cfzXtHhaMu0pF5dV1-sheANmAXMf0m-arr61w@mail.gmail.com>
 <9AF594B0-9477-4EB9-81F5-85A5F9BBAF87@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 17 Nov 2023, at 6:21, Benjamin Coddington wrote:

> On 16 Nov 2023, at 17:11, Anna Schumaker wrote:
>
>> On Thu, Nov 16, 2023 at 5:03 PM Benjamin Coddington <bcodding@redhat.com> wrote:
>>>
>>> On 16 Nov 2023, at 16:44, Anna Schumaker wrote:
>>>
>>>> Hi Ben,
>>>>
>>>> On Wed, Nov 15, 2023 at 4:34 PM Benjamin Coddington <bcodding@redhat.com> wrote:
>>>>>
>>>>> Now that we're calculating how large a remaining IO should be based
>>>>> on the current request's offset, we no longer need to track bytes_left on
>>>>> each struct nfs_direct_req.  Drop the field, and clean up the direct
>>>>> request tracepoints.
>>>>
>>>> I've been having problems with xfstests generic/465 on all NFS
>>>> versions after applying this patch. Looking at wireshark, the client
>>>> appears to be resending the same reads over and over again. Have you
>>>> seen anything like this in your testing?
>>>
>>> I have generic/465 failing before and after these two patches on pNFS SCSI..
>>> but at least it completes.  If I run it without pNFS I can see the same
>>> thing.. it just sends the same reads over and over.  I'll figure out why.
>>
>> Thanks! I have it failing normally as well, so that's expected. It's
>> the hanging forever that's not :)
>
> The direct read is returning 0 when there's data on the device.
>
> Oh, the problem is probably that patch drops the update of dreq->max_count,
> which I overlooked because of the double assignment.  Shame on me.

BTW - I think generic/465 makes bad assumptions about what read() should
return for O_DIRECT, and that's why it fails on NFS.  Basically it does a
bunch of WRITEs and then READs and expects the same data coming back in the
READs, but doesn't use O_SYNC.  On the wire, the client is interleaving the
READs and WRITEs.

Ben