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

Re: [Xen-devel] [PATCH 1/2] tmem: add full support for x86 up to 16Tb



>>> On 23.09.13 at 15:17, Bob Liu <bob.liu@xxxxxxxxxx> wrote:
>> The patch is full of coding style violations; in fact there are too
>> many of them to list them individually.
>> 
> 
> Sorry for that, it's my first xen patch.
> Is there any document or checkpatch script so that I can get rid of the
> coding style problem?

./CODINGSTYLE (and of course you could use other core Xen files
as reference - just don't use the tmem ones themselves because
they are quite inconsistent).

>>>  static inline void tmh_free_page(struct page_info *pi)
>>>  {
>>> +    int xen_heap = is_xen_heap_page(pi);
>>>      ASSERT(IS_VALID_PAGE(pi));
>>> -    tmh_page_list_put(pi);
>>> -    atomic_dec(&freeable_page_count);
>>> +    if (xen_heap){
>>> +       scrub_one_page(pi);
>>> +       ASSERT((pi->count_info & ~(PGC_allocated | 1)) == 0);
>> 
>> Did you perhaps copy this from somewhere else without
>> understanding why it is needed there? It certainly doesn't look
>> necessary here: Such checks are meaningful only when you
>> allow a guest access to a Xen heap page.
>> 
> 
> Sorry, I just copyed this code from _tmh_free_page_thispool().
> Sounds like these checks are also unneeded in _tmh_free_page_thispool()?

Yes, this looks as bogus. But I'm not a tmem expert, so you'd
better check whether there's some non-obvious dependency
(depending on when these got added, doing some archaeology
may also help here).

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®.