Date: Wed, 22 Mar 2017 09:23:31 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Rick Macklem <rmacklem@uoguelph.ca> Cc: Dimitry Andric <dim@FreeBSD.org>, Ian Lepore <ian@FreeBSD.org>, Gergely Czuczy <gergely.czuczy@harmless.hu>, FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: process killed: text file modification Message-ID: <20170322072331.GQ43712@kib.kiev.ua> In-Reply-To: <YQBPR01MB01807C7F00F1480FBBE82BF5DD3D0@YQBPR01MB0180.CANPRD01.PROD.OUTLOOK.COM> References: <YTXPR01MB0189F7147A7C5C5F8C56B2F1DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <20170317141917.GS16105@kib.kiev.ua> <D0770019-3EEA-45D2-A751-18DF1B274F90@FreeBSD.org> <YTXPR01MB0189FBB6CF664653C1162936DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <20170318032150.GW16105@kib.kiev.ua> <YTXPR01MB0189F47B6A23C10BFE8A85E6DD3B0@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <20170320221818.GM43712@kib.kiev.ua> <YTXPR01MB0189F229B71B500B8C3027DFDD3D0@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <20170321175527.GN43712@kib.kiev.ua> <YQBPR01MB01807C7F00F1480FBBE82BF5DD3D0@YQBPR01MB0180.CANPRD01.PROD.OUTLOOK.COM>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Mar 21, 2017 at 09:41:19PM +0000, Rick Macklem wrote:
> Konstantin Belousov wrote:
> > Anyway, my position is that nfs VOP_PUTPAGES() should do write through
> > buffer cache, not issuing the direct rpc call with the pages as source.
> Hmm. Interesting idea. Since a "struct buf" can only refer to
> contiguous bytes, I suspect each page might end up as a separate
> "struct buf", at least until some clustering algorithm succeeded in
> merging them.
>
> I would agree that it would be nice to change VOP_PUTPAGES(), since
> it currently results in a lot of 4K writes (with FILE_SYNC I think?)
> and this is normally slow/inefficient for the server. (It would be
> interesting to try your suggestion above and see if the pages would
> cluster into larger writes. Also, the "struct buf" code knows how to
> do UNSTABLE writes followed by a Commit.)
Below is something to discuss. This is not finished, but it worked for
the simple tests I performed. Clustering should be somewhat handled by
the ncl_write() as is. As an additional advantage, I removed the now
unneeded phys buffer allocation.
If you agree with the approach on principle, I want to ask what to do
about the commit stuff there (I simply removed that for now).
Things that needs to be done is to add missed handling of the IO flags to
ncl_write().
diff --git a/sys/fs/nfsclient/nfs_clbio.c b/sys/fs/nfsclient/nfs_clbio.c
index 1c225c1469a..562754609b1 100644
--- a/sys/fs/nfsclient/nfs_clbio.c
+++ b/sys/fs/nfsclient/nfs_clbio.c
@@ -266,9 +266,7 @@ ncl_putpages(struct vop_putpages_args *ap)
{
struct uio uio;
struct iovec iov;
- vm_offset_t kva;
- struct buf *bp;
- int iomode, must_commit, i, error, npages, count;
+ int ioflags, i, error, npages, count;
off_t offset;
int *rtvals;
struct vnode *vp;
@@ -322,44 +320,34 @@ ncl_putpages(struct vop_putpages_args *ap)
}
mtx_unlock(&np->n_mtx);
- /*
- * We use only the kva address for the buffer, but this is extremely
- * convenient and fast.
- */
- bp = getpbuf(&ncl_pbuf_freecnt);
-
- kva = (vm_offset_t) bp->b_data;
- pmap_qenter(kva, pages, npages);
PCPU_INC(cnt.v_vnodeout);
PCPU_ADD(cnt.v_vnodepgsout, count);
- iov.iov_base = (caddr_t) kva;
+ iov.iov_base = unmapped_buf;
iov.iov_len = count;
uio.uio_iov = &iov;
uio.uio_iovcnt = 1;
uio.uio_offset = offset;
uio.uio_resid = count;
- uio.uio_segflg = UIO_SYSSPACE;
+ uio.uio_segflg = UIO_NOCOPY;
uio.uio_rw = UIO_WRITE;
uio.uio_td = td;
- if ((ap->a_sync & VM_PAGER_PUT_SYNC) == 0)
- iomode = NFSWRITE_UNSTABLE;
- else
- iomode = NFSWRITE_FILESYNC;
+ ioflags = IO_VMIO;
+ if (ap->a_sync & (VM_PAGER_PUT_SYNC | VM_PAGER_PUT_INVAL))
+ ioflags |= IO_SYNC;
+ else if ((ap->a_sync & VM_PAGER_CLUSTER_OK) == 0)
+ ioflags |= IO_ASYNC;
+ ioflags |= (ap->a_sync & VM_PAGER_PUT_INVAL) ? IO_INVAL: 0;
+ ioflags |= (ap->a_sync & VM_PAGER_PUT_NOREUSE) ? IO_NOREUSE : 0;
+ ioflags |= IO_SEQMAX << IO_SEQSHIFT;
- error = ncl_writerpc(vp, &uio, cred, &iomode, &must_commit, 0);
+ error = VOP_WRITE(vp, &uio, ioflags, cred);
crfree(cred);
- pmap_qremove(kva, npages);
- relpbuf(bp, &ncl_pbuf_freecnt);
-
- if (error == 0 || !nfs_keep_dirty_on_error) {
+ if (error == 0 || !nfs_keep_dirty_on_error)
vnode_pager_undirty_pages(pages, rtvals, count - uio.uio_resid);
- if (must_commit)
- ncl_clearcommit(vp->v_mount);
- }
- return rtvals[0];
+ return (rtvals[0]);
}
/*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170322072331.GQ43712>