svn commit: r270759 - in head/sys: cddl/compat/opensolaris/kern cddl/compat/opensolaris/sys cddl/contrib/opensolaris/uts/common/fs/zfs vm

Steven Hartland smh at freebsd.org
Sat Aug 30 19:53:34 UTC 2014


----- Original Message ----- 
From: "Peter Wemm" <peter at wemm.org>
snip...
> > I'm aware of the values involved, and as I said what you're 
> > proposing
> > was more akin to where I started, but I was informed that it had 
> > already
> > been tested and didn't work well.
>
> And Karl also said that his tests are on machines that have no 
> v_cache, so
> he's not testing the scenario.
>
> The code, as written, is wrong.  It's as simple as that.
>
> The logic is wrong.
>
> You've introduced dead code.
>
> Your code changes introduce a scenario that CAUSES one of the very 
> problems
> you're using as a justtification for the changes.
>
> Your own testers have admitted that they don't test the scenario that 
> the
> problem exists with.

Wooo hold on there, I'm trying to react to feedback and come to a proper
solution. I've already said that my initial version was very much like 
what
I believe your requesting its changed to, but I reacted to feedback on 
that.

Now if that feedback was inaccurate or miss-guided I'm sorry, and I'm
thankfull for your input as the domain expert.

The PR was created quite some time ago, and it wasn't till I encountered 
a
related issue in a live system the other weekend that it got some 
attention.

Its clearly an important issue that needs resolving so lets work 
together
to come up with a fix everyones happy with.

> > > Also, what about the magic numbers here:
> > > u_int zfs_arc_free_target = (1 << 19); /* default before 
> > > pagedaemon
> > > init only */
> >
> > That is just a total fall back case and should never be triggered 
> > unless
> > as the comment states the pagedaemon isn't initialised.
> >
> > > That's half a million pages, or 2GB of physical ram on a 4K page 
> > > size
> > > system
> > > How is this going to work on early boot in the machines in the 
> > > cluster
> > > with
> > > less than 2GB of ram?
> >
> > Its there to ensure that ARC doesn't run wild ARC for the few
> > milliseconds
> > / seconds before pagedaemon is initalised.
> >
> > We can change the value no problem, what would you suggest 1<<16 aka
> > 256MB?
>
> Please stop picking magic numbers out of thin air.  You are working 
> with file
> system and VM - critical parts of the system.  This is NOT the place 
> to be
> screwing around with things you don't understand.  alc@ was trying to 
> be
> polite.

Please help me out here, I'm trying to do the right thing so I'm looking
to you guys for advice.

> > Thanks for all the feedback, its great to have my understanding of
> > how things work in this area confirmed by those who know.
> >
> > Hopefully we'll be able to get to the bottom of this with everyones
> > help and get a solid fix for these issues that have plaged 10 into
> > 10.1 :)
>
> I'm very disappointed in the attention to detail and errors in the 
> commit.
> I'm almost at the point where I want to ask for the whole thing to be 
> backed
> out.

I'm not disagreeing with anyone, I'm simply trying to accure the 
information
on how to update this that will result in the best for users, so 
sweeping
statements like this just make me feel bad for trying to help :(

I'm more than happy to address any concerns people have.

I've kicked this off with a webrev https://reviews.freebsd.org/D700

    Regards
    Steve 



More information about the svn-src-all mailing list