diff mbox series

block: restore the old set_task_ioprio() behaviour wrt PF_EXITING

Message ID 20220328085928.7899-1-jslaby@suse.cz (mailing list archive)
State New, archived
Headers show
Series block: restore the old set_task_ioprio() behaviour wrt PF_EXITING | expand

Commit Message

Jiri Slaby March 28, 2022, 8:59 a.m. UTC 
PF_EXITING tasks were silently ignored before the below commits.
Continue doing so. Otherwise python-psutil tests fail:
  ERROR: psutil.tests.test_process.TestProcess.test_zombie_process
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/build/lib.linux-x86_64-3.9/psutil/_pslinux.py", line 1661, in wrapper
      return fun(self, *args, **kwargs)
    File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/build/lib.linux-x86_64-3.9/psutil/_pslinux.py", line 2133, in ionice_set
      return cext.proc_ioprio_set(self.pid, ioclass, value)
  ProcessLookupError: [Errno 3] No such process

  During handling of the above exception, another exception occurred:

  Traceback (most recent call last):
    File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/psutil/tests/test_process.py", line 1313, in test_zombie_process
      succeed_or_zombie_p_exc(fun)
    File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/psutil/tests/test_process.py", line 1288, in succeed_or_zombie_p_exc
      return fun()
    File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/build/lib.linux-x86_64-3.9/psutil/__init__.py", line 792, in ionice
      return self._proc.ionice_set(ioclass, value)
    File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/build/lib.linux-x86_64-3.9/psutil/_pslinux.py", line 1665, in wrapper
      raise NoSuchProcess(self.pid, self._name)
  psutil.NoSuchProcess: process no longer exists (pid=2057)

Cc: Christoph Hellwig <hch@lst.de>
Cc: Jan Kara <jack@suse.cz>
Cc: Jens Axboe <axboe@kernel.dk>
Fixes: 5fc11eebb4 (block: open code create_task_io_context in set_task_ioprio)
Fixes: a957b61254 (block: fix error in handling dead task for ioprio setting)
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 block/blk-ioc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Jan Kara March 28, 2022, 9:59 a.m. UTC | #1 
On Mon 28-03-22 10:59:28, Jiri Slaby wrote:
> PF_EXITING tasks were silently ignored before the below commits.
> Continue doing so. Otherwise python-psutil tests fail:
>   ERROR: psutil.tests.test_process.TestProcess.test_zombie_process
>   ----------------------------------------------------------------------
>   Traceback (most recent call last):
>     File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/build/lib.linux-x86_64-3.9/psutil/_pslinux.py", line 1661, in wrapper
>       return fun(self, *args, **kwargs)
>     File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/build/lib.linux-x86_64-3.9/psutil/_pslinux.py", line 2133, in ionice_set
>       return cext.proc_ioprio_set(self.pid, ioclass, value)
>   ProcessLookupError: [Errno 3] No such process
> 
>   During handling of the above exception, another exception occurred:
> 
>   Traceback (most recent call last):
>     File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/psutil/tests/test_process.py", line 1313, in test_zombie_process
>       succeed_or_zombie_p_exc(fun)
>     File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/psutil/tests/test_process.py", line 1288, in succeed_or_zombie_p_exc
>       return fun()
>     File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/build/lib.linux-x86_64-3.9/psutil/__init__.py", line 792, in ionice
>       return self._proc.ionice_set(ioclass, value)
>     File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/build/lib.linux-x86_64-3.9/psutil/_pslinux.py", line 1665, in wrapper
>       raise NoSuchProcess(self.pid, self._name)
>   psutil.NoSuchProcess: process no longer exists (pid=2057)
> 
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jens Axboe <axboe@kernel.dk>
> Fixes: 5fc11eebb4 (block: open code create_task_io_context in set_task_ioprio)
> Fixes: a957b61254 (block: fix error in handling dead task for ioprio setting)
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>

OK, as much as I find it disputable what ioprio_set() should be doing for
PF_EXITTING task, I guess there's no harm in maintaining the old behavior and
just ignoring the call. So feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

									Honza

> ---
>  block/blk-ioc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index 11f49f78db32..df9cfe4ca532 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -280,7 +280,6 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
>  
>  		task_lock(task);
>  		if (task->flags & PF_EXITING) {
> -			err = -ESRCH;
>  			kmem_cache_free(iocontext_cachep, ioc);
>  			goto out;
>  		}
> @@ -292,7 +291,7 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
>  	task->io_context->ioprio = ioprio;
>  out:
>  	task_unlock(task);
> -	return err;
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(set_task_ioprio);
>  
> -- 
> 2.35.1
>
Jens Axboe March 28, 2022, 12:36 p.m. UTC | #2 
On Mon, 28 Mar 2022 10:59:28 +0200, Jiri Slaby wrote:
> PF_EXITING tasks were silently ignored before the below commits.
> Continue doing so. Otherwise python-psutil tests fail:
>   ERROR: psutil.tests.test_process.TestProcess.test_zombie_process
>   ----------------------------------------------------------------------
>   Traceback (most recent call last):
>     File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/build/lib.linux-x86_64-3.9/psutil/_pslinux.py", line 1661, in wrapper
>       return fun(self, *args, **kwargs)
>     File "/home/abuild/rpmbuild/BUILD/psutil-5.9.0/build/lib.linux-x86_64-3.9/psutil/_pslinux.py", line 2133, in ionice_set
>       return cext.proc_ioprio_set(self.pid, ioclass, value)
>   ProcessLookupError: [Errno 3] No such process
> 
> [...]

Applied, thanks!

[1/1] block: restore the old set_task_ioprio() behaviour wrt PF_EXITING
      commit: 15583a563cd5a7358e975599b7de7caacd9e9ce9

Best regards,
Christoph Hellwig March 29, 2022, 6:55 a.m. UTC | #3 
>  		task_lock(task);
>  		if (task->flags & PF_EXITING) {
> -			err = -ESRCH;
>  			kmem_cache_free(iocontext_cachep, ioc);
>  			goto out;

Please add a commnt here why we are not returning an error, otherwise
this is just getting "fixed" over and over again.
diff mbox series

Patch

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 11f49f78db32..df9cfe4ca532 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -280,7 +280,6 @@  int set_task_ioprio(struct task_struct *task, int ioprio)
 
 		task_lock(task);
 		if (task->flags & PF_EXITING) {
-			err = -ESRCH;
 			kmem_cache_free(iocontext_cachep, ioc);
 			goto out;
 		}
@@ -292,7 +291,7 @@  int set_task_ioprio(struct task_struct *task, int ioprio)
 	task->io_context->ioprio = ioprio;
 out:
 	task_unlock(task);
-	return err;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(set_task_ioprio);