On Tue, Jun 16, 2009 at 10:50:57AM +1000, NeilBrown wrote:
> 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
Whoops--actually, it's the opposite problem: a bugfix patch that went
upstream removed this, and I didn't merge that back into my for-2.6.31
branch. OK, time to do that, and then this is all much simpler....
Thanks for calling my attention to that!
--b.
>
> > 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
