On Tue, Jun 14, 2011 at 7:01 AM, <tao.peng@xxxxxxx> wrote: > Hi, Fred, > >> -----Original Message----- >> From: linux-nfs-owner@xxxxxxxxxxxxxxx [mailto:linux-nfs-owner@xxxxxxxxxxxxxxx] >> On Behalf Of Fred Isaman >> Sent: Monday, June 13, 2011 10:44 PM >> To: Jim Rees >> Cc: linux-nfs@xxxxxxxxxxxxxxx; peter honeyman >> Subject: Re: [PATCH 04/34] pnfs: hook nfs_write_begin/end to allow layout driver >> manipulation >> >> On Sun, Jun 12, 2011 at 7:43 PM, Jim Rees <rees@xxxxxxxxx> wrote: >> > From: Peng Tao <bergwolf@xxxxxxxxx> >> > >> > Signed-off-by: Fred Isaman <iisaman@xxxxxxxxxxxxxx> >> > Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx> >> > Reported-by: Alexandros Batsakis <batsakis@xxxxxxxxxx> >> > Signed-off-by: Andy Adamson <andros@xxxxxxxxxx> >> > Signed-off-by: Fred Isaman <iisaman@xxxxxxxxxx> >> > Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx> >> > Signed-off-by: Peng Tao <bergwolf@xxxxxxxxx> >> > --- >> > fs/nfs/file.c | 26 ++++++++++- >> > fs/nfs/pnfs.c | 41 +++++++++++++++++ >> > fs/nfs/pnfs.h | 115 >> ++++++++++++++++++++++++++++++++++++++++++++++++ >> > fs/nfs/write.c | 12 +++-- >> > include/linux/nfs_fs.h | 3 +- >> > 5 files changed, 189 insertions(+), 8 deletions(-) >> > >> > diff --git a/fs/nfs/file.c b/fs/nfs/file.c >> > index 2f093ed..1768762 100644 >> > --- a/fs/nfs/file.c >> > +++ b/fs/nfs/file.c >> > @@ -384,12 +384,15 @@ static int nfs_write_begin(struct file *file, struct >> address_space *mapping, >> > pgoff_t index = pos >> PAGE_CACHE_SHIFT; >> > struct page *page; >> > int once_thru = 0; >> > + struct pnfs_layout_segment *lseg; >> > >> > dfprintk(PAGECACHE, "NFS: write_begin(%s/%s(%ld), %u@%lld)\n", >> > file->f_path.dentry->d_parent->d_name.name, >> > file->f_path.dentry->d_name.name, >> > mapping->host->i_ino, len, (long long) pos); >> > - >> > + lseg = pnfs_update_layout(mapping->host, >> > + nfs_file_open_context(file), >> > + pos, len, IOMODE_RW, GFP_NOFS); >> >> >> This looks like it is left over from before the rearrangements done to >> where pnfs_update_layout. >> In particular, we don't want to hold the reference on the lseg from >> here until flush time. And there >> seems to be no reason to. If the client needs a layout to deal with >> read-in here, it should instead >> trigger the nfs_want_read_modify_write clause. > Yes, you are right. Directly calling pnfs_update_layout here can be avoided. > But it seems triggering nfs_want_read_modify_write will acquire a read-only layout segment via readpage code path. > For write, client will need a read-write layout segment so it would mean two layoutget for each new segment (one in nfs_readpage and one at flush time). It may not be good for performance. > Does current generic code have method to avoid this? > > Thanks, > Tao > No. However, note that this only hits in the case where you are doing subpage writes. 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