gntdev warning fixes

List overview All Threads
Download

newer

older

[PATCH v10] xen/gntdev: Ignore...

Dearest beloved in the Lord,

Demi Marie Obenour

8 Jul 2022 8 Jul '22

4:37 p.m.

The recent gntdev fixes introduced two incorrect WARN_ON()s, which triggered immediately on my system. Patches for master and all supported stable versions follow.

Show replies by date

Demi Marie Obenour

8 Jul 8 Jul

4:37 p.m.

New subject: [PATCH 4.19] Ignore failure to unmap -1

The error paths of gntdev_mmap() can call unmap_grant_pages() even though not all of the pages have been successfully mapped. This will trigger the WARN_ON()s in __unmap_grant_pages_done(). The number of warnings can be very large; I have observed thousands of lines of warnings in the systemd journal.

Avoid this problem by only warning on unmapping failure if the handle being unmapped is not -1. The handle field of any page that was not successfully mapped will be -1, so this catches all cases wherr unmapping can legitimately fail.

Suggested-by: Juergen Gross jgross@suse.com Cc: stable@vger.kernel.org Signed-off-by: Demi Marie Obenour demi@invisiblethingslab.com Fixes: 73e9e72247b9 ("xen/gntdev: Avoid blocking in unmap_grant_pages()") --- drivers/xen/gntdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 492084814f55d8ed46d2d656db75b28a91dd7f06..27d955c5d9f9076266f77b9fedfa1a6a2ba08f56 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -416,7 +416,8 @@ static void __unmap_grant_pages_done(int result, unsigned int offset = data->unmap_ops - map->unmap_ops;

for (i = 0; i < data->count; i++) { - WARN_ON(map->unmap_ops[offset+i].status); + WARN_ON(map->unmap_ops[offset+i].status && + map->unmap_ops[offset+i].handle != -1); pr_debug("unmap handle=%d st=%d\n", map->unmap_ops[offset+i].handle, map->unmap_ops[offset+i].status);

-- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab

Demi Marie Obenour

4:37 p.m.

New subject: [PATCH 4.14] Ignore failure to unmap -1

Suggested-by: Juergen Gross jgross@suse.com Cc: stable@vger.kernel.org Signed-off-by: Demi Marie Obenour demi@invisiblethingslab.com Fixes: 2fe26a9a7048 ("xen/gntdev: Avoid blocking in unmap_grant_pages()") --- drivers/xen/gntdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 2827015604fbaa45f0ca60c342bbf71ce4132dec..799173755b785e2f3e2483855f75af1eba8e9373 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -393,7 +393,8 @@ static void __unmap_grant_pages_done(int result, unsigned int offset = data->unmap_ops - map->unmap_ops;

-- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab

Demi Marie Obenour

4:37 p.m.

New subject: [PATCH 5.4] Ignore failure to unmap -1

Suggested-by: Juergen Gross jgross@suse.com Cc: stable@vger.kernel.org Signed-off-by: Demi Marie Obenour demi@invisiblethingslab.com Fixes: ee25841221c1 ("xen/gntdev: Avoid blocking in unmap_grant_pages()") --- drivers/xen/gntdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index f464793477650e631c8928e85c1990c5964c2e94..bba849e5d8a7b4d54925b842fbe3c6792e0f0214 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -413,7 +413,8 @@ static void __unmap_grant_pages_done(int result, unsigned int offset = data->unmap_ops - map->unmap_ops;

-- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab

Demi Marie Obenour

4:37 p.m.

New subject: [PATCH 5.10] Ignore failure to unmap -1

Suggested-by: Juergen Gross jgross@suse.com Cc: stable@vger.kernel.org Signed-off-by: Demi Marie Obenour demi@invisiblethingslab.com Fixes: 79963021fd71 ("xen/gntdev: Avoid blocking in unmap_grant_pages()") --- drivers/xen/gntdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index f415c056ff8ab8d808ee2bacfaa3cad57af28204..54fee4087bf1078803c230ad2081aafa8415cf53 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -401,7 +401,8 @@ static void __unmap_grant_pages_done(int result, unsigned int offset = data->unmap_ops - map->unmap_ops;

-- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab

Demi Marie Obenour

4:37 p.m.

New subject: [PATCH 5.15] Ignore failure to unmap INVALID_GRANT_HANDLE

Avoid this problem by only warning on unmapping failure if the handle being unmapped is not INVALID_GRANT_HANDLE. The handle field of any page that was not successfully mapped will be INVALID_GRANT_HANDLE, so this catches all cases where unmapping can legitimately fail.

Suggested-by: Juergen Gross jgross@suse.com Cc: stable@vger.kernel.org Signed-off-by: Demi Marie Obenour demi@invisiblethingslab.com Fixes: 87a54feba68f ("xen/gntdev: Avoid blocking in unmap_grant_pages()") --- drivers/xen/gntdev.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 4b56c39f766d4da68570d08d963f6ef40c8d9c37..22fcd503f4a4487d0aed147c94f432683dad8c73 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -396,13 +396,17 @@ static void __unmap_grant_pages_done(int result, unsigned int offset = data->unmap_ops - map->unmap_ops;

for (i = 0; i < data->count; i++) { - WARN_ON(map->unmap_ops[offset+i].status); + WARN_ON(map->unmap_ops[offset+i].status && + map->unmap_ops[offset+i].handle != + INVALID_GRANT_HANDLE); pr_debug("unmap handle=%d st=%d\n", map->unmap_ops[offset+i].handle, map->unmap_ops[offset+i].status); map->unmap_ops[offset+i].handle = INVALID_GRANT_HANDLE; if (use_ptemod) { - WARN_ON(map->kunmap_ops[offset+i].status); + WARN_ON(map->kunmap_ops[offset+i].status && + map->kunmap_ops[offset+i].handle != + INVALID_GRANT_HANDLE); pr_debug("kunmap handle=%u st=%d\n", map->kunmap_ops[offset+i].handle, map->kunmap_ops[offset+i].status);

-- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab

Demi Marie Obenour

4:37 p.m.

New subject: [PATCH 5.18] Ignore failure to unmap INVALID_GRANT_HANDLE

Suggested-by: Juergen Gross jgross@suse.com Cc: stable@vger.kernel.org Signed-off-by: Demi Marie Obenour demi@invisiblethingslab.com Fixes: d4a49d20cd7c ("xen/gntdev: Avoid blocking in unmap_grant_pages()") --- drivers/xen/gntdev.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)

-- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab

Demi Marie Obenour

4:37 p.m.

New subject: [PATCH master] Ignore failure to unmap INVALID_GRANT_HANDLE

Suggested-by: Juergen Gross jgross@suse.com Cc: stable@vger.kernel.org Signed-off-by: Demi Marie Obenour demi@invisiblethingslab.com Fixes: dbe97cff7dd9 ("xen/gntdev: Avoid blocking in unmap_grant_pages()") --- drivers/xen/gntdev.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)

-- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab

Oleksandr Tyshchenko

10 Jul 10 Jul

7:19 p.m.

New subject: [PATCH master] Ignore failure to unmap INVALID_GRANT_HANDLE

On 08.07.22 19:37, Demi Marie Obenour wrote:

Hello Demi Marie

Please add subsystem to the commit subject, for example "xen/gntdev:".

...

The error paths of gntdev_mmap() can call unmap_grant_pages() even though not all of the pages have been successfully mapped. This will trigger the WARN_ON()s in __unmap_grant_pages_done(). The number of warnings can be very large; I have observed thousands of lines of warnings in the systemd journal.

Avoid this problem by only warning on unmapping failure if the handle being unmapped is not INVALID_GRANT_HANDLE. The handle field of any page that was not successfully mapped will be INVALID_GRANT_HANDLE, so this catches all cases where unmapping can legitimately fail.

Suggested-by: Juergen Gross jgross@suse.com Cc: stable@vger.kernel.org Signed-off-by: Demi Marie Obenour demi@invisiblethingslab.com Fixes: dbe97cff7dd9 ("xen/gntdev: Avoid blocking in unmap_grant_pages()")

drivers/xen/gntdev.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 4b56c39f766d4da68570d08d963f6ef40c8d9c37..22fcd503f4a4487d0aed147c94f432683dad8c73 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -396,13 +396,17 @@ static void __unmap_grant_pages_done(int result, unsigned int offset = data->unmap_ops - map->unmap_ops; for (i = 0; i < data->count; i++) {
WARN_ON(map->unmap_ops[offset+i].status);
WARN_ON(map->unmap_ops[offset+i].status &&
	map->unmap_ops[offset+i].handle !=
	INVALID_GRANT_HANDLE);

Nit: While extending the check I would clarify the first half of it: "map->unmap_ops[offset+i].status != GNTST_okay"

...

pr_debug("unmap handle=%d st=%d\n",
	map->unmap_ops[offset+i].handle,
	map->unmap_ops[offset+i].status);
map->unmap_ops[offset+i].handle = INVALID_GRANT_HANDLE;
if (use_ptemod) {

	WARN_ON(map->kunmap_ops[offset+i].status);

	WARN_ON(map->kunmap_ops[offset+i].status &&

```
		map->kunmap_ops[offset+i].handle !=
```
```
		INVALID_GRANT_HANDLE);
```

ditto

With updated subject:

Reviewed-by: Oleksandr Tyshchenko oleksandr_tyshchenko@epam.com

...

	pr_debug("kunmap handle=%u st=%d\n",
		 map->kunmap_ops[offset+i].handle,
		 map->kunmap_ops[offset+i].status);

-- Regards, Oleksandr Tyshchenko

Greg KH

9 Jul 9 Jul

6:34 a.m.

On Fri, Jul 08, 2022 at 12:37:44PM -0400, Demi Marie Obenour wrote:

...

The recent gntdev fixes introduced two incorrect WARN_ON()s, which triggered immediately on my system. Patches for master and all supported stable versions follow.

This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly.

</formletter>

916

days inactive

918

days old

linux-stable-mirror@lists.linaro.org

9 comments

participants

tags (0)

participants (3)

Demi Marie Obenour
Greg KH
Oleksandr Tyshchenko