Re: [PATCH v2] t10-pi: reduce ref tag code duplication

[Caleb Sander Mateos]

On Fri, Apr 17, 2026 at 12:56 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>
> On Thu, Apr 16, 2026 at 08:38:07AM -0700, Caleb Sander Mateos wrote:
> > On Wed, Apr 15, 2026 at 10:18 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> > >
> > > On Wed, Apr 15, 2026 at 03:08:47PM -0600, Caleb Sander Mateos wrote:
> > > >  #ifndef _LINUX_T10_PI_H
> > > >  #define _LINUX_T10_PI_H
> > > >
> > > >  #include <linux/types.h>
> > > >  #include <linux/blk-mq.h>
> > > > +#include <linux/wordpart.h>
> > >
> > > We must have already gotten this implicitly given that the code
> > > already uses lower_48_bits.
> >
> > lower_48_bits() is defined (and only used) in this header. Yes,
> > wordpart.h is already transitively included by the other headers, but
> > I think it's good practice for each file to explicitly include the
> > headers defining all the items it uses. It reduces the risk that
> > refactoring the other header files in the future will result in a
> > compilation error here by dropping the transitive include.
>
> In general pulling in a new header when no new user of it shows up is a
> bit weird.  I don't want to hold the patch up on this, but there are

I'm not sure what you mean by "no new user". There weren't previously
any uses of items from wordpart.h in t10-pi.h, but now it's using
lower_32_bits(). Personally, I think lower_32_bits() seems excessive
when we could just cast to u32, but I was following your suggestion...

> folks out there actually dropping not needed includes from headers as
> it can significantly reduce compile time.  Now this is not a heavily
> included header so it's unlikely to make a difference anyway.

Is a file being included by the preprocessor multiple times really
that expensive? I would have assumed it would be cached the first time
it was included.

Best,
Caleb