Re: [Xen-devel] [PATCH v3 05/10] xen/arm: Setup virtual paging for non-boot CPUs on hotplug/resume

[Julien Grall <julien.grall@xxxxxxx>]

On 08/05/18 15:28, Mirela Simonovic wrote:
> Hi Julien,
>
> On Tue, May 8, 2018 at 4:14 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
> > On 07/05/18 15:55, Mirela Simonovic wrote:
> > > Hi Julien,
> >
> > Hi Mirela,
> >
> > > On Mon, Apr 30, 2018 at 4:47 PM, Julien Grall <julien.grall@xxxxxxx>
> > > wrote:
> > > > On 27/04/18 18:12, Mirela Simonovic wrote:
> > > > >         printk("P2M: %d levels with order-%d root, VTCR 0x%lx\n",
> > > > > -           4 - P2M_ROOT_LEVEL, P2M_ROOT_ORDER, val);
> > > > > +           4 - P2M_ROOT_LEVEL, P2M_ROOT_ORDER, vtcr);
> > > > >           p2m_vmid_allocator_init();
> > > > >           /* It is not allowed to concatenate a level zero root */
> > > > >         BUG_ON( P2M_ROOT_LEVEL == 0 && P2M_ROOT_ORDER > 0 );
> > > > > -    setup_virt_paging_one((void *)val);
> > > > > -    smp_call_function(setup_virt_paging_one, (void *)val, 1);
> > > > > +    setup_virt_paging_one(NULL);
> > > > > +    smp_call_function(setup_virt_paging_one, NULL, 1);
> > > > > +}
> > > > > +
> > > > > +static int cpu_virt_paging_callback(
> > > > > +    struct notifier_block *nfb, unsigned long action, void *hcpu)
> > > >
> > > >
> > > > The indentation looks wrong.
> > > Editor indented this for me and it looks the same as in other places
> > > where a notifier is defined. I did
> > > grep -r "struct notifier_block \*nfb,"
> > > to check. It looks weird but seems correct
> >
> > Indeed, I am not sure why it is done like that for notifiers. I can't see
> > any reason to split like that given the first parameter can fit on the first
> > line without hitting the 80 columns.
> >
> > So I would much prefer if we follow Xen coding style:
> >
> > static int cpu_virt_paging_callback(struct notifier_block *nfb,
> >                                      unsigned long action,
> >                                      void *hcpu);
> Please just one more clarification: why did you split line after 2nd
> argument? 3rd argument could fit in 80 chars.
Both are valid. I feel having the argument separated here is easier to read.

> The only coding style I found is in CODING_STYLE file, which doesn't
> specify that much details - I'm just trying to understand where to
> find more info in order to avoid coding style-related iterations in
> future. Is there any other source specifying coding style for Xen?
Sadly the coding style is not very well formalized. At the moment it is 
more a consensus between all the reviewers. There are work going on to 
fix that by introducing a checkpatch.pl.
Cheers,

--
Julien Grall

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