| Message ID | 20240621062617.595007-2-yi.sun@unisoc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | add io priority feature to work and use it in f2fs fsverity work | expand |
Hello On Fri, Jun 21, 2024 at 2:27 PM Yi Sun <yi.sun@unisoc.com> wrote: > index 4c38824f3ab4..d9969596bbc3 100644 > --- a/include/linux/workqueue_types.h > +++ b/include/linux/workqueue_types.h > @@ -17,6 +17,12 @@ struct work_struct { > atomic_long_t data; > struct list_head entry; > work_func_t func; > + /* If the work does submit_bio, io priority may be needed. */ > + unsigned short ioprio; > + /* Record kworker's original io priority. */ > + unsigned short ori_ioprio; > + /* Whether the work has set io priority? */ > + long ioprio_flag; I don't see any ioprio code being integrated into workqueue in your patchset, from which what you need might be: struct ioprio_work { /******* the work item to be scheduled *******/ struct work_struct work; /******* the stuff need for ioprio ******/ /* If the work does submit_bio, io priority may be needed. */ unsigned short ioprio; /* Record kworker's original io priority. */ unsigned short ori_ioprio; /* Whether the work has set io priority? */ long ioprio_flag; } And if ioprio needs to be integrated into workqueue, it should be attributes added to the workqueue itself as in the struct workqueue_attrs. Thanks Lai
On Fri, Jun 21, 2024 at 02:26:16PM +0800, Yi Sun wrote: > Many works will go to submit_bio(), and in many cases the io priority of > kworker cannot meet the real-time requirements of this work. > > So add the basic attribute ioprio to work_struct, and kworker can adjust > its io priority according to this attribute. > > Add function set_work_ioprio() to set the io priority of this work. > Add function may_adjust_work_task_ioprio() to adjust kworker's io priority. > Add function restore_work_task_ioprio() to restore kworker's io priority. work_struct!? there's a lot of task_struct properties we would want work_struct to inherit if we went this route, but it's just not feasible, work_struct should be small and thin. You're always embedding work_struct into your own struct, I would suggest tracking this yourself - or coming up with a new heavier standard struct that embeds a work_struct and has io path options, there's more than just priority. > > Signed-off-by: Yi Sun <yi.sun@unisoc.com> > --- > include/linux/workqueue.h | 9 ++++++ > include/linux/workqueue_types.h | 6 ++++ > kernel/workqueue.c | 51 +++++++++++++++++++++++++++++++++ > 3 files changed, 66 insertions(+) > > diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h > index fb3993894536..f6191774b730 100644 > --- a/include/linux/workqueue.h > +++ b/include/linux/workqueue.h > @@ -286,6 +286,9 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; } > lockdep_init_map(&(_work)->lockdep_map, "(work_completion)"#_work, (_key), 0); \ > INIT_LIST_HEAD(&(_work)->entry); \ > (_work)->func = (_func); \ > + (_work)->ioprio = 0; \ > + (_work)->ori_ioprio = 0; \ > + (_work)->ioprio_flag = 0; \ > } while (0) > #else > #define __INIT_WORK_KEY(_work, _func, _onstack, _key) \ > @@ -294,6 +297,9 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; } > (_work)->data = (atomic_long_t) WORK_DATA_INIT(); \ > INIT_LIST_HEAD(&(_work)->entry); \ > (_work)->func = (_func); \ > + (_work)->ioprio = 0; \ > + (_work)->ori_ioprio = 0; \ > + (_work)->ioprio_flag = 0; \ > } while (0) > #endif > > @@ -585,6 +591,9 @@ extern struct work_struct *current_work(void); > extern bool current_is_workqueue_rescuer(void); > extern bool workqueue_congested(int cpu, struct workqueue_struct *wq); > extern unsigned int work_busy(struct work_struct *work); > +extern void set_work_ioprio(struct work_struct *work, unsigned short ioprio); > +extern void may_adjust_work_task_ioprio(struct work_struct *work); > +extern void restore_work_task_ioprio(struct work_struct *work); > extern __printf(1, 2) void set_worker_desc(const char *fmt, ...); > extern void print_worker_info(const char *log_lvl, struct task_struct *task); > extern void show_all_workqueues(void); > diff --git a/include/linux/workqueue_types.h b/include/linux/workqueue_types.h > index 4c38824f3ab4..d9969596bbc3 100644 > --- a/include/linux/workqueue_types.h > +++ b/include/linux/workqueue_types.h > @@ -17,6 +17,12 @@ struct work_struct { > atomic_long_t data; > struct list_head entry; > work_func_t func; > + /* If the work does submit_bio, io priority may be needed. */ > + unsigned short ioprio; > + /* Record kworker's original io priority. */ > + unsigned short ori_ioprio; > + /* Whether the work has set io priority? */ > + long ioprio_flag; > #ifdef CONFIG_LOCKDEP > struct lockdep_map lockdep_map; > #endif > diff --git a/kernel/workqueue.c b/kernel/workqueue.c > index 003474c9a77d..a44a8f92eec2 100644 > --- a/kernel/workqueue.c > +++ b/kernel/workqueue.c > @@ -55,6 +55,7 @@ > #include <linux/kvm_para.h> > #include <linux/delay.h> > #include <linux/irq_work.h> > +#include <linux/ioprio.h> > > #include "workqueue_internal.h" > > @@ -6025,6 +6026,56 @@ unsigned int work_busy(struct work_struct *work) > } > EXPORT_SYMBOL_GPL(work_busy); > > +/** > + * set_work_ioprio - set io priority for the current work > + * @work: the work to be set > + * @ioprio: desired io priority > + * > + * This function can be called after INIT_WORK if the io priority > + * of the work needs to adjust. And it is recommended to use this > + * function together with may_adjust_work_task_ioprio() and > + * restore_work_task_ioprio(). > + */ > +void set_work_ioprio(struct work_struct *work, unsigned short ioprio) > +{ > + work->ioprio = ioprio; > + work->ioprio_flag = 1; > +} > +EXPORT_SYMBOL_GPL(set_work_ioprio); > + > +/** > + * may_adjust_work_task_ioprio - adjust the io priority of kworker > + * @work: the work that kworker will do > + * > + * It is recommended to use this function together with set_work_ioprio() > + * and restore_work_task_ioprio(). > + */ > +void may_adjust_work_task_ioprio(struct work_struct *work) > +{ > + if (work->ioprio_flag) { > + work->ori_ioprio = get_current_ioprio(); > + set_task_ioprio(current, work->ioprio); > + } > +} > +EXPORT_SYMBOL_GPL(may_adjust_work_task_ioprio); > + > +/** > + * restore_work_task_ioprio - restore the io priority of kworker > + * @work: the work that kworker just did > + * > + * When kworker finishes the work, the original io priority of > + * kworker should be restored. It is recommended to use this function > + * together with set_work_ioprio() and may_adjust_work_task_ioprio(). > + */ > +void restore_work_task_ioprio(struct work_struct *work) > +{ > + if (work->ioprio_flag) { > + set_task_ioprio(current, work->ori_ioprio); > + work->ioprio_flag = 0; > + } > +} > +EXPORT_SYMBOL_GPL(restore_work_task_ioprio); > + > /** > * set_worker_desc - set description for the current work item > * @fmt: printf-style format string > -- > 2.25.1 >
Hi Yi,
kernel test robot noticed the following build errors:
[auto build test ERROR on tj-wq/for-next]
[also build test ERROR on jaegeuk-f2fs/dev-test jaegeuk-f2fs/dev linus/master v6.10-rc5 next-20240625]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Yi-Sun/workqueue-add-io-priority-to-work_struct/20240625-165935
base: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-next
patch link: https://lore.kernel.org/r/20240621062617.595007-2-yi.sun%40unisoc.com
patch subject: [PATCH 1/2] workqueue: add io priority to work_struct
config: arm-randconfig-003-20240626 (https://download.01.org/0day-ci/archive/20240626/202406261201.0hc4qgvm-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240626/202406261201.0hc4qgvm-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406261201.0hc4qgvm-lkp@intel.com/
All errors (new ones prefixed by >>):
arm-linux-gnueabi-ld: kernel/workqueue.o: in function `restore_work_task_ioprio':
>> workqueue.c:(.text+0xd2e): undefined reference to `set_task_ioprio'
arm-linux-gnueabi-ld: kernel/workqueue.o: in function `may_adjust_work_task_ioprio':
workqueue.c:(.text+0xd18): undefined reference to `set_task_ioprio'
Hi Yi,
kernel test robot noticed the following build errors:
[auto build test ERROR on tj-wq/for-next]
[also build test ERROR on jaegeuk-f2fs/dev-test jaegeuk-f2fs/dev linus/master v6.10-rc5 next-20240625]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Yi-Sun/workqueue-add-io-priority-to-work_struct/20240625-165935
base: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-next
patch link: https://lore.kernel.org/r/20240621062617.595007-2-yi.sun%40unisoc.com
patch subject: [PATCH 1/2] workqueue: add io priority to work_struct
config: csky-randconfig-001-20240626 (https://download.01.org/0day-ci/archive/20240626/202406261227.ngU3vsdJ-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240626/202406261227.ngU3vsdJ-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406261227.ngU3vsdJ-lkp@intel.com/
All errors (new ones prefixed by >>):
csky-linux-ld: kernel/workqueue.o: in function `may_adjust_work_task_ioprio':
workqueue.c:(.text+0x460): undefined reference to `set_task_ioprio'
csky-linux-ld: kernel/workqueue.o: in function `restore_work_task_ioprio':
workqueue.c:(.text+0x47c): undefined reference to `set_task_ioprio'
>> csky-linux-ld: workqueue.c:(.text+0x4bc): undefined reference to `set_task_ioprio'
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index fb3993894536..f6191774b730 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -286,6 +286,9 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; } lockdep_init_map(&(_work)->lockdep_map, "(work_completion)"#_work, (_key), 0); \ INIT_LIST_HEAD(&(_work)->entry); \ (_work)->func = (_func); \ + (_work)->ioprio = 0; \ + (_work)->ori_ioprio = 0; \ + (_work)->ioprio_flag = 0; \ } while (0) #else #define __INIT_WORK_KEY(_work, _func, _onstack, _key) \ @@ -294,6 +297,9 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; } (_work)->data = (atomic_long_t) WORK_DATA_INIT(); \ INIT_LIST_HEAD(&(_work)->entry); \ (_work)->func = (_func); \ + (_work)->ioprio = 0; \ + (_work)->ori_ioprio = 0; \ + (_work)->ioprio_flag = 0; \ } while (0) #endif @@ -585,6 +591,9 @@ extern struct work_struct *current_work(void); extern bool current_is_workqueue_rescuer(void); extern bool workqueue_congested(int cpu, struct workqueue_struct *wq); extern unsigned int work_busy(struct work_struct *work); +extern void set_work_ioprio(struct work_struct *work, unsigned short ioprio); +extern void may_adjust_work_task_ioprio(struct work_struct *work); +extern void restore_work_task_ioprio(struct work_struct *work); extern __printf(1, 2) void set_worker_desc(const char *fmt, ...); extern void print_worker_info(const char *log_lvl, struct task_struct *task); extern void show_all_workqueues(void); diff --git a/include/linux/workqueue_types.h b/include/linux/workqueue_types.h index 4c38824f3ab4..d9969596bbc3 100644 --- a/include/linux/workqueue_types.h +++ b/include/linux/workqueue_types.h @@ -17,6 +17,12 @@ struct work_struct { atomic_long_t data; struct list_head entry; work_func_t func; + /* If the work does submit_bio, io priority may be needed. */ + unsigned short ioprio; + /* Record kworker's original io priority. */ + unsigned short ori_ioprio; + /* Whether the work has set io priority? */ + long ioprio_flag; #ifdef CONFIG_LOCKDEP struct lockdep_map lockdep_map; #endif diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 003474c9a77d..a44a8f92eec2 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -55,6 +55,7 @@ #include <linux/kvm_para.h> #include <linux/delay.h> #include <linux/irq_work.h> +#include <linux/ioprio.h> #include "workqueue_internal.h" @@ -6025,6 +6026,56 @@ unsigned int work_busy(struct work_struct *work) } EXPORT_SYMBOL_GPL(work_busy); +/** + * set_work_ioprio - set io priority for the current work + * @work: the work to be set + * @ioprio: desired io priority + * + * This function can be called after INIT_WORK if the io priority + * of the work needs to adjust. And it is recommended to use this + * function together with may_adjust_work_task_ioprio() and + * restore_work_task_ioprio(). + */ +void set_work_ioprio(struct work_struct *work, unsigned short ioprio) +{ + work->ioprio = ioprio; + work->ioprio_flag = 1; +} +EXPORT_SYMBOL_GPL(set_work_ioprio); + +/** + * may_adjust_work_task_ioprio - adjust the io priority of kworker + * @work: the work that kworker will do + * + * It is recommended to use this function together with set_work_ioprio() + * and restore_work_task_ioprio(). + */ +void may_adjust_work_task_ioprio(struct work_struct *work) +{ + if (work->ioprio_flag) { + work->ori_ioprio = get_current_ioprio(); + set_task_ioprio(current, work->ioprio); + } +} +EXPORT_SYMBOL_GPL(may_adjust_work_task_ioprio); + +/** + * restore_work_task_ioprio - restore the io priority of kworker + * @work: the work that kworker just did + * + * When kworker finishes the work, the original io priority of + * kworker should be restored. It is recommended to use this function + * together with set_work_ioprio() and may_adjust_work_task_ioprio(). + */ +void restore_work_task_ioprio(struct work_struct *work) +{ + if (work->ioprio_flag) { + set_task_ioprio(current, work->ori_ioprio); + work->ioprio_flag = 0; + } +} +EXPORT_SYMBOL_GPL(restore_work_task_ioprio); + /** * set_worker_desc - set description for the current work item * @fmt: printf-style format string
Many works will go to submit_bio(), and in many cases the io priority of kworker cannot meet the real-time requirements of this work. So add the basic attribute ioprio to work_struct, and kworker can adjust its io priority according to this attribute. Add function set_work_ioprio() to set the io priority of this work. Add function may_adjust_work_task_ioprio() to adjust kworker's io priority. Add function restore_work_task_ioprio() to restore kworker's io priority. Signed-off-by: Yi Sun <yi.sun@unisoc.com> --- include/linux/workqueue.h | 9 ++++++ include/linux/workqueue_types.h | 6 ++++ kernel/workqueue.c | 51 +++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+)