Messages in this thread Patch in this message | | | Date | Fri, 7 Apr 2017 10:04:33 +0200 | From | Michal Hocko <> | Subject | Re: [PATCH] Rename tsk_restore_flags() to current_restore_flags() |
| |
On Fri 07-04-17 10:03:26, NeilBrown wrote: > > It is not safe for one thread to modify the ->flags > of another thread as there is no locking that can protect > the update. > So tsk_restore_flags(), which takes a task pointer and modifies > the flags, is an invitation to do the wrong thing. > > All current users pass "current" as the task, so no developers have > accepted that invitation. It would be best to ensure it remains > that way. > > So rename tsk_restore_flags() to current_restore_flags() and don't > pass in a task_struct pointer. Always operate on current->flags.
yes this makes a lot of sense to me.
It would be great if somebody cleaned up setting of ->flags to be always set on current as well. This seems to be the case but there are places where we do struct task_struct *tsk = current; [...] tsk->flags |= PF_$FOO
> Signed-off-by: NeilBrown <neilb@suse.com>
Acked-by: Michal Hocko <mhocko@suse.com>
> --- > > get_maintainer.pl suggested 35 recipients for this patch. > I decided to trim that a bit .... > > Can it just go through the tip tree?? It should be completely > uncontroversial. > > Thanks, > NeilBrown > > drivers/block/nbd.c | 2 +- > drivers/scsi/iscsi_tcp.c | 2 +- > fs/nfsd/vfs.c | 2 +- > include/linux/sched.h | 6 +++--- > kernel/softirq.c | 2 +- > net/core/dev.c | 2 +- > net/core/sock.c | 2 +- > 7 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 7e4287bc19e5..04ec921727b7 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -239,7 +239,7 @@ static int sock_xmit(struct nbd_device *nbd, int index, int send, > } > } while (msg_data_left(&msg)); > > - tsk_restore_flags(current, pflags, PF_MEMALLOC); > + current_restore_flags(pflags, PF_MEMALLOC); > > return result; > } > diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c > index 4228aba1f654..bbea8eac9abb 100644 > --- a/drivers/scsi/iscsi_tcp.c > +++ b/drivers/scsi/iscsi_tcp.c > @@ -387,7 +387,7 @@ static int iscsi_sw_tcp_pdu_xmit(struct iscsi_task *task) > rc = 0; > } > > - tsk_restore_flags(current, pflags, PF_MEMALLOC); > + current_restore_flags(pflags, PF_MEMALLOC); > return rc; > } > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > index 19d50f600e8d..9aaf6ca77569 100644 > --- a/fs/nfsd/vfs.c > +++ b/fs/nfsd/vfs.c > @@ -1004,7 +1004,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file, > else > err = nfserrno(host_err); > if (test_bit(RQ_LOCAL, &rqstp->rq_flags)) > - tsk_restore_flags(current, pflags, PF_LESS_THROTTLE); > + current_restore_flags(pflags, PF_LESS_THROTTLE); > return err; > } > > diff --git a/include/linux/sched.h b/include/linux/sched.h > index d67eee84fd43..0978fb74e45a 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1286,10 +1286,10 @@ TASK_PFA_TEST(LMK_WAITING, lmk_waiting) > TASK_PFA_SET(LMK_WAITING, lmk_waiting) > > static inline void > -tsk_restore_flags(struct task_struct *task, unsigned long orig_flags, unsigned long flags) > +current_restore_flags(unsigned long orig_flags, unsigned long flags) > { > - task->flags &= ~flags; > - task->flags |= orig_flags & flags; > + current->flags &= ~flags; > + current->flags |= orig_flags & flags; > } > > extern int cpuset_cpumask_can_shrink(const struct cpumask *cur, const struct cpumask *trial); > diff --git a/kernel/softirq.c b/kernel/softirq.c > index 744fa611cae0..4e09821f9d9e 100644 > --- a/kernel/softirq.c > +++ b/kernel/softirq.c > @@ -309,7 +309,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void) > account_irq_exit_time(current); > __local_bh_enable(SOFTIRQ_OFFSET); > WARN_ON_ONCE(in_interrupt()); > - tsk_restore_flags(current, old_flags, PF_MEMALLOC); > + current_restore_flags(old_flags, PF_MEMALLOC); > } > > asmlinkage __visible void do_softirq(void) > diff --git a/net/core/dev.c b/net/core/dev.c > index 7869ae3837ca..e8a366387a99 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -4240,7 +4240,7 @@ static int __netif_receive_skb(struct sk_buff *skb) > */ > current->flags |= PF_MEMALLOC; > ret = __netif_receive_skb_core(skb, true); > - tsk_restore_flags(current, pflags, PF_MEMALLOC); > + current_restore_flags(pflags, PF_MEMALLOC); > } else > ret = __netif_receive_skb_core(skb, false); > > diff --git a/net/core/sock.c b/net/core/sock.c > index a96d5f7a5734..20fb01b01ddb 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -325,7 +325,7 @@ int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb) > > current->flags |= PF_MEMALLOC; > ret = sk->sk_backlog_rcv(sk, skb); > - tsk_restore_flags(current, pflags, PF_MEMALLOC); > + current_restore_flags(pflags, PF_MEMALLOC); > > return ret; > } > -- > 2.12.2 >
-- Michal Hocko SUSE Labs
| |