RE: [PATCH V2 1/2] xen/granttable: Support sub-page grants

[Paul Durrant]

> -----Original Message-----
> From: ANNIE LI [mailto:annie.li@xxxxxxxxxx]
> Sent: 08 December 2011 10:02
> To: Paul Durrant
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> konrad.wilk@xxxxxxxxxx; jeremy@xxxxxxxx; Ian Campbell;
> kurt.hackel@xxxxxxxxxx
> Subject: Re: [PATCH V2 1/2] xen/granttable: Support sub-page grants
> 
> 
> >> +
> >> +int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned
> >> long frame,
> >> +					int flags, unsigned page_off,
> >> +					unsigned length)
> >> +{
> >> +	int ref;
> >> +
> >> +	if (flags&  (GTF_accept_transfer | GTF_reading |
> >> +		     GTF_writing | GTF_transitive))
> >> +		return -EPERM;
> >> +
> >> +	if (gnttab_interface->update_subpage_entry == NULL)
> >> +		return -ENOSYS;
> >> +
> >> +	ref = get_free_entries(1);
> >> +	if (unlikely(ref<  0))
> >> +		return -ENOSPC;
> >> +
> >> +	gnttab_interface->update_subpage_entry(ref, domid, frame,
> >> flags,
> >> +					       page_off, length);
> >> +
> >> +	return ref;
> >> +}
> >> +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage);
> > There's quite a lot of duplicated code here. What about something
> along the lines of:
> >
> > #define get_free_entry()	get_free_entries(1)
> >
> > int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned
> long frame,
> > 					int flags, unsigned page_off,
> > 					unsigned length)
> > {
> > 	int ref;
> >
> > 	ref = get_free_entry();
> > 	if (unlikely(ref<  0))
> > 		return -ENOSPC;
> >
> > 	rc = gnttab_grant_foreign_access_subpage_ref(ref, domid,
> frame, flags, page_off, length);
> > 	if (rc<  0)
> > 		put_free_entry(ref);
> >
> > 	return (rc<  0) rc : ref;
> > }
> > EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage);
> >
> > I think this is more akin to the format for existing non-ref
> variants.
> >
> I hesitated between those two implement ways before sending them
> out.
> Those two ways all have shortcoming, one has duplicated code, but is
> simple when condition does not meet. The other way you pointed out
> added more put_free_entry process when _ref function fails.
> 

I say let's be optimistic :-) Yes, there's more overhead in the failure case, but that's not what we should be optimising for.

  Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/