| Message ID | 1717146645-18829-1-git-send-email-zhiguo.niu@unisoc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [f2fs-dev] f2fs: use new ioprio Macro to get ckpt thread ioprio data | expand |
On 2024/5/31 17:10, Zhiguo Niu wrote: > Use new Macro IOPRIO_PRIO_LEVEL to get ckpt thread ioprio data(level), > it is more accurate and consisten with the way setting ckpt thread > ioprio(IOPRIO_PRIO_VALUE(class, data)). > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > --- > fs/f2fs/sysfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c > index 72676c5..55d46da 100644 > --- a/fs/f2fs/sysfs.c > +++ b/fs/f2fs/sysfs.c > @@ -340,7 +340,7 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a, > if (!strcmp(a->attr.name, "ckpt_thread_ioprio")) { > struct ckpt_req_control *cprc = &sbi->cprc_info; > int class = IOPRIO_PRIO_CLASS(cprc->ckpt_thread_ioprio); > - int data = IOPRIO_PRIO_DATA(cprc->ckpt_thread_ioprio); > + int data = IOPRIO_PRIO_LEVEL(cprc->ckpt_thread_ioprio); So, can you please use 'level' to instead 'data' in f2fs_sbi_show() and __sbi_store()? Thanks, > > if (class != IOPRIO_CLASS_RT && class != IOPRIO_CLASS_BE) > return -EINVAL;
On Mon, Jun 3, 2024 at 2:39 PM Chao Yu <chao@kernel.org> wrote: > > On 2024/5/31 17:10, Zhiguo Niu wrote: > > Use new Macro IOPRIO_PRIO_LEVEL to get ckpt thread ioprio data(level), > > it is more accurate and consisten with the way setting ckpt thread > > ioprio(IOPRIO_PRIO_VALUE(class, data)). > > > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > > --- > > fs/f2fs/sysfs.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c > > index 72676c5..55d46da 100644 > > --- a/fs/f2fs/sysfs.c > > +++ b/fs/f2fs/sysfs.c > > @@ -340,7 +340,7 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a, > > if (!strcmp(a->attr.name, "ckpt_thread_ioprio")) { > > struct ckpt_req_control *cprc = &sbi->cprc_info; > > int class = IOPRIO_PRIO_CLASS(cprc->ckpt_thread_ioprio); > > - int data = IOPRIO_PRIO_DATA(cprc->ckpt_thread_ioprio); > > + int data = IOPRIO_PRIO_LEVEL(cprc->ckpt_thread_ioprio); > > So, can you please use 'level' to instead 'data' in f2fs_sbi_show() and > __sbi_store()? Hi Chao, IOPRIO_PRIO_DATA in the new kernel version includes two parts: level and hint. In __sbi_store, " IOPRIO_PRIO_VALUE(class, data)" is used to set ckpt iopriority, and it will pass default hint value, we just need to keep class /level right. #define IOPRIO_PRIO_VALUE(prioclass, priolevel) \ ioprio_value(prioclass, priolevel, IOPRIO_HINT_NONE) so i think this part is not needed to modify. thanks! > > Thanks, > > > > > if (class != IOPRIO_CLASS_RT && class != IOPRIO_CLASS_BE) > > return -EINVAL; >
On 2024/6/3 14:52, Zhiguo Niu wrote: > On Mon, Jun 3, 2024 at 2:39 PM Chao Yu <chao@kernel.org> wrote: >> >> On 2024/5/31 17:10, Zhiguo Niu wrote: >>> Use new Macro IOPRIO_PRIO_LEVEL to get ckpt thread ioprio data(level), >>> it is more accurate and consisten with the way setting ckpt thread >>> ioprio(IOPRIO_PRIO_VALUE(class, data)). >>> >>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> >>> --- >>> fs/f2fs/sysfs.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c >>> index 72676c5..55d46da 100644 >>> --- a/fs/f2fs/sysfs.c >>> +++ b/fs/f2fs/sysfs.c >>> @@ -340,7 +340,7 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a, >>> if (!strcmp(a->attr.name, "ckpt_thread_ioprio")) { >>> struct ckpt_req_control *cprc = &sbi->cprc_info; >>> int class = IOPRIO_PRIO_CLASS(cprc->ckpt_thread_ioprio); >>> - int data = IOPRIO_PRIO_DATA(cprc->ckpt_thread_ioprio); >>> + int data = IOPRIO_PRIO_LEVEL(cprc->ckpt_thread_ioprio); >> >> So, can you please use 'level' to instead 'data' in f2fs_sbi_show() and >> __sbi_store()? > Hi Chao, > > IOPRIO_PRIO_DATA in the new kernel version includes two parts: level and hint. > In __sbi_store, " IOPRIO_PRIO_VALUE(class, data)" is used to set ckpt > iopriority, > and it will pass default hint value, we just need to keep class /level right. Zhiguo, I think f2fs only support configuring priolevel rather than priolevel and priohint of ckpt thread via ckpt_thread_ioprio sysfs interface, so it will be more readable to use 'level' instead of 'data' in context of the interface, thoughts? Thanks, > > #define IOPRIO_PRIO_VALUE(prioclass, priolevel) \ > ioprio_value(prioclass, priolevel, IOPRIO_HINT_NONE) > so i think this part is not needed to modify. > > thanks! >> >> Thanks, >> >>> >>> if (class != IOPRIO_CLASS_RT && class != IOPRIO_CLASS_BE) >>> return -EINVAL; >>
On Mon, Jun 3, 2024 at 3:50 PM Chao Yu <chao@kernel.org> wrote: > > On 2024/6/3 14:52, Zhiguo Niu wrote: > > On Mon, Jun 3, 2024 at 2:39 PM Chao Yu <chao@kernel.org> wrote: > >> > >> On 2024/5/31 17:10, Zhiguo Niu wrote: > >>> Use new Macro IOPRIO_PRIO_LEVEL to get ckpt thread ioprio data(level), > >>> it is more accurate and consisten with the way setting ckpt thread > >>> ioprio(IOPRIO_PRIO_VALUE(class, data)). > >>> > >>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > >>> --- > >>> fs/f2fs/sysfs.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c > >>> index 72676c5..55d46da 100644 > >>> --- a/fs/f2fs/sysfs.c > >>> +++ b/fs/f2fs/sysfs.c > >>> @@ -340,7 +340,7 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a, > >>> if (!strcmp(a->attr.name, "ckpt_thread_ioprio")) { > >>> struct ckpt_req_control *cprc = &sbi->cprc_info; > >>> int class = IOPRIO_PRIO_CLASS(cprc->ckpt_thread_ioprio); > >>> - int data = IOPRIO_PRIO_DATA(cprc->ckpt_thread_ioprio); > >>> + int data = IOPRIO_PRIO_LEVEL(cprc->ckpt_thread_ioprio); > >> > >> So, can you please use 'level' to instead 'data' in f2fs_sbi_show() and > >> __sbi_store()? > > Hi Chao, > > > > IOPRIO_PRIO_DATA in the new kernel version includes two parts: level and hint. > > In __sbi_store, " IOPRIO_PRIO_VALUE(class, data)" is used to set ckpt > > iopriority, > > and it will pass default hint value, we just need to keep class /level right. > > Zhiguo, > > I think f2fs only support configuring priolevel rather than priolevel and > priohint of ckpt thread via ckpt_thread_ioprio sysfs interface, so it will > be more readable to use 'level' instead of 'data' in context of the interface, > thoughts? Hi Chao, I understand what you said and I agree with this. I will update it in the next version. Thanks! > > Thanks, > > > > > #define IOPRIO_PRIO_VALUE(prioclass, priolevel) \ > > ioprio_value(prioclass, priolevel, IOPRIO_HINT_NONE) > > so i think this part is not needed to modify. > > > > thanks! > >> > >> Thanks, > >> > >>> > >>> if (class != IOPRIO_CLASS_RT && class != IOPRIO_CLASS_BE) > >>> return -EINVAL; > >>
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c index 72676c5..55d46da 100644 --- a/fs/f2fs/sysfs.c +++ b/fs/f2fs/sysfs.c @@ -340,7 +340,7 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a, if (!strcmp(a->attr.name, "ckpt_thread_ioprio")) { struct ckpt_req_control *cprc = &sbi->cprc_info; int class = IOPRIO_PRIO_CLASS(cprc->ckpt_thread_ioprio); - int data = IOPRIO_PRIO_DATA(cprc->ckpt_thread_ioprio); + int data = IOPRIO_PRIO_LEVEL(cprc->ckpt_thread_ioprio); if (class != IOPRIO_CLASS_RT && class != IOPRIO_CLASS_BE) return -EINVAL;
Use new Macro IOPRIO_PRIO_LEVEL to get ckpt thread ioprio data(level), it is more accurate and consisten with the way setting ckpt thread ioprio(IOPRIO_PRIO_VALUE(class, data)). Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> --- fs/f2fs/sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)