diff mbox series

[4/8] block: Fix handling of tasks without ioprio in ioprio_get(2)

Message ID 20220620161153.11741-4-jack@suse.cz (mailing list archive)
State New, archived
Headers show
Series block: Fix IO priority mess | expand

Commit Message

Jan Kara June 20, 2022, 4:11 p.m. UTC 
ioprio_get(2) can be asked to return the best IO priority from several
tasks (IOPRIO_WHO_PGRP, IOPRIO_WHO_USER). Currently the call treats
tasks without set IO priority as having priority
IOPRIO_CLASS_BE/IOPRIO_BE_NORM however this does not really reflect the
IO priority the task will get (which depends on task's nice value).

Fix the code to use the real IO priority task's IO will use. For this we
do some factoring out to share the code converting task's CPU priority
to IO priority and we also have to modify code for
ioprio_get(IOPRIO_WHO_PROCESS) to keep returning IOPRIO_CLASS_NONE
priority for tasks without set IO priority as a special case to maintain
userspace visible API.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/ioprio.c         | 49 ++++++++++++++++++++++++++++++++++++------
 include/linux/ioprio.h | 19 +++-------------
 2 files changed, 45 insertions(+), 23 deletions(-)

Comments

kernel test robot June 20, 2022, 8:28 p.m. UTC | #1 
Hi Jan,

I love your patch! Yet something to improve:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Jan-Kara/block-Fix-IO-priority-mess/20220621-001427
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: s390-randconfig-r044-20220620 (https://download.01.org/0day-ci/archive/20220621/202206210431.uTcCuUoS-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project af6d2a0b6825e71965f3e2701a63c239fa0ad70f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/73f39284cec50db7a3e973a9a4ed56f7f706dd1b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jan-Kara/block-Fix-IO-priority-mess/20220621-001427
        git checkout 73f39284cec50db7a3e973a9a4ed56f7f706dd1b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   /opt/cross/gcc-11.3.0-nolibc/s390x-linux/bin/s390x-linux-ld: fs/read_write.o: in function `get_current_ioprio':
>> include/linux/ioprio.h:53: undefined reference to `__get_task_ioprio'
>> /opt/cross/gcc-11.3.0-nolibc/s390x-linux/bin/s390x-linux-ld: include/linux/ioprio.h:53: undefined reference to `__get_task_ioprio'
>> /opt/cross/gcc-11.3.0-nolibc/s390x-linux/bin/s390x-linux-ld: include/linux/ioprio.h:53: undefined reference to `__get_task_ioprio'
>> /opt/cross/gcc-11.3.0-nolibc/s390x-linux/bin/s390x-linux-ld: include/linux/ioprio.h:53: undefined reference to `__get_task_ioprio'
>> /opt/cross/gcc-11.3.0-nolibc/s390x-linux/bin/s390x-linux-ld: include/linux/ioprio.h:53: undefined reference to `__get_task_ioprio'
   /opt/cross/gcc-11.3.0-nolibc/s390x-linux/bin/s390x-linux-ld: fs/seq_file.o:include/linux/ioprio.h:53: more undefined references to `__get_task_ioprio' follow


vim +53 include/linux/ioprio.h

    50	
    51	static inline int get_current_ioprio(void)
    52	{
  > 53		return __get_task_ioprio(current);
    54	}
    55
kernel test robot June 20, 2022, 8:28 p.m. UTC | #2 
Hi Jan,

I love your patch! Yet something to improve:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Jan-Kara/block-Fix-IO-priority-mess/20220621-001427
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: x86_64-randconfig-a003-20220620 (https://download.01.org/0day-ci/archive/20220621/202206210442.Th4HzcuP-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/73f39284cec50db7a3e973a9a4ed56f7f706dd1b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jan-Kara/block-Fix-IO-priority-mess/20220621-001427
        git checkout 73f39284cec50db7a3e973a9a4ed56f7f706dd1b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: vmlinux.o: in function `get_current_ioprio':
>> include/linux/ioprio.h:53: undefined reference to `__get_task_ioprio'
>> ld: include/linux/ioprio.h:53: undefined reference to `__get_task_ioprio'
>> ld: include/linux/ioprio.h:53: undefined reference to `__get_task_ioprio'
>> ld: include/linux/ioprio.h:53: undefined reference to `__get_task_ioprio'
>> ld: include/linux/ioprio.h:53: undefined reference to `__get_task_ioprio'


vim +53 include/linux/ioprio.h

    50	
    51	static inline int get_current_ioprio(void)
    52	{
  > 53		return __get_task_ioprio(current);
    54	}
    55
kernel test robot June 20, 2022, 9:49 p.m. UTC | #3 
Hi Jan,

I love your patch! Yet something to improve:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Jan-Kara/block-Fix-IO-priority-mess/20220621-001427
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: riscv-nommu_k210_defconfig (https://download.01.org/0day-ci/archive/20220621/202206210522.eVU8S1DL-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/73f39284cec50db7a3e973a9a4ed56f7f706dd1b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jan-Kara/block-Fix-IO-priority-mess/20220621-001427
        git checkout 73f39284cec50db7a3e973a9a4ed56f7f706dd1b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   riscv64-linux-ld: fs/read_write.o: in function `.L10':
   read_write.c:(.text+0x80): undefined reference to `__get_task_ioprio'
   riscv64-linux-ld: fs/seq_file.o: in function `seq_read':
>> seq_file.c:(.text+0x458): undefined reference to `__get_task_ioprio'
   riscv64-linux-ld: fs/splice.o: in function `.L171':
>> splice.c:(.text+0x8c6): undefined reference to `__get_task_ioprio'
Damien Le Moal June 20, 2022, 11:57 p.m. UTC | #4 
On 6/21/22 01:11, Jan Kara wrote:
> ioprio_get(2) can be asked to return the best IO priority from several
> tasks (IOPRIO_WHO_PGRP, IOPRIO_WHO_USER). Currently the call treats
> tasks without set IO priority as having priority
> IOPRIO_CLASS_BE/IOPRIO_BE_NORM however this does not really reflect the
> IO priority the task will get (which depends on task's nice value).
> 
> Fix the code to use the real IO priority task's IO will use. For this we
> do some factoring out to share the code converting task's CPU priority
> to IO priority and we also have to modify code for
> ioprio_get(IOPRIO_WHO_PROCESS) to keep returning IOPRIO_CLASS_NONE
> priority for tasks without set IO priority as a special case to maintain
> userspace visible API.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  block/ioprio.c         | 49 ++++++++++++++++++++++++++++++++++++------
>  include/linux/ioprio.h | 19 +++-------------
>  2 files changed, 45 insertions(+), 23 deletions(-)
> 
> diff --git a/block/ioprio.c b/block/ioprio.c
> index b5cf7339709b..a4c19ce0de4c 100644
> --- a/block/ioprio.c
> +++ b/block/ioprio.c
> @@ -138,6 +138,27 @@ SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio)
>  	return ret;
>  }
>  
> +/*
> + * If the task has set an I/O priority, use that. Otherwise, return
> + * the default I/O priority.
> + */
> +int __get_task_ioprio(struct task_struct *p)
> +{
> +	struct io_context *ioc = p->io_context;
> +	int prio;
> +
> +	if (ioc)
> +		prio = ioc->ioprio;
> +	else
> +		prio = IOPRIO_DEFAULT;
> +
> +	if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE)
> +		prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(p),
> +					 task_nice_ioprio(p));
> +	return prio;
> +}
> +EXPORT_SYMBOL_GPL(__get_task_ioprio);
> +
>  static int get_task_ioprio(struct task_struct *p)
>  {
>  	int ret;
> @@ -145,10 +166,29 @@ static int get_task_ioprio(struct task_struct *p)
>  	ret = security_task_getioprio(p);
>  	if (ret)
>  		goto out;
> -	ret = IOPRIO_DEFAULT;
> +	task_lock(p);
> +	ret = __get_task_ioprio(p);
> +	task_unlock(p);
> +out:
> +	return ret;
> +}
> +
> +/*
> + * Return raw IO priority value as set by userspace. We use this for
> + * ioprio_get(pid, IOPRIO_WHO_PROCESS) so that we keep historical behavior and
> + * also so that userspace can distinguish unset IO priority (which just gets
> + * overriden based on task's nice value) from IO priority set to some value.
> + */
> +static int get_task_raw_ioprio(struct task_struct *p) { int ret;

The "int ret;" declaration is on the wrong line, so is the curly bracket.

> +
> +	ret = security_task_getioprio(p);
> +	if (ret)
> +		goto out;
>  	task_lock(p);
>  	if (p->io_context)
>  		ret = p->io_context->ioprio;
> +	else
> +		ret = IOPRIO_DEFAULT;
>  	task_unlock(p);
>  out:
>  	return ret;
> @@ -156,11 +196,6 @@ static int get_task_ioprio(struct task_struct *p)
>  
>  static int ioprio_best(unsigned short aprio, unsigned short bprio)
>  {
> -	if (!ioprio_valid(aprio))
> -		aprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM);
> -	if (!ioprio_valid(bprio))
> -		bprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM);
> -
>  	return min(aprio, bprio);
>  }

This function could be declared as inline now...

>  
> @@ -181,7 +216,7 @@ SYSCALL_DEFINE2(ioprio_get, int, which, int, who)
>  			else
>  				p = find_task_by_vpid(who);
>  			if (p)
> -				ret = get_task_ioprio(p);
> +				ret = get_task_raw_ioprio(p);
>  			break;
>  		case IOPRIO_WHO_PGRP:
>  			if (!who)
> diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h
> index 519d51fc8902..24e648dc4fb3 100644
> --- a/include/linux/ioprio.h
> +++ b/include/linux/ioprio.h
> @@ -46,24 +46,11 @@ static inline int task_nice_ioclass(struct task_struct *task)
>  		return IOPRIO_CLASS_BE;
>  }
>  
> -/*
> - * If the calling process has set an I/O priority, use that. Otherwise, return
> - * the default I/O priority.
> - */
> +int __get_task_ioprio(struct task_struct *p);
> +
>  static inline int get_current_ioprio(void)
>  {
> -	struct io_context *ioc = current->io_context;
> -	int prio;
> -
> -	if (ioc)
> -		prio = ioc->ioprio;
> -	else
> -		prio = IOPRIO_DEFAULT;
> -
> -	if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE)
> -		prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(current),
> -					 task_nice_ioprio(current));
> -	return prio;
> +	return __get_task_ioprio(current);

The build bot complained about this one, but I do not understand why.
Could it be because you do not have declared __get_task_ioprio() as "extern" ?

Also, to reduce refactoring changes in this patch, you could introduce
__get_task_ioprio() and make the above change in patch 2. No ?

>  }
>  
>  extern int set_task_ioprio(struct task_struct *task, int ioprio);
kernel test robot June 21, 2022, 12:11 a.m. UTC | #5 
Hi Jan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Jan-Kara/block-Fix-IO-priority-mess/20220621-001427
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: mips-randconfig-m031-20220619 (https://download.01.org/0day-ci/archive/20220621/202206210847.sBhjsEiQ-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 11.3.0

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

smatch warnings:
block/ioprio.c:184 get_task_raw_ioprio() warn: inconsistent indenting

vim +184 block/ioprio.c

   183	
 > 184		ret = security_task_getioprio(p);
   185		if (ret)
   186			goto out;
   187		task_lock(p);
   188		if (p->io_context)
   189			ret = p->io_context->ioprio;
   190		else
   191			ret = IOPRIO_DEFAULT;
   192		task_unlock(p);
   193	out:
   194		return ret;
   195	}
   196
Jan Kara June 21, 2022, 8:15 a.m. UTC | #6 
On Tue 21-06-22 08:57:29, Damien Le Moal wrote:
> On 6/21/22 01:11, Jan Kara wrote:
> > ioprio_get(2) can be asked to return the best IO priority from several
> > tasks (IOPRIO_WHO_PGRP, IOPRIO_WHO_USER). Currently the call treats
> > tasks without set IO priority as having priority
> > IOPRIO_CLASS_BE/IOPRIO_BE_NORM however this does not really reflect the
> > IO priority the task will get (which depends on task's nice value).
> > 
> > Fix the code to use the real IO priority task's IO will use. For this we
> > do some factoring out to share the code converting task's CPU priority
> > to IO priority and we also have to modify code for
> > ioprio_get(IOPRIO_WHO_PROCESS) to keep returning IOPRIO_CLASS_NONE
> > priority for tasks without set IO priority as a special case to maintain
> > userspace visible API.
> > 
> > Signed-off-by: Jan Kara <jack@suse.cz>

...

> > +/*
> > + * Return raw IO priority value as set by userspace. We use this for
> > + * ioprio_get(pid, IOPRIO_WHO_PROCESS) so that we keep historical behavior and
> > + * also so that userspace can distinguish unset IO priority (which just gets
> > + * overriden based on task's nice value) from IO priority set to some value.
> > + */
> > +static int get_task_raw_ioprio(struct task_struct *p) { int ret;
> 
> The "int ret;" declaration is on the wrong line, so is the curly bracket.

Huh, don't know how that got messed up. Anyway fixed. Thanks for noticing.

> > +
> > +	ret = security_task_getioprio(p);
> > +	if (ret)
> > +		goto out;
> >  	task_lock(p);
> >  	if (p->io_context)
> >  		ret = p->io_context->ioprio;
> > +	else
> > +		ret = IOPRIO_DEFAULT;
> >  	task_unlock(p);
> >  out:
> >  	return ret;
> > @@ -156,11 +196,6 @@ static int get_task_ioprio(struct task_struct *p)
> >  
> >  static int ioprio_best(unsigned short aprio, unsigned short bprio)
> >  {
> > -	if (!ioprio_valid(aprio))
> > -		aprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM);
> > -	if (!ioprio_valid(bprio))
> > -		bprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM);
> > -
> >  	return min(aprio, bprio);
> >  }
> 
> This function could be declared as inline now...

Yeah, but compilers inline (or not inline!) static functions as they see
fit anyway. So 'inline' keyword for static functions is generally a noise
which is why I just avoid it. But please let me know if you feel strongly
about that.

> >  static inline int get_current_ioprio(void)
> >  {
> > -	struct io_context *ioc = current->io_context;
> > -	int prio;
> > -
> > -	if (ioc)
> > -		prio = ioc->ioprio;
> > -	else
> > -		prio = IOPRIO_DEFAULT;
> > -
> > -	if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE)
> > -		prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(current),
> > -					 task_nice_ioprio(current));
> > -	return prio;
> > +	return __get_task_ioprio(current);
> 
> The build bot complained about this one, but I do not understand why.
> Could it be because you do not have declared __get_task_ioprio() as "extern" ?

No, likely it is because !CONFIG_BLOCK kernel does not have ioprio support
but something uses the get_current_ioprio() anyway. I'll fix it up.

> Also, to reduce refactoring changes in this patch, you could introduce
> __get_task_ioprio() and make the above change in patch 2. No ?

OK, I will move some refactoring.

								Honza
Damien Le Moal June 21, 2022, 8:31 a.m. UTC | #7 
On 6/21/22 17:15, Jan Kara wrote:
> On Tue 21-06-22 08:57:29, Damien Le Moal wrote:
>> On 6/21/22 01:11, Jan Kara wrote:
>>> ioprio_get(2) can be asked to return the best IO priority from several
>>> tasks (IOPRIO_WHO_PGRP, IOPRIO_WHO_USER). Currently the call treats
>>> tasks without set IO priority as having priority
>>> IOPRIO_CLASS_BE/IOPRIO_BE_NORM however this does not really reflect the
>>> IO priority the task will get (which depends on task's nice value).
>>>
>>> Fix the code to use the real IO priority task's IO will use. For this we
>>> do some factoring out to share the code converting task's CPU priority
>>> to IO priority and we also have to modify code for
>>> ioprio_get(IOPRIO_WHO_PROCESS) to keep returning IOPRIO_CLASS_NONE
>>> priority for tasks without set IO priority as a special case to maintain
>>> userspace visible API.
>>>
>>> Signed-off-by: Jan Kara <jack@suse.cz>
> 
> ...
> 
>>> +/*
>>> + * Return raw IO priority value as set by userspace. We use this for
>>> + * ioprio_get(pid, IOPRIO_WHO_PROCESS) so that we keep historical behavior and
>>> + * also so that userspace can distinguish unset IO priority (which just gets
>>> + * overriden based on task's nice value) from IO priority set to some value.
>>> + */
>>> +static int get_task_raw_ioprio(struct task_struct *p) { int ret;
>>
>> The "int ret;" declaration is on the wrong line, so is the curly bracket.
> 
> Huh, don't know how that got messed up. Anyway fixed. Thanks for noticing.
> 
>>> +
>>> +	ret = security_task_getioprio(p);
>>> +	if (ret)
>>> +		goto out;
>>>  	task_lock(p);
>>>  	if (p->io_context)
>>>  		ret = p->io_context->ioprio;
>>> +	else
>>> +		ret = IOPRIO_DEFAULT;
>>>  	task_unlock(p);
>>>  out:
>>>  	return ret;
>>> @@ -156,11 +196,6 @@ static int get_task_ioprio(struct task_struct *p)
>>>  
>>>  static int ioprio_best(unsigned short aprio, unsigned short bprio)
>>>  {
>>> -	if (!ioprio_valid(aprio))
>>> -		aprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM);
>>> -	if (!ioprio_valid(bprio))
>>> -		bprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM);
>>> -
>>>  	return min(aprio, bprio);
>>>  }
>>
>> This function could be declared as inline now...
> 
> Yeah, but compilers inline (or not inline!) static functions as they see
> fit anyway. So 'inline' keyword for static functions is generally a noise
> which is why I just avoid it. But please let me know if you feel strongly
> about that.

Not at all. Fine as-is !

> 
>>>  static inline int get_current_ioprio(void)
>>>  {
>>> -	struct io_context *ioc = current->io_context;
>>> -	int prio;
>>> -
>>> -	if (ioc)
>>> -		prio = ioc->ioprio;
>>> -	else
>>> -		prio = IOPRIO_DEFAULT;
>>> -
>>> -	if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE)
>>> -		prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(current),
>>> -					 task_nice_ioprio(current));
>>> -	return prio;
>>> +	return __get_task_ioprio(current);
>>
>> The build bot complained about this one, but I do not understand why.
>> Could it be because you do not have declared __get_task_ioprio() as "extern" ?
> 
> No, likely it is because !CONFIG_BLOCK kernel does not have ioprio support
> but something uses the get_current_ioprio() anyway. I'll fix it up.
> 
>> Also, to reduce refactoring changes in this patch, you could introduce
>> __get_task_ioprio() and make the above change in patch 2. No ?
> 
> OK, I will move some refactoring.
> 
> 								Honza
diff mbox series

Patch

diff --git a/block/ioprio.c b/block/ioprio.c
index b5cf7339709b..a4c19ce0de4c 100644
--- a/block/ioprio.c
+++ b/block/ioprio.c
@@ -138,6 +138,27 @@  SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio)
 	return ret;
 }
 
+/*
+ * If the task has set an I/O priority, use that. Otherwise, return
+ * the default I/O priority.
+ */
+int __get_task_ioprio(struct task_struct *p)
+{
+	struct io_context *ioc = p->io_context;
+	int prio;
+
+	if (ioc)
+		prio = ioc->ioprio;
+	else
+		prio = IOPRIO_DEFAULT;
+
+	if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE)
+		prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(p),
+					 task_nice_ioprio(p));
+	return prio;
+}
+EXPORT_SYMBOL_GPL(__get_task_ioprio);
+
 static int get_task_ioprio(struct task_struct *p)
 {
 	int ret;
@@ -145,10 +166,29 @@  static int get_task_ioprio(struct task_struct *p)
 	ret = security_task_getioprio(p);
 	if (ret)
 		goto out;
-	ret = IOPRIO_DEFAULT;
+	task_lock(p);
+	ret = __get_task_ioprio(p);
+	task_unlock(p);
+out:
+	return ret;
+}
+
+/*
+ * Return raw IO priority value as set by userspace. We use this for
+ * ioprio_get(pid, IOPRIO_WHO_PROCESS) so that we keep historical behavior and
+ * also so that userspace can distinguish unset IO priority (which just gets
+ * overriden based on task's nice value) from IO priority set to some value.
+ */
+static int get_task_raw_ioprio(struct task_struct *p) { int ret;
+
+	ret = security_task_getioprio(p);
+	if (ret)
+		goto out;
 	task_lock(p);
 	if (p->io_context)
 		ret = p->io_context->ioprio;
+	else
+		ret = IOPRIO_DEFAULT;
 	task_unlock(p);
 out:
 	return ret;
@@ -156,11 +196,6 @@  static int get_task_ioprio(struct task_struct *p)
 
 static int ioprio_best(unsigned short aprio, unsigned short bprio)
 {
-	if (!ioprio_valid(aprio))
-		aprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM);
-	if (!ioprio_valid(bprio))
-		bprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_BE_NORM);
-
 	return min(aprio, bprio);
 }
 
@@ -181,7 +216,7 @@  SYSCALL_DEFINE2(ioprio_get, int, which, int, who)
 			else
 				p = find_task_by_vpid(who);
 			if (p)
-				ret = get_task_ioprio(p);
+				ret = get_task_raw_ioprio(p);
 			break;
 		case IOPRIO_WHO_PGRP:
 			if (!who)
diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h
index 519d51fc8902..24e648dc4fb3 100644
--- a/include/linux/ioprio.h
+++ b/include/linux/ioprio.h
@@ -46,24 +46,11 @@  static inline int task_nice_ioclass(struct task_struct *task)
 		return IOPRIO_CLASS_BE;
 }
 
-/*
- * If the calling process has set an I/O priority, use that. Otherwise, return
- * the default I/O priority.
- */
+int __get_task_ioprio(struct task_struct *p);
+
 static inline int get_current_ioprio(void)
 {
-	struct io_context *ioc = current->io_context;
-	int prio;
-
-	if (ioc)
-		prio = ioc->ioprio;
-	else
-		prio = IOPRIO_DEFAULT;
-
-	if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE)
-		prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(current),
-					 task_nice_ioprio(current));
-	return prio;
+	return __get_task_ioprio(current);
 }
 
 extern int set_task_ioprio(struct task_struct *task, int ioprio);