Date: Sun, 4 Oct 1998 02:50:50 +0000 (GMT) From: Terry Lambert <tlambert@primenet.com> To: dg@root.com Cc: julian@whistle.com, tlambert@primenet.com, current@FreeBSD.ORG Subject: Re: VM mmap file extension bug still exists? Message-ID: <199810040250.TAA20866@usr06.primenet.com> In-Reply-To: <199809300920.CAA04277@implode.root.com> from "David Greenman" at Sep 30, 98 02:20:17 am
next in thread | previous in thread | raw e-mail | index | archive | help
> >> It's definitely a workaround. The real fix is to add a parementer to > >> the mapping function. > > > >Ok here's the patch done the 'correct way' > >I have looked at the problem with terry and he has convinced me that > >there IS a problem there. > > Well, as I said in previous email, I'm not convinced that adding another > argument to the pager_alloc functions is "correct", and I think your patches > pretty well demonstrate that. It appears to me that in every case, the > "size_in_bytes" is just the non-rounded/indexed version of the original > "size" parameter, so why don't we simply change the definition of "size" to > be an off_t number of bytes and then do the OFF_TO_IDX translation in the > pager(s) as necessary, instead of at the caller? I realize that the patches provided are less than satisfactory; the second, by it's nature, has caught a case in vfs_bio.c that the first ignored, so it was a useful exercise, if only for that. I'm pretty sure that unifying on a byte parameter would be incorrect in vfs_bio.c and in the vm_map_split() function, for two places in particular. I'm also pretty sure it would be a problem elsewhere. One big issue: one number is the number of backing pages to allocate, and the other is the size of the backing object. I think for the vm_map_split code, in particular, that these numbers must not be simply related via a multipl (or bit shift). There seems to be a large bit of cruft in there, unfortunately, that resulted from the code doing its damnedest to avoid 64 bit math. I'm pretty sure that on the Alpha, doing this would reduce the number of backable pages by a factor of the page size. I wasn't really comfortable with doing this. The real issue is "who needs to know the size of the backing object?" (right now, that's *only* vnode_pager), and "what restrictions on the backing object size are inherent in this change?". My problem is the places where the IDX/OFF conversion macros are used. I also have a bit of a problem with putting a bunch of extra 64 bit math in the paging path, because it's a critical path. I'm very happy to cede the architecture related decisions about what the exact implementation should be to you and to others in the FreeBSD core team, so long as *something* is done about the problems, rather than *nothing*. Terry Lambert terry@lambert.org --- Any opinions in this posting are my own and not those of my present or previous employers. 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?199810040250.TAA20866>