Date: Tue, 10 Mar 1998 02:59:01 +0300 From: Dmitrij Tejblum <dima@tejblum.dnttm.rssi.ru> To: Terry Lambert <tlambert@primenet.com> Cc: current@FreeBSD.ORG Subject: Re: vnode_pager: *** WARNING *** stale FS code in system Message-ID: <199803092359.CAA03508@tejblum.dnttm.rssi.ru> In-Reply-To: Your message of "Sun, 08 Mar 1998 23:31:19 GMT." <199803082331.QAA08328@usr08.primenet.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Terry Lambert wrote:
> > > The reason I went the way I did was that putting the vnode_pager.c
> > > code in the defaultops vector does nothing toward making the code
> > > in vnode_pager.c _go away_, which is the eventual goal.
> >
> > Your changes do same nothing toward this goal. Everyone can insert a
> > function into a vnops table. It is trivial. Almost nobody can write a
> > real getpages or putpages routine. For example, you failed even to
> > write a readonly cd9660_putpages, while it would be, apparently,
> > trivial. (I suspect, simple 'return (VM_PAGER_BAD);' would do a right
> > thing.) You also failed to mention this problem in your standard comment.
> > So, when real implementation for getpages and putpages for all these
> > filesystem will written?
>
> Most assuredly, at some time *after* a skeleton versions of FS specific
> getpages/putpages have been written. Organizationally, the patches I
> gave *do* move toward that goal.
Before these changes, simple grep for getpages_desc gave the list of
real getpages implementations. The tool in src/tools/tools/vop_table
also could be used to find such list. Now you succefully created
illusion that all filesystems already have own specific getpages
implementation. The "organizational" side of these changes, as I
already stated, is trivial.
> If you insist, I can duplicate the generic code into the FS's themselves
> immediately, and remove it from the generic location. I don't see this
> action as being useful.
Me too. Please don't duplicate it.
> [...]
> > Filesystem-specific getpages/putpages possible since end of 1995. You
> > saved a filesystem writer from writing 7 obvious lines of each
> > implementations (even these lines contain style bugs, so actual number
> > is slightly less). Is this the big win?
>
> Codewise, no. That's why I would have preferred to leave the other FS's
> issuing the warning until such time as I could fix the code; after all,
> I am a purist. But pragmatically, it makes the desired organization
> obvious (instead of clouding it unnecessarily), and the patches were
> necessary, given the stabilization requirements now in force.
Desired organization was obvious. You made the organization more
complex without a real advantage.
> I would cetrainly welcome your assitance in unstubbing the local media
> getpages/putpages, if you were willing to give it. This would resolve
> your implementation issues, without stepping on my organizational issues.
Thanks. It is hardly possible since I know almost nothing about VM.
> > > for a large variety of reasons (I can enumerate these
> > > reasons again, if you need me to).
> >
> > Well, actually I don't know any reasons for it, other than reference to
> > John Dyson's authority and complexity of the generic code. So it would
> > be interesting. But it is outside of scope of this discussion.
>
> Then casting aspersions on my reasoning by implying that my only
> validation is lodged in "appeal to authority" is probably *also*
> outside the scope of this discussion, don't you think?
Uh. Yes. But the list of reasons would be interesting :). And I
didn't want "aspersions", but referred only to my, quite limited,
knowledges...
>
> > From point of view of a
> > stacking layer, the filesystem below provide a getpages/putpages
> > implementation, and this is all that you want.
>
> Actually, I want to use the local media FS's pager. The difference here
> is that if I have a crypto FS stacked on top of a transaction FS stacked
> on top of a ACL FS stacked on top of a quota FS on top of an FFS FS,
> when I try to getpages on the crypto FS, it should try to getpages on the
> underlying FFS.
>
> In your suggested implementation, I can't collapse the call graph
> because in the stacking FS's will each have a getpages/putpages to
> access the bypass explicitly, instead of accessing it implicitly.
I don't see what force stacking layers to access bypass explicitly.
Look:
int
vop_defaultop(struct vop_generic_args *ap)
{
return (VOCALL(default_vnodeop_p, ap->a_desc->vdesc_offset, ap));
}
Does your stacking layer modify ap->a_desc->vdesc_offset? If yes, how
and why? If no, everything is OK: vop_defaultop will do the right thing.
In other words, the call to vop_defaultop is absolutely equivalent to the
operation in the default vector itself.
Anyway, this doesn't matter much, since I don't insist on the defaultop
way.
>
> Here's my preferred soloution, since it will pacify you as well: make
> the warning non-fatal *for now*; ie:
> [...]
This looks even worse. And no, it will not pacify me :) To pacify me,
either remove msdosfs_getpages and msdosfs_putpages, or make them real
msdosfs-specific implementation. These two dumb functions without any
purpose irritates me. Actually, all functions without a purpose irritates
me. (Well, actually I am already almost pacified :)
> > > A tertiary reason is that if the code is in the defaultops vector,
> > > I can't know when it becomes safe to remove from vnode_pager.c
> > > and the defaultops vector. There's no way to measure usage of
> > > the defaultops code (that's kind of the point, really).
> >
> > OK, don't put it to defaultops vector, put vop_stdgetpages and
> > vop_stdputpages to vnops table of every affected filesystem. I suggest
> > it in every mail on this topic :-).
>
> Now you are basically griping only about naming.
About functions which do nothing themselves.
> The problem with calling it standard is that *it's not supposed to be
> the standard, correct way of doing things*.
Well, call it as you like :)
> The blunt fact is that the way it was being done is screwed up, and
> *something* needs to be done to unscrew it, and done in such a way
> as it doesn't screw the FS stacking at the same time.
I would not call it 'unscrew'. Your action keep vnode_pager.c screwed,
and also abused lot of virgin filesystems.
> > One may say that my complains are more about decorative issues that
> > about real programming issues. Sorry. It is because all discussing
> > changes actually more decorative than real code. And as decorative,
> > they are IMHO bad. Also, unfortunately, most points in this discussion
> > were repeated several times...
>
> I don't know how many times I should have to repeat this, but the code
> *should not be in vnode_pager.c* and was, in fact *designed to not be
> in vnode_pager.c*.
And here is my usual answer: you did *nothing* to remove the code.
> [...]
Dima
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199803092359.CAA03508>