|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86/mm: Drop MEM_LOG() and correct some printed information
>>> On 29.03.17 at 14:29, <andrew.cooper3@xxxxxxxxxx> wrote:
> * Use 0x prefix for otherwise unqualified hex numbers.
I'm glad this in fact refers to just a single place.
> @@ -1057,10 +1062,10 @@ get_page_from_l1e(
> put_page_type(page);
> put_page(page);
>
> - MEM_LOG("Error updating mappings for mfn %lx (pfn %lx,"
> - " from L1 entry %" PRIpte ") for %d",
> - mfn, get_gpfn_from_mfn(mfn),
> - l1e_get_intpte(l1e), l1e_owner->domain_id);
> + gdprintk(XENLOG_WARNING, "Error updating mappings for mfn %"
> PRI_mfn
> + " (pfn %" PRI_pfn ", from L1 entry %" PRIpte ") for
> d%d\n",
> + mfn, get_gpfn_from_mfn(mfn),
> + l1e_get_intpte(l1e), l1e_owner->domain_id);
> return err;
> }
> }
> @@ -1068,10 +1073,10 @@ get_page_from_l1e(
> return 0;
>
> could_not_pin:
> - MEM_LOG("Error getting mfn %lx (pfn %lx) from L1 entry %" PRIpte
> - " for l1e_owner=%d, pg_owner=%d",
> - mfn, get_gpfn_from_mfn(mfn),
> - l1e_get_intpte(l1e), l1e_owner->domain_id, pg_owner->domain_id);
> + gdprintk(XENLOG_WARNING, "Error getting mfn %" PRI_mfn " (pfn %" PRI_pfn
> + ") from L1 entry %" PRIpte " for l1e_owner d%d, pg_owner d%d",
> + mfn, get_gpfn_from_mfn(mfn),
> + l1e_get_intpte(l1e), l1e_owner->domain_id, pg_owner->domain_id);
Especially here the wrapping of the format string is rather
unfortunate. Didn't we agree to allow format strings to exceed
the 80 column restriction anyway?
> @@ -1388,7 +1398,7 @@ static int alloc_l1_table(struct page_info *page)
> return 0;
>
> fail:
> - MEM_LOG("Failure in alloc_l1_table: entry %d", i);
> + gdprintk(XENLOG_WARNING, "Failure in alloc_l1_table: entry %d\n", i);
%u (or even %03x; same in alloc_l[234]_table())
> @@ -1979,7 +1991,8 @@ static int mod_l2_entry(l2_pgentry_t *pl2e,
>
> if ( unlikely(!is_guest_l2_slot(d, type, pgentry_ptr_to_slot(pl2e))) )
> {
> - MEM_LOG("Illegal L2 update attempt in Xen-private area %p", pl2e);
> + gdprintk(XENLOG_WARNING,
> + "Illegal L2 update attempt in Xen-private area %p\n", pl2e);
Could you make this message useful at once? The pointer is
not really helpful to diagnose anything, I think. Same for
mod_l[34]_entry() then.
> @@ -3179,7 +3208,7 @@ long do_mmuext_op(
>
> if ( unlikely(__copy_from_guest(&op, uops, 1) != 0) )
> {
> - MEM_LOG("Bad __copy_from_guest");
> + gdprintk(XENLOG_WARNING, "Bad __copy_from_guest\n");
I'd suggest to drop this one altogether.
> @@ -3195,7 +3224,8 @@ long do_mmuext_op(
> case MMUEXT_UNPIN_TABLE:
> break;
> default:
> - MEM_LOG("Invalid extended pt command %#x", op.cmd);
> + gdprintk(XENLOG_WARNING,
> + "Invalid extended pt command %#x\n", op.cmd);
And this one too.
> @@ -3297,7 +3329,8 @@ long do_mmuext_op(
> page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC);
> if ( unlikely(!page) )
> {
> - MEM_LOG("Mfn %lx bad domain", op.arg1.mfn);
> + gdprintk(XENLOG_WARNING,
> + "mfn %" PRI_mfn " bad domain\n", op.arg1.mfn);
Perhaps also include the domain which was supposedly bad?
> @@ -3458,7 +3493,8 @@ long do_mmuext_op(
> rc = -EPERM;
> else if ( unlikely(!cache_flush_permitted(d)) )
> {
> - MEM_LOG("Non-physdev domain tried to FLUSH_CACHE.");
> + gdprintk(XENLOG_WARNING,
> + "Non-physdev domain tried to FLUSH_CACHE\n");
> rc = -EACCES;
> }
> else
> @@ -3484,7 +3520,8 @@ long do_mmuext_op(
> }
> else
> {
> - MEM_LOG("Non-physdev domain tried to FLUSH_CACHE_GLOBAL");
> + gdprintk(XENLOG_WARNING,
> + "Non-physdev domain tried to FLUSH_CACHE_GLOBAL\n");
> rc = -EINVAL;
> }
> break;
I think these could also be dropped (and perhaps a few more right
below here).
> @@ -3734,7 +3779,7 @@ long do_mmu_update(
>
> if ( unlikely(__copy_from_guest(&req, ureqs, 1) != 0) )
> {
> - MEM_LOG("Bad __copy_from_guest");
> + gdprintk(XENLOG_WARNING, "Bad __copy_from_guest\n");
And this one.
> @@ -4201,7 +4250,8 @@ static int replace_grant_va_mapping(
> /* Delete pagetable entry. */
> if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, v, 0)) )
> {
> - MEM_LOG("Cannot delete PTE entry at %p", (unsigned long *)pl1e);
> + gdprintk(XENLOG_WARNING,
> + "Cannot delete PTE entry at %p\n", (unsigned long *)pl1e);
Please drop the stray cast.
> @@ -4316,7 +4367,7 @@ int replace_grant_host_mapping(
> if ( !new_addr )
> return destroy_grant_pte_mapping(addr, frame, curr->domain);
>
> - MEM_LOG("Unsupported grant table operation");
> + gdprintk(XENLOG_WARNING, "Unsupported grant table operation\n");
> return GNTST_general_error;
> }
Drop again?
> @@ -4408,10 +4460,11 @@ int donate_page(
>
> fail:
> spin_unlock(&d->page_alloc_lock);
> - MEM_LOG("Bad donate %lx: ed=%d sd=%d caf=%08lx taf=%" PRtype_info,
> - page_to_mfn(page), d->domain_id,
> - owner ? owner->domain_id : DOMID_INVALID,
> - page->count_info, page->u.inuse.type_info);
> + gdprintk(XENLOG_WARNING, "Bad donate mfn %" PRI_mfn
> + ": ed=%d sd=%d caf=%08lx taf=%" PRtype_info "\n",
> + page_to_mfn(page), d->domain_id,
> + owner ? owner->domain_id : DOMID_INVALID,
> + page->count_info, page->u.inuse.type_info);
Why did you not use d%d formatting here?
> @@ -4459,10 +4512,11 @@ int steal_page(
>
> fail:
> spin_unlock(&d->page_alloc_lock);
> - MEM_LOG("Bad page %lx: ed=%d sd=%d caf=%08lx taf=%" PRtype_info,
> - page_to_mfn(page), d->domain_id,
> - owner ? owner->domain_id : DOMID_INVALID,
> - page->count_info, page->u.inuse.type_info);
> + gdprintk(XENLOG_WARNING, "Bad mfn %" PRI_mfn
> + ": ed=%d sd=%d caf=%08lx taf=%" PRtype_info "\n",
> + page_to_mfn(page), d->domain_id,
> + owner ? owner->domain_id : DOMID_INVALID,
> + page->count_info, page->u.inuse.type_info);
Same here.
Is this intended for 4.9?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |