Re: [Xen-ia64-devel] [PATCH 6/12]MCA handler support for Xen/ia64 TAKE 2

[Alex Williamson <alex.williamson@xxxxxx>]

On Thu, 2006-09-28 at 16:11 +0900, SUZUKI Kazuhiro wrote:
> 
>   Interrupts disabled state was made only the operation of sal_queue[]
> like list_entry() and list_del().
> 
>           case SAL_GET_STATE_INFO:
...
> +                     static sal_log_record_header_t *record = NULL;
> +
> +                     if (record == NULL) {
> +                             unsigned int pageorder;
> +
> +                             pageorder  = get_order_from_bytes(size);
> +                             record = (sal_log_record_header_t 
> *)alloc_xenheap_pages(pageorder);
> +                     }
> +                     memset(record, 0, size);
...
> +                             r9 = ia64_sal_get_state_info(e->sal_info_type, 
> (u64 *)record);


Hi Kaz,

   There's one more problem here, SAL_GET_STATE_INFO is SMP safe and
re-entrant, but there's only one record header allocated, and it's not
protected by a lock.  Couldn't a second caller of SAL_GET_STATE_INFO
clear the record header while it's in use?  Also, please clean up the
line wrapping in this area (tabs are 8 spaces, lines are 80 chars).
Thanks,

        Alex

-- 
Alex Williamson                             HP Open Source & Linux Org.


_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel