On Thu, Mar 28, 2024 at 06:35:33AM +0100, Christoph Hellwig wrote: > On Wed, Mar 27, 2024 at 05:45:09PM +0100, Christian Brauner wrote: > > There's a bunch of flags that are purely based on what the file > > operations support while also never being conditionally set or unset. > > IOW, they're not subject to change for individual file opens. Imho, such > > flags don't need to live in f_mode they might as well live in the fops > > structs itself. > > Yes. I actually have a half-finished patch doing the same lying around, > which I've not found time to rabse. > > > (Fwiw, FMODE_NOACCOUNT and FMODE_BACKING could live in fops_flags as > > well because they're also completely static but they aren't really > > about file operations so they're better suited for FMODE_* imho.) > > I'd still move them there. I've also simply called fops_flags flags > so maybe it didn't bother me too much :) Possible that we can do that as well but I'd keep calling it fop_flags for the sake of grepping. If you git grep \\.fop_flags you get a nice unique match and you get an overview who uses what. I'm not married to this but I'll keep it for now. > > > +/* File ops support async buffered reads */ > > +#define FOP_BUF_RASYNC BIT(0) > > +/* File ops support async nowait buffered writes */ > > +#define FOP_BUF_WASYNC BIT(1) > > Can we spell out BUFFERED here when changing things? BUF always confuses > me as it let's me thing of the buffer cache. Ok. > > And can be please avoid this silly BIT() junk? 1u << N is shorter > and a lot more obvious than this random macro. Everyone and their grandmother has an opinion on this hex, <<, BIT(). :) Fine, I don't care enough and my grandmothers aren't around anymore. > > > +#define FOP_MMAP_SYNC BIT(2) > > Please throw in a comment for this one while you're at it. Ok. > > > +/* File ops support non-exclusive O_DIRECT writes from multiple threads */ > > +#define FOP_DIO_PARALLEL_WRITE BIT(3) > > + > > +#define __fops_supported(f_op, flag) ((f_op)->fops_flags & (flag)) > > +#define fops_buf_rasync(file) __fops_supported((file)->f_op, FOP_BUF_RASYNC) > > +#define fops_buf_wasync(file) __fops_supported((file)->f_op, FOP_BUF_WASYNC) > > +#define fops_mmap_sync(file) __fops_supported((file)->f_op, FOP_MMAP_SYNC) > > +#define fops_dio_parallel_write(file) __fops_supported((file)->f_op, FOP_DIO_PARALLEL_WRITE) > > And please drop these helpers. They just make grepping for the flags > a complete pain. Ok.
