>>> On 25.05.16 at 17:34, <quan...@intel.com> wrote: > On May 23, 2016 10:19 PM, Jan Beulich <jbeul...@suse.com> wrote: >> >>> On 18.05.16 at 10:08, <quan...@intel.com> wrote: >> > + unsigned long type, >> > + int preemptible) >> > { >> > unsigned long nx, x, y = page->u.inuse.type_info; >> > - int rc = 0; >> > + int rc = 0, ret = 0; >> > >> > ASSERT(!(type & ~(PGT_type_mask | PGT_pae_xen_l2))); >> > >> > @@ -2578,11 +2579,11 @@ static int __get_page_type(struct page_info >> *page, unsigned long type, >> > if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) ) >> > { >> > if ( (x & PGT_type_mask) == PGT_writable_page ) >> > - iommu_unmap_page(d, mfn_to_gmfn(d, page_to_mfn(page))); >> > + ret = iommu_unmap_page(d, mfn_to_gmfn(d, >> > + page_to_mfn(page))); >> > else if ( type == PGT_writable_page ) >> > - iommu_map_page(d, mfn_to_gmfn(d, page_to_mfn(page)), >> > - page_to_mfn(page), >> > - IOMMUF_readable|IOMMUF_writable); >> > + ret = iommu_map_page(d, mfn_to_gmfn(d, page_to_mfn(page)), >> > + page_to_mfn(page), >> > + >> > + IOMMUF_readable|IOMMUF_writable); >> > } >> > } >> > >> > @@ -2599,6 +2600,9 @@ static int __get_page_type(struct page_info >> *page, unsigned long type, >> > if ( (x & PGT_partial) && !(nx & PGT_partial) ) >> > put_page(page); >> > >> > + if ( !rc ) >> > + rc = ret; >> >> I know I've seen this a couple of time already, but with the special purpose >> of >> "ret" I really wonder whether a more specific name wouldn't be warranted - >> e.g. "iommu_rc" or "iommu_ret". > > > rc is return value for this function, and no directly association with IOMMU > related code ( rc is only for alloc_page_type() ). > So the rc cannot be "iommu_rc".. > > ret can be "iommu_ret", but I think the pair 'rc' / 'ret' may look good.
Well, I'm not entirely opposed to keeping the names, but I continue to think that while at the call sites the shorter name is reasonable, it is quite a bit less clear at the point where you conditionally update rc. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel