All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joerg Roedel <joro@8bytes.org>
To: Eric Auger <eric.auger@redhat.com>
Cc: eric.auger.pro@gmail.com, iommu@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, dwmw2@infradead.org,
	lorenzo.pieralisi@arm.com, robin.murphy@arm.com,
	will.deacon@arm.com, hanjun.guo@linaro.org, sudeep.holla@arm.com,
	alex.williamson@redhat.com, shameerali.kolothum.thodi@huawei.com
Subject: Re: [PATCH v4 3/8] iommu/vt-d: Duplicate iommu_resv_region objects per device list
Date: Mon, 27 May 2019 17:23:04 +0200	[thread overview]
Message-ID: <20190527152303.GD12745@8bytes.org> (raw)
In-Reply-To: <20190527085541.5294-4-eric.auger@redhat.com>

On Mon, May 27, 2019 at 10:55:36AM +0200, Eric Auger wrote:
> -			list_add_tail(&rmrr->resv->list, head);
> +			length = rmrr->end_address - rmrr->base_address + 1;
> +			resv = iommu_alloc_resv_region(rmrr->base_address,
> +						       length, prot,
> +						       IOMMU_RESV_DIRECT,
> +						       GFP_ATOMIC);
> +			if (!resv)
> +				break;
> +
> +			list_add_tail(&resv->list, head);

Okay, so this happens in a rcu_read_locked section and must be atomic,
but I don't like this extra parameter to iommu_alloc_resv_region().

How about replacing the rcu-lock with the dmar_global_lock, which
protects against changes to the global rmrr list? This will make this
loop preemptible and taking the global lock is okay because this
function is in no way performance relevant.

Regards,

	Joerg

WARNING: multiple messages have this Message-ID (diff)
From: Joerg Roedel <joro@8bytes.org>
To: Eric Auger <eric.auger@redhat.com>
Cc: alex.williamson@redhat.com, dwmw2@infradead.org,
	will.deacon@arm.com, linux-kernel@vger.kernel.org,
	iommu@lists.linux-foundation.org, sudeep.holla@arm.com,
	robin.murphy@arm.com, eric.auger.pro@gmail.com
Subject: Re: [PATCH v4 3/8] iommu/vt-d: Duplicate iommu_resv_region objects per device list
Date: Mon, 27 May 2019 17:23:04 +0200	[thread overview]
Message-ID: <20190527152303.GD12745@8bytes.org> (raw)
In-Reply-To: <20190527085541.5294-4-eric.auger@redhat.com>

On Mon, May 27, 2019 at 10:55:36AM +0200, Eric Auger wrote:
> -			list_add_tail(&rmrr->resv->list, head);
> +			length = rmrr->end_address - rmrr->base_address + 1;
> +			resv = iommu_alloc_resv_region(rmrr->base_address,
> +						       length, prot,
> +						       IOMMU_RESV_DIRECT,
> +						       GFP_ATOMIC);
> +			if (!resv)
> +				break;
> +
> +			list_add_tail(&resv->list, head);

Okay, so this happens in a rcu_read_locked section and must be atomic,
but I don't like this extra parameter to iommu_alloc_resv_region().

How about replacing the rcu-lock with the dmar_global_lock, which
protects against changes to the global rmrr list? This will make this
loop preemptible and taking the global lock is okay because this
function is in no way performance relevant.

Regards,

	Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply	other threads:[~2019-05-27 15:23 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-27  8:55 [PATCH v4 0/8] RMRR related fixes and enhancements Eric Auger
2019-05-27  8:55 ` Eric Auger
2019-05-27  8:55 ` [PATCH v4 1/8] iommu: Fix a leak in iommu_insert_resv_region Eric Auger
2019-05-27  8:55   ` Eric Auger
2019-05-27  8:55 ` [PATCH v4 2/8] iommu: Pass a GFP flag parameter to iommu_alloc_resv_region() Eric Auger
2019-05-27  8:55   ` Eric Auger
2019-05-27  8:55 ` [PATCH v4 3/8] iommu/vt-d: Duplicate iommu_resv_region objects per device list Eric Auger
2019-05-27  8:55   ` Eric Auger
2019-05-27 15:23   ` Joerg Roedel [this message]
2019-05-27 15:23     ` Joerg Roedel
2019-05-28 11:51     ` Auger Eric
2019-05-28 11:51       ` Auger Eric
2019-05-27  8:55 ` [PATCH v4 4/8] iommu/vt-d: Introduce is_downstream_to_pci_bridge helper Eric Auger
2019-05-27  8:55   ` Eric Auger
2019-05-27  8:55 ` [PATCH v4 5/8] iommu/vt-d: Handle RMRR with PCI bridge device scopes Eric Auger
2019-05-27  8:55   ` Eric Auger
2019-05-27  8:55 ` [PATCH v4 6/8] iommu/vt-d: Handle PCI bridge RMRR device scopes in intel_iommu_get_resv_regions Eric Auger
2019-05-27  8:55   ` Eric Auger
2019-05-27  8:55 ` [PATCH v4 7/8] iommu: Introduce IOMMU_RESV_DIRECT_RELAXABLE reserved memory regions Eric Auger
2019-05-27  8:55   ` Eric Auger
2019-05-27  8:55 ` [PATCH v4 8/8] iommu/vt-d: Differentiate relaxable and non relaxable RMRRs Eric Auger
2019-05-27  8:55   ` Eric Auger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190527152303.GD12745@8bytes.org \
    --to=joro@8bytes.org \
    --cc=alex.williamson@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=eric.auger.pro@gmail.com \
    --cc=eric.auger@redhat.com \
    --cc=hanjun.guo@linaro.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=robin.murphy@arm.com \
    --cc=shameerali.kolothum.thodi@huawei.com \
    --cc=sudeep.holla@arm.com \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.