On Tue, June 16, 2009 10:33 am, J. Bruce Fields wrote:
> On Tue, Jun 16, 2009 at 10:21:50AM +1000, NeilBrown wrote:
>> On Tue, June 16, 2009 9:08 am, J. Bruce Fields wrote:
>>
>> > + if (host_err >= 0 && stable)
>> > + wait_for_concurrent_writes(file, use_wgather, &host_err);
>> >
>>
>> Surely you want this to be:
>>
>> if (host_err >= 0 && stable && use_wgather)
>> host_err = wait_for_concurrent_writes(file);
>> as
>> - this is more readable
>> - setting last_ino and last_dev is pointless when !use_wgather
>
> Yep, thanks.
>
>> - we aren't interested in differentiation between non-negative values
>> of
>> host_err.
>
> Unfortunately, just below:
>
> if (host_err >= 0) {
> err = 0;
> *cnt = host_err;
> } else
> err = nfserrno(host_err);
>
Ahh.... that must be in code you haven't pushed out yet.
I don't see it in mainline or git.linux-nfs.org
> We could save that count earlier, e.g.:
>
> @@ -1014,6 +1013,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh
> *fhp,
> int host_err;
> int stable = *stablep;
> int use_wgather;
> + int bytes;
>
> #ifdef MSNFS
> err = nfserr_perm;
> @@ -1056,6 +1056,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh
> *fhp,
> set_fs(oldfs);
> if (host_err >= 0) {
> nfsdstats.io_write += host_err;
> + bytes = host_err;
> fsnotify_modify(file->f_path.dentry);
Or even
if (host_err >= 0) {
bytes = host_err;
nfsdstats.io_write += bytes
...
And if you did that in whatever patch move the assignment to
*cnt to the bottom of the function, it might be even more readable!
Thanks,
NeilBrown
--
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
