This does not appear to fix the problem I'm seeing.
With this patch applied, the client is silly renaming the file, deleting it, then writing it again and failing with NFS4ERR_STALE.
I had to apply the change by hand, so I could have missed something. I'll go back and check.
On Jun 6, 2012, at 9:59 AM, Adamson, Dros wrote:
> ACK - This fixes the problem I was seeing!
>
> For the list, we were seeing duplicate writes sent to unstable servers even though the COMMIT succeeds (with matching verf):
>
> - mount a server that returns writes with committed = UNSTABLE4
> - run "dd if=/dev/zero of=/mnt/foo bs=1024 count=10240"
> - umount
> - in wireshark, you'll see the COMMIT succeed, but after a short pause there will be a flurry of FILE_SYNC4 writes that are duplicates (same ranges, data) of the writes associated with the COMMIT.
>
> -dros
>
> On Jun 5, 2012, at 6:43 PM, Trond Myklebust wrote:
>
>> The new commit code fails to copy the verifier into the wb_verf field
>> of _all_ the nfs_page structures; it only copies it into the first entry.
>> The consequence is that most requests end up failing to match in
>> nfs_commit_release.
>>
>> Fix is to copy the verifier into the req->wb_verf field in
>> nfs_write_completion.
>>
>> Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
>> Cc: Fred Isaman <iisaman@xxxxxxxxxx>
>> ---
>> fs/nfs/direct.c | 4 ++--
>> fs/nfs/write.c | 7 ++++---
>> include/linux/nfs_xdr.h | 2 ++
>> 3 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
>> index 23d170b..b5385a7 100644
>> --- a/fs/nfs/direct.c
>> +++ b/fs/nfs/direct.c
>> @@ -710,12 +710,12 @@ static void nfs_direct_write_completion(struct nfs_pgio_header *hdr)
>> if (dreq->flags == NFS_ODIRECT_RESCHED_WRITES)
>> bit = NFS_IOHDR_NEED_RESCHED;
>> else if (dreq->flags == 0) {
>> - memcpy(&dreq->verf, &req->wb_verf,
>> + memcpy(&dreq->verf, hdr->verf,
>> sizeof(dreq->verf));
>> bit = NFS_IOHDR_NEED_COMMIT;
>> dreq->flags = NFS_ODIRECT_DO_COMMIT;
>> } else if (dreq->flags == NFS_ODIRECT_DO_COMMIT) {
>> - if (memcmp(&dreq->verf, &req->wb_verf, sizeof(dreq->verf))) {
>> + if (memcmp(&dreq->verf, hdr->verf, sizeof(dreq->verf))) {
>> dreq->flags = NFS_ODIRECT_RESCHED_WRITES;
>> bit = NFS_IOHDR_NEED_RESCHED;
>> } else
>> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
>> index e6fe3d6..4d6861c 100644
>> --- a/fs/nfs/write.c
>> +++ b/fs/nfs/write.c
>> @@ -80,6 +80,7 @@ struct nfs_write_header *nfs_writehdr_alloc(void)
>> INIT_LIST_HEAD(&hdr->rpc_list);
>> spin_lock_init(&hdr->lock);
>> atomic_set(&hdr->refcnt, 0);
>> + hdr->verf = &p->verf;
>> }
>> return p;
>> }
>> @@ -619,6 +620,7 @@ static void nfs_write_completion(struct nfs_pgio_header *hdr)
>> goto next;
>> }
>> if (test_bit(NFS_IOHDR_NEED_COMMIT, &hdr->flags)) {
>> + memcpy(&req->wb_verf, hdr->verf, sizeof(req->wb_verf));
>> nfs_mark_request_commit(req, hdr->lseg, &cinfo);
>> goto next;
>> }
>> @@ -1255,15 +1257,14 @@ static void nfs_writeback_release_common(void *calldata)
>> struct nfs_write_data *data = calldata;
>> struct nfs_pgio_header *hdr = data->header;
>> int status = data->task.tk_status;
>> - struct nfs_page *req = hdr->req;
>>
>> if ((status >= 0) && nfs_write_need_commit(data)) {
>> spin_lock(&hdr->lock);
>> if (test_bit(NFS_IOHDR_NEED_RESCHED, &hdr->flags))
>> ; /* Do nothing */
>> else if (!test_and_set_bit(NFS_IOHDR_NEED_COMMIT, &hdr->flags))
>> - memcpy(&req->wb_verf, &data->verf, sizeof(req->wb_verf));
>> - else if (memcmp(&req->wb_verf, &data->verf, sizeof(req->wb_verf)))
>> + memcpy(hdr->verf, &data->verf, sizeof(*hdr->verf));
>> + else if (memcmp(hdr->verf, &data->verf, sizeof(*hdr->verf)))
>> set_bit(NFS_IOHDR_NEED_RESCHED, &hdr->flags);
>> spin_unlock(&hdr->lock);
>> }
>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>> index 7519bae..8aadd90 100644
>> --- a/include/linux/nfs_xdr.h
>> +++ b/include/linux/nfs_xdr.h
>> @@ -1237,6 +1237,7 @@ struct nfs_pgio_header {
>> struct list_head rpc_list;
>> atomic_t refcnt;
>> struct nfs_page *req;
>> + struct nfs_writeverf *verf;
>> struct pnfs_layout_segment *lseg;
>> loff_t io_start;
>> const struct rpc_call_ops *mds_ops;
>> @@ -1274,6 +1275,7 @@ struct nfs_write_data {
>> struct nfs_write_header {
>> struct nfs_pgio_header header;
>> struct nfs_write_data rpc_data;
>> + struct nfs_writeverf verf;
>> };
>>
>> struct nfs_mds_commit_info {
>> --
>> 1.7.10.2
>>
>
---
Chuck Lever
chuck [dot] lever [at] oracle [dot] com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
