On Tue, May 28, 2024 at 09:12:02AM +0000, Pankaj Raghav (Samsung) wrote: > On Tue, May 28, 2024 at 09:00:00AM +1000, Dave Chinner wrote: > > On Mon, May 27, 2024 at 11:43:43PM +0100, Matthew Wilcox wrote: > > > On Mon, May 27, 2024 at 11:39:47PM +0100, Matthew Wilcox wrote: > > > > > > + AS_FOLIO_ORDER_MIN = 16, > > > > > > + AS_FOLIO_ORDER_MAX = 21, /* Bits 16-25 are used for FOLIO_ORDER */ > > > > > > }; > > > > > > > > > > > > +#define AS_FOLIO_ORDER_MIN_MASK 0x001f0000 > > > > > > +#define AS_FOLIO_ORDER_MAX_MASK 0x03e00000 > > > > > > > > > > As you changed the mapping flag offset, these masks also needs to be > > > > > changed accordingly. > > > > > > > > That's why I did change them? > > > > > > How about: > > > > > > -#define AS_FOLIO_ORDER_MIN_MASK 0x001f0000 > > > -#define AS_FOLIO_ORDER_MAX_MASK 0x03e00000 > > > +#define AS_FOLIO_ORDER_MIN_MASK (31 << AS_FOLIO_ORDER_MIN) > > > +#define AS_FOLIO_ORDER_MAX_MASK (31 << AS_FOLIO_ORDER_MAX) > > > > Lots of magic numbers based on the order having only having 5 bits > > of resolution. Removing that magic looks like this: > > > > AS_FOLIO_ORDER_BITS = 5, > > I think this needs to be defined outside of the enum as 5 is already > taken by AS_NO_WRITEBACK_TAGS? But I like the idea of making it generic > like this. Duplicate values in assigned enums are legal and fairly common. This: enum { FOO = 1, BAR = 2, BAZ = 1, }; int main(int argc, char *argv[]) { printf("foo %d, bar %d, baz %d\n", FOO, BAR, BAZ); } compiles without warnings or errors and gives the output: foo 1, bar 2, baz 1 -Dave. -- Dave Chinner david@xxxxxxxxxxxxx