On Mon, Apr 26, 2021 at 07:02:11PM +0100, Matthew Wilcox wrote:
> On Mon, Apr 26, 2021 at 01:52:17PM -0400, Vivek Goyal wrote:
> > On Mon, Apr 26, 2021 at 02:46:32PM +0100, Matthew Wilcox wrote:
> > > On Fri, Apr 23, 2021 at 09:07:21AM -0400, Vivek Goyal wrote:
> > > > +enum dax_wake_mode {
> > > > + WAKE_NEXT,
> > > > + WAKE_ALL,
> > > > +};
> > >
> > > Why define them in this order when ...
> > >
> > > > @@ -196,7 +207,7 @@ static void dax_wake_entry(struct xa_state *xas,
> > > > void *entry, bool wake_all)
> > > > * must be in the waitqueue and the following check will see
> > > > them.
> > > > */
> > > > if (waitqueue_active(wq))
> > > > - __wake_up(wq, TASK_NORMAL, wake_all ? 0 : 1, &key);
> > > > + __wake_up(wq, TASK_NORMAL, mode == WAKE_ALL ? 0 : 1,
> > > > &key);
> > >
> > > ... they're used like this? This is almost as bad as
> > >
> > > enum bool {
> > > true,
> > > false,
> > > };
> >
> > Hi Matthew,
> >
> > So you prefer that I should switch order of WAKE_NEXT and WAKE_ALL?
> >
> > enum dax_wake_mode {
> > WAKE_ALL,
> > WAKE_NEXT,
> > };
>
> That, yes.
>
> > And then do following to wake task.
> >
> > if (waitqueue_active(wq))
> > __wake_up(wq, TASK_NORMAL, mode, &key);
>
> No, the third argument to __wake_up() is a count, not an enum. It just so
> happens that '0' means 'all' and we only ever wake up 1 and not, say, 5.
> So the logical way to define the enum is ALL, NEXT which _just happens
> to match_ the usage of __wake_up().
Ok, In that case, I will retain existing code.
__wake_up(wq, TASK_NORMAL, mode == WAKE_ALL ? 0 : 1, &key);
Vivek
_______________________________________________
Linux-nvdimm mailing list -- [email protected]
To unsubscribe send an email to [email protected]
