tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: buffer cache & ufs changes (preliminary ffsv2 extattr support)

On Mon, Jan 16, 2012 at 07:37:19PM +0100, Manuel Bouyer wrote:
 > > > The fisrt change is to the buffer cache.
 > > 
 > > My first reaction is that I don't think it's a good idea to make major
 > > changes to the buffer cache at this stage in a release cycle. It's not
 > > exactly the most robust or stable code we have in the system.
 > On the other hand, changing the kernel ABI in the release branch
 > is not a good idea either, given the state of the module subsystem ...

Indeed. But that isn't really the question. The question is really
whether we're past the date for brand-new feature proposals for
netbsd-6... or at least ones that involve invasive changes.

releng and/or core will need to rule on that but I don't think it's a
good idea.

The buffer cache code is ratty, poorly structured, and full of races
that may or may not have visible consequences. Any change to it could
turn out to be unexpectedly disruptive, and we don't at this point
have time to clean up if the changes turn out to make a mess.
(Especially since the person who'd likely have to do the cleanup,
namely me, is already oversubscribed right up to the branch deadline
on other stuff. Which, not to put too fine a point on it, includes
sorting out a previous mess.)

 > I wouldn't call this a major change. The alternative way (adding a b_type
 > member to struct buf) is even less intrusive; if the alternative (*2())
 > functions are not used it'll always be 0.

Changing the way the buffer cache is indexed is semantically intrusive
even if it's not physically intrusive. While I think adding a type
field to modify the block number is a good idea, for various reasons,
it needs to be thought through, and it hasn't been yet and there isn't
time to thrash it out fully before the branch deadline. Furthermore,
there's the existing question of indexing by physical vs. virtual
block numbers; that is not in any way a resolved issue and what you're
proposing interacts directly with it, and this too needs to be thought
through and there isn't time before the branch deadline.

Meanwhile, adding a whole set of extra functions instead of doing
things properly with a massedit is highly undesirable; not only is it
ugly but it leaves us unable to tidy the extra functions away in HEAD
after branching, which is bad... unless we want to vastly complicate
any pullup that goes near the buffer cache, which would be worse.

(Note that adding a flag instead of a type field is even worse; it has
all the same drawbacks, plus we'd be likely to want to rip it out and
replace it with the type field; plus on top of that race conditions
involving the flags are one of the primary things I'm worried about

 > > (Also, you're aware that it isn't used for file data blocks, right?)
 > It depends on what you call "file data". Directory data blocks ends
 > there AFAIK, as do symlink data blocks when the link's target doesn't
 > fit in the inode.

Right. So, should extended attribute blocks go in the old buffer cache
or should they be managed by uvm? This is another question we likely
don't have time to address properly by the branch deadline.

David A. Holland

Home | Main Index | Thread Index | Old Index