Re: LOR on nfs: vfs_vnops.c:301 kern_descrip.c:1580

On Wednesday, August 18, 2010 3:17:56 pm pluknet wrote:
> On 18 August 2010 23:11, pluknet <pluknet_at_gmail.com> wrote:
> > On 18 August 2010 17:46, Kostik Belousov <kostikbel_at_gmail.com> wrote:
> >> On Wed, Aug 18, 2010 at 02:43:19PM +0400, pluknet wrote:
> >>> On 18 August 2010 12:07, pluknet <pluknet_at_gmail.com> wrote:
> >>> > On 17 August 2010 20:04, Kostik Belousov <kostikbel_at_gmail.com> wrote:
> >>> >
> >>> >>
> >>> >> Also please take a note of the John' suggestion to use the taskqueue.
> >>> >
> >>> > I decided to go this road. Thank you both.
> >>> > Now I do nfs buildkernel survive and prepare some benchmark results.
> >>> >
> >>>
> >>> So, I modified the patch to defer proc_create() with taskqueue(9).
> >>> Below is `time make -j5 buildkernel WITHOUT_MODULES=yes` perf. 
evaluation.
> >>> Done on 4-way CPU on clean /usr/obj with /usr/src & /usr/obj both
> >>> nfs-mounted over 1Gbit LAN.
> >>>
> >>> clean old
> >>> 1137.985u 239.411s 7:42.15 298.0%       6538+2133k 87+43388io 226pf+0w
> >>>
> >>> clean new
> >>> 1134.755u 240.032s 7:41.25 298.0%       6553+2133k 87+43367io 224pf+0w
> >>>
> >>> Patch needs polishing, though it generally works.
> >>> Not sure if shep_chan (or whatever name it will get) needs locking.
> >> As I said yesterday, if several requests to create nfsiod coming one
> >> after another, you would loose all but the last.
> >>
> >> You should put the requests into the list, probably protected by
> >> nfs_iod_mtx.
> >>
> >
> > How about this patch? Still several things to ask.
> > 1) I used malloc instance w/ M_NOWAIT, since it's called with nfs_iod_mtx 
held.
> > 2) Probably busy/done gymnastics is a wrong mess. Your help is 
appreciated.
> > 3) if (1) is fine, is it right to use fail: logic (i.e. set
> > NFSIOD_NOT_AVAILABLE)
> > on memory shortage? Not tested.
> >
> > There are debug printf() left intentionally to see how 3 contexts run 
under load
> > to each other. I attached these messages as well if that makes sense.
> >
> 
> Ah, yes. Sorry, forgot about that.

Your task handler needs to run in a loop until the list of requests is empty.  
If multiple threads call taskqueue_enqueue() before your task is scheduled, 
they will be consolidated down to a single call of your task handler.

-       int error, i;
-       int newiod;
+       int i, newiod, error;

You should sort these alphabetically if you are going to change this.  I would 
probably just leave it as-is.

Also, I'm not sure how this works as you don't actually wait for the task to 
complete.  If the taskqueue_enqueue() doesn't preempt, then you may read 
ni_error as zero before the kproc has actually been created and return success 
even though an nfsiod hasn't been created.

-- 
John Baldwin