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

Re: [Xen-devel] [PATCH 2/2] use return value of domain_adjust_tot_pages() where feasible



>>> On 27.11.13 at 15:44, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> On 11/27/2013 08:26 AM, Jan Beulich wrote:
>> This is generally cheaper than re-reading ->tot_pages.
>>
>> While doing so I also noticed an improper use (lacking error handling)
>> of get_domain() as well as lacks of ->is_dying checks in the memory
>> sharing code, which the patch fixes at once. In the course of doing
>> this I further noticed other error paths there pointlessly calling
>> put_page() et al with ->page_alloc_lock still held, which is also being
>> reversed.
> 
> Thus hiding two very important changes underneath a summary that looks 
> completely unimportant.
> 
> If this patch only did as the title suggests, I would question whether 
> it should be included for 4.4, since it seems to have little benefit.  
> Are the other two changes bug fixes?

I'm sure the missing ->is_dying check is one. I'm not sure the
put vs unlock ordering is just inefficient or could actively cause
issues.

> In any case, the summary should indicate to someone just browsing 
> through what important changes might be inside.

The issue is that you can't really put all three things in the title.
And hence I decided to keep the title what it was supposed to be
when I started correcting this code.

With - in my understanding - the memory sharing code still being
experimental, it also didn't really seem worthwhile splitting these
changes out into separate patches (I generally try to do so when
spotting isolated issues, but tend to keep things together when
in the course of doing other adjustments I recognize further small
issues in code that's not production ready anyway, i.e. not
needing to be backported, and hence the title not needing to hint
at that).

In any event, as to the freeze: The code was written and would
have been submitted before the code freeze if I hadn't been
required to hold it back until after XSA-74 went public (which
goes back to our [unwritten?] policy of not publishing changes
that were found necessary/desirable in the course of researching
a specific security issue).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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