Re: [Xen-devel] [PATCH 3/5] xen: clean up grant_table.h

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.08.17 at 20:05, <jgross@xxxxxxxx> wrote:
> @@ -118,6 +154,18 @@ struct grant_mapping {
>      uint32_t pad;           /* round size to a power of 2 */
>  };
>  
> +/* Number of grant table frames. Caller must hold d's grant table lock. */
> +static inline unsigned int nr_grant_frames(struct grant_table *gt)
> +{
> +    return gt->nr_grant_frames;
> +}
> +
> +/* Number of status grant table frames. Caller must hold d's gr. table 
> lock.*/
> +static inline unsigned int nr_status_frames(struct grant_table *gt)
> +{
> +    return gt->nr_status_frames;
> +}

For both the parameters want to be constified.

> @@ -250,6 +318,15 @@ static inline void active_entry_release(struct 
> active_grant_entry *act)
>      spin_unlock(&act->lock);
>  }
>  
> +#define GRANT_STATUS_PER_PAGE (PAGE_SIZE / sizeof(grant_status_t))
> +#define GRANT_PER_PAGE (PAGE_SIZE / sizeof(grant_entry_v2_t))
> +/* Number of grant table status entries. Caller must hold d's gr. table 
> lock.*/
> +static inline unsigned int grant_to_status_frames(int grant_frames)

unsigned int also for the parameter.

> --- a/xen/include/xen/grant_table.h
> +++ b/xen/include/xen/grant_table.h
> @@ -29,65 +29,8 @@
>  #include <asm/page.h>
>  #include <asm/grant_table.h>
>  
> -#ifndef DEFAULT_MAX_NR_GRANT_FRAMES /* to allow arch to override */
> -/* Default maximum size of a grant table. [POLICY] */
> -#define DEFAULT_MAX_NR_GRANT_FRAMES   32
> -#endif
>  /* The maximum size of a grant table. */
> -extern unsigned int max_grant_frames;
> -
> -DECLARE_PERCPU_RWLOCK_GLOBAL(grant_rwlock);
> -
> -/* Per-domain grant information. */
> -struct grant_table {
> -    /*
> -     * Lock protecting updates to grant table state (version, active
> -     * entry list, etc.)
> -     */
> -    percpu_rwlock_t       lock;
> -    /* Table size. Number of frames shared with guest */
> -    unsigned int          nr_grant_frames;
> -    /* Shared grant table (see include/public/grant_table.h). */
> -    union {
> -        void **shared_raw;
> -        struct grant_entry_v1 **shared_v1;
> -        union grant_entry_v2 **shared_v2;
> -    };
> -    /* Number of grant status frames shared with guest (for version 2) */
> -    unsigned int          nr_status_frames;
> -    /* State grant table (see include/public/grant_table.h). */
> -    grant_status_t       **status;
> -    /* Active grant table. */
> -    struct active_grant_entry **active;
> -    /* Mapping tracking table per vcpu. */
> -    struct grant_mapping **maptrack;
> -    unsigned int          maptrack_limit;
> -    /* Lock protecting the maptrack limit */
> -    spinlock_t            maptrack_lock;
> -    /* The defined versions are 1 and 2.  Set to 0 if we don't know
> -       what version to use yet. */
> -    unsigned              gt_version;
> -};
> -
> -static inline void grant_read_lock(struct grant_table *gt)
> -{
> -    percpu_read_lock(grant_rwlock, &gt->lock);
> -}
> -
> -static inline void grant_read_unlock(struct grant_table *gt)
> -{
> -    percpu_read_unlock(grant_rwlock, &gt->lock);
> -}
> -
> -static inline void grant_write_lock(struct grant_table *gt)
> -{
> -    percpu_write_lock(grant_rwlock, &gt->lock);
> -}
> -
> -static inline void grant_write_unlock(struct grant_table *gt)
> -{
> -    percpu_write_unlock(grant_rwlock, &gt->lock);
> -}
> +extern unsigned int __read_mostly max_grant_frames;

Why are you adding __read_mostly here? Like all section placement
annotations it belongs on definitions only, not declarations.

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -307,6 +307,7 @@ struct vm_event_per_domain
>  };
>  
>  struct evtchn_port_ops;
> +struct grant_table;

Why is this needed? There's no function with a respective parameter
in the header here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel