RE: [Patch v8 03/10] vfio/type1: Report iommu nesting info to userspace

["Liu, Yi L" <yi.l.liu@intel.com>]

Hi Jason,

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Tuesday, March 2, 2021 8:52 PM
> 
> On Wed, Mar 03, 2021 at 04:35:38AM +0800, Liu Yi L wrote:
> > diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c
> > index 4bb162c1d649..3a5c84d4f19b 100644
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -63,22 +63,24 @@ MODULE_PARM_DESC(dma_entry_limit,
> >  		 "Maximum number of user DMA mappings per container
> (65535).");
> >
> >  struct vfio_iommu {
> > -	struct list_head	domain_list;
> > -	struct list_head	iova_list;
> > -	struct vfio_domain	*external_domain; /* domain for external
> user */
> > -	struct mutex		lock;
> > -	struct rb_root		dma_list;
> > -	struct blocking_notifier_head notifier;
> > -	unsigned int		dma_avail;
> > -	unsigned int		vaddr_invalid_count;
> > -	uint64_t		pgsize_bitmap;
> > -	uint64_t		num_non_pinned_groups;
> > -	wait_queue_head_t	vaddr_wait;
> > -	bool			v2;
> > -	bool			nesting;
> > -	bool			dirty_page_tracking;
> > -	bool			pinned_page_dirty_scope;
> > -	bool			container_open;
> > +	struct list_head		domain_list;
> > +	struct list_head		iova_list;
> > +	/* domain for external user */
> > +	struct vfio_domain		*external_domain;
> > +	struct mutex			lock;
> > +	struct rb_root			dma_list;
> > +	struct blocking_notifier_head	notifier;
> > +	unsigned int			dma_avail;
> > +	unsigned int			vaddr_invalid_count;
> > +	uint64_t			pgsize_bitmap;
> > +	uint64_t			num_non_pinned_groups;
> > +	wait_queue_head_t		vaddr_wait;
> > +	struct iommu_nesting_info	*nesting_info;
> > +	bool				v2;
> > +	bool				nesting;
> > +	bool				dirty_page_tracking;
> > +	bool				pinned_page_dirty_scope;
> > +	bool				container_open;
> >  };
> 
> I always hate seeing one line patches done like this. If you want to
> re-indent you should remove the horizontal whitespace, not add an
> unreadable amount more.

Oops. will be careful in next version. Perhaps no need to re-indent
the existing fields to avoid the whitespace?

> 
> Also, Linus has been unhappy before to see lists of bool's in structs
> due to the huge amount of memory they waste.

How about something like below? I can do it if Alex is fine with it.

		u64 v2:1;
		u64 nesting:1;
		u64 dirty_page_tracking:1;
		u64 pinned_page_dirty_scope:1;
		u64 container_open:1;
		u64 reserved:59;

And thanks for sharing me what Linus prefers.

Regards,
Yi Liu

> Jason