Redundant code in fn identity_mapping of intel-iommu.c (intel IOMMU driver)?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

I hope this is the right place for driver code related queries.

I came across some redundant checks in function identity_mapping of file
intel-iommu.c
(https://github.com/torvalds/linux/blob/master/drivers/iommu/intel-iommu.c#L2681).
This function is called by function iommu_no_mapping of same file
(https://github.com/torvalds/linux/blob/master/drivers/iommu/intel-iommu.c#L3452):

	static int iommu_no_mapping(struct device *dev)
	{
		int found;

		if (iommu_dummy(dev))
		/* dev->archdata.iommu == DUMMY_DEVICE_DOMAIN_INFO */
			return 1;

		if (!iommu_identity_mapping)
			return 0;

		found = identity_mapping(dev);	/* this function */
		if (found) {
	// ...

The function iommu_dummy is checking whether the device belongs to the
dummy domain, DUMMY_DEVICE_DOMAIN_INFO.  Followed by that we're checking 
whether identity mapping is set.
After that we come to the identity_mapping function (comments are mine):

	static int identity_mapping(struct device *dev)
	{
		struct device_domain_info *info;

		if (likely(!iommu_identity_mapping))
		/* we just checked this expression and we are here because it
		   was false */
			return 0;

		info = dev->archdata.iommu;
		if (info && info != DUMMY_DEVICE_DOMAIN_INFO)
		/* this is just !iommu_dummy(dev); which we know is the case */
			return (info->domain == si_domain);

		return 0;
	}

If we inline that code we get:

	int found;
	struct device_domain_info *info;

	/* iommu_dummy */
	if (dev->archdata.iommu == DUMMY_DEVICE_DOMAIN_INFO)
		return 1;

	if (!iommu_identity_mapping)
		return 0;

	/* identity_mapping */
	if (likely(!iommu_identity_mapping)) /* redundant? */
		found = 0;

	found = 0;
	info = dev->archdata.iommu;
	if (info && info != DUMMY_DEVICE_DOMAIN_INFO) /* redundant? */
		found = (info->domain == si_domain);


No other function is calling identity_mapping so it couldn't be the case
that there can some other flows which require these checks.

I'm not sure whether this is intentional (doesn't seem to me).  Can anyone
confirm this?

I'm a newbie so I posted here first to check whether I'm not making that
"newbie" mistake.


Thanks and regards,
Abhishek

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@xxxxxxxxxxxxxxxxx
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies



[Index of Archives]     [Newbies FAQ]     [Linux Kernel Mentors]     [Linux Kernel Development]     [IETF Annouce]     [Git]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux SCSI]     [Linux ACPI]
  Powered by Linux