| Message ID | 20170914125852.22129-3-wei.liu2@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 14.09.17 at 14:58, <wei.liu2@citrix.com> wrote: > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -705,22 +705,6 @@ bool map_ldt_shadow_page(unsigned int offset) > return true; > } > > - > -static bool get_page_from_mfn(mfn_t mfn, struct domain *d) > -{ > - struct page_info *page = mfn_to_page(mfn); > - > - if ( unlikely(!mfn_valid(mfn)) || unlikely(!get_page(page, d)) ) > - { > - gdprintk(XENLOG_WARNING, > - "Could not get page ref for mfn %"PRI_mfn"\n", mfn_x(mfn)); > - return false; > - } > - > - return true; > -} > - > - > static int get_page_and_type_from_mfn( > mfn_t mfn, unsigned long type, struct domain *d, > int partial, int preemptible) > diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h > index bef45e8e9f..7670912e0a 100644 > --- a/xen/include/asm-x86/mm.h > +++ b/xen/include/asm-x86/mm.h > @@ -369,6 +369,20 @@ int get_page_from_l1e( > l1_pgentry_t l1e, struct domain *l1e_owner, struct domain *pg_owner); > void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner); > > +static inline bool get_page_from_mfn(mfn_t mfn, struct domain *d) > +{ > + struct page_info *page = __mfn_to_page(mfn_x(mfn)); Why can this not remain to be mfn_to_page(mfn)? The header has been introduced only in the immediately preceding patch, so there can't be users of the header not coping with this. And once other code includes this header, it should be made type-safe first. Jan
On Fri, Sep 22, 2017 at 05:44:58AM -0600, Jan Beulich wrote: > >>> On 14.09.17 at 14:58, <wei.liu2@citrix.com> wrote: > > --- a/xen/arch/x86/mm.c > > +++ b/xen/arch/x86/mm.c > > @@ -705,22 +705,6 @@ bool map_ldt_shadow_page(unsigned int offset) > > return true; > > } > > > > - > > -static bool get_page_from_mfn(mfn_t mfn, struct domain *d) > > -{ > > - struct page_info *page = mfn_to_page(mfn); > > - > > - if ( unlikely(!mfn_valid(mfn)) || unlikely(!get_page(page, d)) ) > > - { > > - gdprintk(XENLOG_WARNING, > > - "Could not get page ref for mfn %"PRI_mfn"\n", mfn_x(mfn)); > > - return false; > > - } > > - > > - return true; > > -} > > - > > - > > static int get_page_and_type_from_mfn( > > mfn_t mfn, unsigned long type, struct domain *d, > > int partial, int preemptible) > > diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h > > index bef45e8e9f..7670912e0a 100644 > > --- a/xen/include/asm-x86/mm.h > > +++ b/xen/include/asm-x86/mm.h > > @@ -369,6 +369,20 @@ int get_page_from_l1e( > > l1_pgentry_t l1e, struct domain *l1e_owner, struct domain *pg_owner); > > void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner); > > > > +static inline bool get_page_from_mfn(mfn_t mfn, struct domain *d) > > +{ > > + struct page_info *page = __mfn_to_page(mfn_x(mfn)); > > Why can this not remain to be mfn_to_page(mfn)? The header has > been introduced only in the immediately preceding patch, so there > can't be users of the header not coping with this. And once other > code includes this header, it should be made type-safe first. I think you misread. This is asm-x86/mm.h, not pv/mm.h. This header file has been used in many places. Furthermore, this patch introduces no change. If you look at mm.c at the beginning: #undef mfn_to_page #define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn))
>>> On 22.09.17 at 13:51, <wei.liu2@citrix.com> wrote: > On Fri, Sep 22, 2017 at 05:44:58AM -0600, Jan Beulich wrote: >> >>> On 14.09.17 at 14:58, <wei.liu2@citrix.com> wrote: >> > --- a/xen/arch/x86/mm.c >> > +++ b/xen/arch/x86/mm.c >> > @@ -705,22 +705,6 @@ bool map_ldt_shadow_page(unsigned int offset) >> > return true; >> > } >> > >> > - >> > -static bool get_page_from_mfn(mfn_t mfn, struct domain *d) >> > -{ >> > - struct page_info *page = mfn_to_page(mfn); >> > - >> > - if ( unlikely(!mfn_valid(mfn)) || unlikely(!get_page(page, d)) ) >> > - { >> > - gdprintk(XENLOG_WARNING, >> > - "Could not get page ref for mfn %"PRI_mfn"\n", > mfn_x(mfn)); >> > - return false; >> > - } >> > - >> > - return true; >> > -} >> > - >> > - >> > static int get_page_and_type_from_mfn( >> > mfn_t mfn, unsigned long type, struct domain *d, >> > int partial, int preemptible) >> > diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h >> > index bef45e8e9f..7670912e0a 100644 >> > --- a/xen/include/asm-x86/mm.h >> > +++ b/xen/include/asm-x86/mm.h >> > @@ -369,6 +369,20 @@ int get_page_from_l1e( >> > l1_pgentry_t l1e, struct domain *l1e_owner, struct domain *pg_owner); >> > void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner); >> > >> > +static inline bool get_page_from_mfn(mfn_t mfn, struct domain *d) >> > +{ >> > + struct page_info *page = __mfn_to_page(mfn_x(mfn)); >> >> Why can this not remain to be mfn_to_page(mfn)? The header has >> been introduced only in the immediately preceding patch, so there >> can't be users of the header not coping with this. And once other >> code includes this header, it should be made type-safe first. > > I think you misread. This is asm-x86/mm.h, not pv/mm.h. Oh, indeed. I'm sorry. Acked-by: Jan Beulich <jbeulich@suse.com> Jan
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 8d2a4682c9..faa161b767 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -705,22 +705,6 @@ bool map_ldt_shadow_page(unsigned int offset) return true; } - -static bool get_page_from_mfn(mfn_t mfn, struct domain *d) -{ - struct page_info *page = mfn_to_page(mfn); - - if ( unlikely(!mfn_valid(mfn)) || unlikely(!get_page(page, d)) ) - { - gdprintk(XENLOG_WARNING, - "Could not get page ref for mfn %"PRI_mfn"\n", mfn_x(mfn)); - return false; - } - - return true; -} - - static int get_page_and_type_from_mfn( mfn_t mfn, unsigned long type, struct domain *d, int partial, int preemptible) diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h index bef45e8e9f..7670912e0a 100644 --- a/xen/include/asm-x86/mm.h +++ b/xen/include/asm-x86/mm.h @@ -369,6 +369,20 @@ int get_page_from_l1e( l1_pgentry_t l1e, struct domain *l1e_owner, struct domain *pg_owner); void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner); +static inline bool get_page_from_mfn(mfn_t mfn, struct domain *d) +{ + struct page_info *page = __mfn_to_page(mfn_x(mfn)); + + if ( unlikely(!mfn_valid(mfn)) || unlikely(!get_page(page, d)) ) + { + gdprintk(XENLOG_WARNING, + "Could not get page ref for mfn %"PRI_mfn"\n", mfn_x(mfn)); + return false; + } + + return true; +} + static inline void put_page_and_type(struct page_info *page) { put_page_type(page);
It will be used later in multiple files. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- xen/arch/x86/mm.c | 16 ---------------- xen/include/asm-x86/mm.h | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 16 deletions(-)