| Message ID | 20231130-vfs-files-fixes-v1-3-e73ca6f4ea83@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | file: minor fixes | expand |
On Thu 30-11-23 13:49:09, Christian Brauner wrote: > The naming is actively misleading since we switched to > SLAB_TYPESAFE_BY_RCU. rcu_head is #define callback_head. Use > callback_head directly and rename f_rcuhead to f_tw. > > Signed-off-by: Christian Brauner <brauner@kernel.org> Nice. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/file_table.c | 6 +++--- > include/linux/fs.h | 2 +- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/file_table.c b/fs/file_table.c > index 6deac386486d..78614204ef2c 100644 > --- a/fs/file_table.c > +++ b/fs/file_table.c > @@ -407,7 +407,7 @@ static void delayed_fput(struct work_struct *unused) > > static void ____fput(struct callback_head *work) > { > - __fput(container_of(work, struct file, f_rcuhead)); > + __fput(container_of(work, struct file, f_tw)); > } > > /* > @@ -438,8 +438,8 @@ void fput(struct file *file) > return; > } > if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) { > - init_task_work(&file->f_rcuhead, ____fput); > - if (!task_work_add(task, &file->f_rcuhead, TWA_RESUME)) > + init_task_work(&file->f_tw, ____fput); > + if (!task_work_add(task, &file->f_tw, TWA_RESUME)) > return; > /* > * After this task has run exit_task_work(), > diff --git a/include/linux/fs.h b/include/linux/fs.h > index f171505940ff..d23a886df8fa 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -992,7 +992,7 @@ static inline int ra_has_index(struct file_ra_state *ra, pgoff_t index) > struct file { > union { > struct llist_node f_llist; > - struct rcu_head f_rcuhead; > + struct callback_head f_tw; > unsigned int f_iocb_flags; > }; > > > -- > 2.42.0 >
On Thu, 30 Nov 2023 at 21:49, Christian Brauner <brauner@kernel.org> wrote: > > The naming is actively misleading since we switched to > SLAB_TYPESAFE_BY_RCU. rcu_head is #define callback_head. Use > callback_head directly and rename f_rcuhead to f_tw. Please just write it out, ie "f_taskwork" or something. Using random letter combinations is very illegible. It may make sense at the time (when you are literally editing a line about "init_task_work()") but it's literally just line noise in that structure definition that makes me go "f_tw is wtf backwards, wtf?". It might even be a good idea to say *what* it is a task work for. Linus
On Fri, Dec 01, 2023 at 08:37:30AM +0900, Linus Torvalds wrote: > On Thu, 30 Nov 2023 at 21:49, Christian Brauner <brauner@kernel.org> wrote: > > > > The naming is actively misleading since we switched to > > SLAB_TYPESAFE_BY_RCU. rcu_head is #define callback_head. Use > > callback_head directly and rename f_rcuhead to f_tw. > > Please just write it out, ie "f_taskwork" or something. Using random > letter combinations is very illegible. It may make sense at the time > (when you are literally editing a line about "init_task_work()") but > it's literally just line noise in that structure definition that makes > me go "f_tw is wtf backwards, wtf?". Oh, I didn't even think about this. > It might even be a good idea to say *what* it is a task work for. Ok, I've added a comment about for both f_llist and f_task_work.
diff --git a/fs/file_table.c b/fs/file_table.c index 6deac386486d..78614204ef2c 100644 --- a/fs/file_table.c +++ b/fs/file_table.c @@ -407,7 +407,7 @@ static void delayed_fput(struct work_struct *unused) static void ____fput(struct callback_head *work) { - __fput(container_of(work, struct file, f_rcuhead)); + __fput(container_of(work, struct file, f_tw)); } /* @@ -438,8 +438,8 @@ void fput(struct file *file) return; } if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) { - init_task_work(&file->f_rcuhead, ____fput); - if (!task_work_add(task, &file->f_rcuhead, TWA_RESUME)) + init_task_work(&file->f_tw, ____fput); + if (!task_work_add(task, &file->f_tw, TWA_RESUME)) return; /* * After this task has run exit_task_work(), diff --git a/include/linux/fs.h b/include/linux/fs.h index f171505940ff..d23a886df8fa 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -992,7 +992,7 @@ static inline int ra_has_index(struct file_ra_state *ra, pgoff_t index) struct file { union { struct llist_node f_llist; - struct rcu_head f_rcuhead; + struct callback_head f_tw; unsigned int f_iocb_flags; };
The naming is actively misleading since we switched to SLAB_TYPESAFE_BY_RCU. rcu_head is #define callback_head. Use callback_head directly and rename f_rcuhead to f_tw. Signed-off-by: Christian Brauner <brauner@kernel.org> --- fs/file_table.c | 6 +++--- include/linux/fs.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)