On Tue, Mar 20, 2012 at 12:56 PM, Myklebust, Trond
<Trond.Myklebust@xxxxxxxxxx> wrote:
> On Tue, 2012-03-20 at 12:25 -0400, Fred Isaman wrote:
>> It was reporting success even if it did nothing.
>>
>> Note this is still not right. Caller nfs_commit_unstable_pages
>> assumes this is all or none, and count is going to be off.
>>
>> Signed-off-by: Fred Isaman <iisaman@xxxxxxxxxx>
>> ---
>> fs/nfs/nfs4filelayout.c | 4 +++-
>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
>> index e0bdbf4..08625a8 100644
>> --- a/fs/nfs/nfs4filelayout.c
>> +++ b/fs/nfs/nfs4filelayout.c
>> @@ -1027,6 +1027,7 @@ filelayout_commit_pagelist(struct inode *inode, struct list_head *mds_pages,
>> struct nfs_write_data *data, *tmp;
>> LIST_HEAD(list);
>> unsigned int nreq = 0;
>> + int status = PNFS_ATTEMPTED;
>>
>> if (!list_empty(mds_pages)) {
>> data = nfs_commitdata_alloc();
>> @@ -1042,6 +1043,7 @@ filelayout_commit_pagelist(struct inode *inode, struct list_head *mds_pages,
>>
>> if (nreq == 0) {
>> nfs_commit_clear_lock(NFS_I(inode));
>> + status = -ENOMEM;
>
> What if nreq != 0, but some of the allocations still failed?
>
> Then there's the issue of what if the one or more of the calls to
> nfs_initiate_commit() fail?
>
> Finally, there is the question of what if the commit RPC calls
> themselves fail, or cause a retransmission?
>
> IOW: Why do we single out some of these errors as worthy of reporting,
> but not others? The cleanup seems to be the same in all cases above
> (mark the requests for retransmission and mark the inode dirty).
>
I agree (which is why I didn't fix it "in full"), but I haven't traced
all the VFS callers and their expectations. However, at one point
nfs_commit_inode returned the count of the number of pages sent to RPC
(and nfs_commit_unstable_pages uses that fact). It would not be too
hard to get that to happen again. Or stop returning a count
entirely...which means doing something about
nfs_commit_unstable_pages. Or at least make clear in comments that
the count returned is an undercount
Fred
--
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
