On Wed, Apr 19, 2023 at 11:11:48AM -0700, Reinette Chatre wrote:
> Hi Alex,
>
> On 4/18/2023 3:38 PM, Alex Williamson wrote:
> > On Tue, 18 Apr 2023 10:29:19 -0700
> > Reinette Chatre <reinette.chatre@xxxxxxxxx> wrote:
> >
>
> ...
>
> >> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
> >> index 148fd1ae6c1c..4f070f2d6fde 100644
> >> --- a/include/linux/vfio_pci_core.h
> >> +++ b/include/linux/vfio_pci_core.h
> >> @@ -67,6 +67,7 @@ struct vfio_pci_core_device {
> >> u8 msix_bar;
> >> u16 msix_size;
> >> u32 msix_offset;
> >> + bool has_dyn_msix;
> >> u32 rbar[7];
> >> bool pci_2_3;
> >> bool virq_disabled;
> >
> > Nit, the whole data structure probably needs to be sorted with pahole,
> > but creating a hole here for locality to other msix fields should
> > probably be secondary to keeping the structure well packed, which
> > suggests including this new field among the bools below. Thanks,
>
> Thanks for catching this. Moving it one field lower as shown in the
> delta patch below seems to improve the layout:
>
> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
> index 4f070f2d6fde..d730d78754a2 100644
> --- a/include/linux/vfio_pci_core.h
> +++ b/include/linux/vfio_pci_core.h
> @@ -67,8 +67,8 @@ struct vfio_pci_core_device {
> u8 msix_bar;
> u16 msix_size;
> u32 msix_offset;
> - bool has_dyn_msix;
> u32 rbar[7];
> + bool has_dyn_msix;
> bool pci_2_3;
> bool virq_disabled;
> bool reset_works;
Also, Linus on record as strongly disliking these lists of bools
If they don't need read_once/etc stuff then use a list of bitfields
bool abc:1;
bool xyz:1;
Jason
