Re: [PATCH 3/4] NFSv4.1 add nfs_inode book keeping for mdsthreshold

["Adamson, Andy" <William.Adamson@xxxxxxxxxx>]

On May 23, 2012, at 2:19 PM, Myklebust, Trond wrote:

> On Wed, 2012-05-23 at 05:02 -0400, andros@xxxxxxxxxx wrote:
>> From: Andy Adamson <andros@xxxxxxxxxx>
>> 
>> Keep track of the number of bytes read or written, including those queued
>> up to be flushed. For use by mdsthreshold i/o size hints.
>> 
>> No locking needed as this is used as hint information.
>> 
>> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
>> ---
>> fs/nfs/file.c          |    8 ++++++--
>> fs/nfs/inode.c         |    2 ++
>> fs/nfs/pnfs.c          |    3 +++
>> include/linux/nfs_fs.h |    3 +++
>> 4 files changed, 14 insertions(+), 2 deletions(-)
>> 
>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>> index 8eda8a6..c4cc096 100644
>> --- a/fs/nfs/file.c
>> +++ b/fs/nfs/file.c
>> @@ -203,8 +203,10 @@ nfs_file_read(struct kiocb *iocb, const struct iovec *iov,
>> 	result = nfs_revalidate_mapping(inode, iocb->ki_filp->f_mapping);
>> 	if (!result) {
>> 		result = generic_file_aio_read(iocb, iov, nr_segs, pos);
>> -		if (result > 0)
>> +		if (result > 0) {
>> +			NFS_I(inode)->read_io += result;
> 
> Should we perhaps rather do this from nfs_readpages(), nfs_readpage()
> and nfs_direct_read()?
> 
> If we do it here in nfs_file_read, we miss mmaped reads, O_DIRECT reads,
> as well as splice reads.

Well that's not good.

> We also count read cache hits where we don't
> have to actually access the server.

OK.

> 
>> 			nfs_add_stats(inode, NFSIOS_NORMALREADBYTES, result);
>> +		}
>> 	}
>> 	return result;
>> }
>> @@ -613,8 +615,10 @@ static ssize_t nfs_file_write(struct kiocb *iocb, const struct iovec *iov,
>> 		if (err < 0)
>> 			result = err;
>> 	}
>> -	if (result > 0)
>> +	if (result > 0) {
>> +		NFS_I(inode)->write_io += written;
> 
> For the same reason, perhaps we should move this to
> nfs_direct_write_schedule_iovec(), and nfs_write_end().
> 
>> 		nfs_add_stats(inode, NFSIOS_NORMALWRITTENBYTES, written);
>> +	}
>> out:
>> 	return result;
>> 
>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>> index 889f7e5..a6f5fbb 100644
>> --- a/fs/nfs/inode.c
>> +++ b/fs/nfs/inode.c
>> @@ -323,6 +323,8 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>> 		inode->i_gid = -2;
>> 		inode->i_blocks = 0;
>> 		memset(nfsi->cookieverf, 0, sizeof(nfsi->cookieverf));
>> +		nfsi->write_io = 0;
>> +		nfsi->read_io = 0;
>> 
>> 		nfsi->read_cache_jiffies = fattr->time_start;
>> 		nfsi->attr_gencount = fattr->gencount;
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index cbcb6ae..6620606 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -395,6 +395,9 @@ mark_matching_lsegs_invalid(struct pnfs_layout_hdr *lo,
>> 	dprintk("%s:Begin lo %p\n", __func__, lo);
>> 
>> 	if (list_empty(&lo->plh_segs)) {
>> +		/* Reset MDS Threshold I/O counters */
>> +		NFS_I(lo->plh_inode)->write_io = 0;
>> +		NFS_I(lo->plh_inode)->read_io = 0;
>> 		if (!test_and_set_bit(NFS_LAYOUT_DESTROYED, &lo->plh_flags))
>> 			put_layout_hdr_locked(lo);
>> 		return 0;
>> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
>> index ca4a707..c6954ac 100644
>> --- a/include/linux/nfs_fs.h
>> +++ b/include/linux/nfs_fs.h
>> @@ -201,6 +201,9 @@ struct nfs_inode {
>> 
>> 	/* pNFS layout information */
>> 	struct pnfs_layout_hdr *layout;
>> +	/* how many bytes have been written/read and how many bytes queued up */
>> +	__u64 write_io;
>> +	__u64 read_io;
>> #endif /* CONFIG_NFS_V4*/
> 
> ^^^^ This doesn't look as if it will compile without CONFIG_NFS_V4.

I'll fix and resend. Thanks for the review :)

-->Andy

> 
>> #ifdef CONFIG_NFS_FSCACHE
>> 	struct fscache_cookie	*fscache;
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@xxxxxxxxxx
> www.netapp.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