Re: [Xen-devel] [PATCH v6 5/5] x86/mem_sharing: style cleanup

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Thu, Jul 18, 2019 at 4:56 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
>
> On 17.07.2019 21:33, Tamas K Lengyel wrote:
> > @@ -136,8 +137,8 @@ static inline bool _page_lock(struct page_info *page)
> >               cpu_relax();
> >           nx = x + (1 | PGT_locked);
> >           if ( !(x & PGT_validated) ||
> > -             !(x & PGT_count_mask) ||
> > -             !(nx & PGT_count_mask) )
> > +                !(x & PGT_count_mask) ||
> > +                !(nx & PGT_count_mask) )
> >               return false;
> >       } while ( cmpxchg(&page->u.inuse.type_info, x, nx) != x );
>
> Aren't you screwing up indentation here? It looks wrong both in my
> mail client's view and on the list archives, whereas. Furthermore
> this is code you've introduced earlier in the series, so it should
> be got right there, not here.

The style was auto-applied with astyle using the bsd format. In the
previous patch there were no style-changes applied because it was a
copy-paste job from the other code location. I rather keep
code-copying and style fixes separate.

>
> > @@ -225,7 +225,7 @@ rmap_init(struct page_info *page)
> >   #define HASH(domain, gfn)       \
> >       (((gfn) + (domain)) % RMAP_HASHTAB_SIZE)
> >
> > -/* Conversions. Tuned by the thresholds. Should only happen twice
> > +/* Conversions. Tuned by the thresholds. Should only happen twice
> >    * (once each) during the lifetime of a shared page */
>
> Please fix the comment style as a whole, not just the stray trailing
> blank.
>
> > @@ -288,13 +288,13 @@ rmap_count(struct page_info *pg)
> >   }
> >
> >   /* The page type count is always decreased after removing from the rmap.
> > - * Use a convert flag to avoid mutating the rmap if in the middle of an
> > + * Use a convert flag to avoid mutating the rmap if in the middle of an
> >    * iterator, or if the page will be soon destroyed anyways. */
>
> Same here.
>
> >   static inline void
> >   rmap_del(gfn_info_t *gfn_info, struct page_info *page, int convert)
> >   {
> >       if ( RMAP_USES_HASHTAB(page) && convert &&
> > -         (rmap_count(page) <= RMAP_LIGHT_SHARED_PAGE) )
> > +            (rmap_count(page) <= RMAP_LIGHT_SHARED_PAGE) )
>
> Here you again seem to be screwing up correct indentation. There are
> more such instances, so I guess I'll leave it to you to go over the
> whole patch once more.

Again, this is the astyle bsd format auto-applied. I rather have this
style if it means I don't ever have to check for this kind of stuff
manually in the future.

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel