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

Re: [Xen-devel] [PATCH v9 4/5] x86/mem_sharing: reset a fork

> >> In such a case, please put in a comment explaining why failure is
> >> impossible. In the general case e.g. a 2Mb page may need splitting,
> >> which may yield -ENOMEM. Such a comment will then also be useful in
> >> case a new failure mode gets added to ->set_entry(), where it then
> >> will need judging whether the assumption here still holds. (This is
> >> also why in general it'd be better to handle the error. It'll still
> >> be better to crash the guest than the host in case you can't. See
> >> the bottom of ./CODING_STYLE.)
> >
> > The mem_sharing codebase uses ASSERT(!rc) on p2m->set_entry already
> > when removing pages like we do here (see relinquish_shared_pages).
> > This only gets called on shared pages that we know for sure are
> > present. Since these are shared pages we know that their size is 4k
> > thus there is no splitting. I can certainly add a comment to this
> > effect to spell it out why the ASSERT is appropriate.
> Well, if this is a pre-existing pattern in the file, then - you
> being the maintainer - so be it. I consider this bad practice though,
> and I would suggest that every such site needs a comment (possibly
> all but one simply referring to the one where things get actually
> explained).

Noted. I think for an experimental code-base it's fine.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.