Re: [Xen-devel] [PATCH V2 1/2] xen/granttable: Support sub-page grants

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

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

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