Date: Thu, 23 Mar 2017 00:55:09 +0000 From: Rick Macklem <rmacklem@uoguelph.ca> To: Konstantin Belousov <kostikbel@gmail.com> 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: <YTXPR01MB01896F39B9733D3CAE6A4EF3DD3F0@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> In-Reply-To: <20170322072331.GQ43712@kib.kiev.ua> 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>, <20170322072331.GQ43712@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Konstantin Belousov wrote:
[stuff snipped]
> 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).
Wow, this is looking good to me. I had thought that the simple way to make
ncl_putpages() go through the buffer cache was to replace ncl_writerpc() wi=
th
VOP_WRITE(). My concern was all the memory<->memory copying that would
go on between the pages being written and the buffers allocated by VOP_WRIT=
E().
If there is a way to avoid some (if not all) of this memory<->memory copyin=
g, then
I think it would be a big improvement..
As far as the commit goes, you don't need to do anything if you are calling=
VOP_WRITE().
(The code below VOP_WRITE() takes care of all of that.)
--> You might want to implement a function like nfs_write(), but with extra=
arguments.
If you did that, you could indicate when you want the writes to happe=
n synchronously
vs. async/delayed and that would decide when FILESYNC would be specif=
ied.
As far as I know, the unpatched nc_putpages() is badly broken for the UNSTA=
BLE/commit
case. For UNSTABLE writes, the client is supposed to know how to write the =
data again
if the server crashes/reboots before a Commit RPC is successfully done for =
the data.
(The ncl_clearcommit() function is the one called when the server indicates=
it has
rebooted and needs this. It makes no sense whatsoever and breaks the clien=
t to call
it in ncl_putpages() when mustcommit is set. All mustcommit being set indi=
cates is
that the write RPC was done UNSTABLE and the above applies to it. Some ser=
vers always
do FILESYNC, so it isn't ever necessary to do a Commit PRC or redo the wri=
te RPCs.)
Summary. If you are calling VOP_WRITE() or a similar call above the buffer =
cache,
then you don't have to worry about any of this.
> Things that needs to be done is to add missed handling of the IO flags to
> ncl_write().
> + if (error =3D=3D 0 || !nfs_keep_dirty_on_error)
> vnode_pager_undirty_pages(pages, rtvals, count - uio.uio_=
resid);
If the data isn't copied, will this data still be available to the NFS buff=
er cache code,
so that it can redo the writes for the UNSTABLE case, if the server reboots=
before a
Commit RPC has succeeded?
> - if (must_commit)
> - ncl_clearcommit(vp->v_mount);
No matter what else we do, this should go away. As above, it breaks the NFS=
client
and basically forces all dirty buffer cache blocks to be rewritten when it =
shouldn't
be necessary.
rick=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YTXPR01MB01896F39B9733D3CAE6A4EF3DD3F0>