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

[Juergen Gross <jgross@xxxxxxxx>]

On 24/08/17 16:17, Jan Beulich wrote:
>>>> 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.

Okay.

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

Okay.

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

Oops, sorry.

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

You are right, of course. Will remove it again.


Juergen

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