On Sat, Aug 14, 2021 at 10:37:23AM +0800, Muchun Song wrote:
> On Sat, Aug 14, 2021 at 12:58 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> >
> > On Sat, Aug 14, 2021 at 12:40:53AM +0800, yanghui wrote:
> > > +++ b/mm/mempolicy.c
> > > @@ -193,7 +193,7 @@ static int mpol_new_interleave(struct mempolicy *pol, const nodemask_t *nodes)
> > > {
> > > if (nodes_empty(*nodes))
> > > return -EINVAL;
> > > - pol->nodes = *nodes;
> > > + WRITE_ONCE(pol->nodes, *nodes);
> >
> > typedef struct { DECLARE_BITMAP(bits, MAX_NUMNODES); } nodemask_t;
> >
> > If MAX_NUMNODES is large enough, is WRITE_ONCE going to work? It could
> > be 128 bits, and few architectures have an atomic 128-bit store
> > instruction.
> >
>
> Hi Matthew,
>
> In my memory, the WRITE_ONCE will become memcpy if
> the size is greater than sizeof(long). But I found that it didn't
> support the size over sizeof(long) nowadays. So you are
> right. It doesn't work when MAX_NUMNODES is large enough.
>
> If we don't use WRITE_ONCE/READ_ONCE here, it seems
> like no problem. At least we can sure offset_il_node returns
> a valid node id. What is your opinion about this fix?
Yes, I think that's a reasonable fix. Include a comment about how
copying pol->nodes to the stack might race with a change to pol->nodes,
but that doesn't matter as long as we don't look at pol->nodes later?
