[GIT PULL] VFIO updates for v6.0-rc1 (part 2)

[GIT PULL] VFIO updates for v6.0-rc1 (part 2)

[Alex Williamson]

Hi Linus,

We've had a request to get this into the current merge window to ease
re-bases in the next development cycle.  Thanks,

Alex

The following changes since commit c8a684e2e110376c58f0bfa30fb3855d1e319670:

  Merge tag 'leds-5.20-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds (2022-08-08 11:36:21 -0700)

are available in the Git repository at:

  https://github.com/awilliam/linux-vfio.git tags/vfio-v6.0-rc1pt2

for you to fetch changes up to 0f3e72b5c8cfa0b57dc4fc7703a0a42dbc200ba9:

  vfio: Move vfio.c to vfio_main.c (2022-08-08 14:33:41 -0600)

----------------------------------------------------------------
VFIO updates for v6.0-rc1 (part 2)

 - Rename vfio source file to more easily allow additional source
   files in the upcoming development cycles (Jason Gunthorpe)

----------------------------------------------------------------
Jason Gunthorpe (1):
      vfio: Move vfio.c to vfio_main.c

 drivers/vfio/Makefile                | 2 ++
 drivers/vfio/{vfio.c => vfio_main.c} | 0
 2 files changed, 2 insertions(+)
 rename drivers/vfio/{vfio.c => vfio_main.c} (100%)

Re: [GIT PULL] VFIO updates for v6.0-rc1 (part 2)

[Linus Torvalds]

On Thu, Aug 11, 2022 at 2:36 PM Alex Williamson
<alex.williamson@redhat.com> wrote:
>
>  - Rename vfio source file to more easily allow additional source
>    files in the upcoming development cycles (Jason Gunthorpe)
>
> ----------------------------------------------------------------
> Jason Gunthorpe (1):
>       vfio: Move vfio.c to vfio_main.c
>
>  drivers/vfio/Makefile                | 2 ++
>  drivers/vfio/{vfio.c => vfio_main.c} | 0

Guys, why do you do this ludicrously redundant file naming?

The directory is called "vfio".

Why is every file in it called "vfio_xyzzy.c"?

This is a bad pattern, and I don't understand why you do this and
continue to just make it worse.

We don't have "drivers/block/block_floppy.c".

We don't have "kernel/kernel_exit.c".

And then when somebody finally points out that "vfio/vfio.c" is kind
of silly and bad naming practice because it doesn't say what the file
is all about, instead of realizing what the problem is, you just
continue the same broken pattern.

Is vfio the only subsystem that does this? No. We have the same odd
pattern in "drivers/leds/leds-xyzzy.c" and a few other subsystems, and
I don't understand it there either.

I don't care that much, because I never touch those files, but if I
did, I would have complained long ago about how auto-complete of
filenames is broken because of that silly non-unique and pointless
prefix that is just repeated mindlessly over and over again.

So I've pulled this, since hey, "maintainer preference" and me not
really having a lot of reason to *care*, but when I get this kind of
pure rename pull request, I just have to pipe up about how silly and
pointless the new name seems to be.

Am I the only one that just uses auto-complete for everything when I'm
off editing files in a terminal?

And if you don't use autocomplete, and actually type things out fully,
doesn't that double redundant 'vtio' bother you even *more*?

I'm just confused and wondering about this all, since it seems so *odd*.

It's like people have entirely missed what the point of using
directories to give you a hierarchy of things is all about..

               Linus

Re: [GIT PULL] VFIO updates for v6.0-rc1 (part 2)

[Jason Gunthorpe]

On Fri, Aug 12, 2022 at 09:35:34AM -0700, Linus Torvalds wrote:
> On Thu, Aug 11, 2022 at 2:36 PM Alex Williamson
> <alex.williamson@redhat.com> wrote:
> >
> >  - Rename vfio source file to more easily allow additional source
> >    files in the upcoming development cycles (Jason Gunthorpe)
> >
> > ----------------------------------------------------------------
> > Jason Gunthorpe (1):
> >       vfio: Move vfio.c to vfio_main.c
> >
> >  drivers/vfio/Makefile                | 2 ++
> >  drivers/vfio/{vfio.c => vfio_main.c} | 0
> 
> Guys, why do you do this ludicrously redundant file naming?
> 
> The directory is called "vfio".
> 
> Why is every file in it called "vfio_xyzzy.c"?

I think this is partly a historical artifact because each of the files
in the directory are compiling to actual kernel modules so the file
name has to have vfio_ in it to fit into the global module namespace.

Now, there is no reason I can see for all these files to be different
modules, but that is how it is, and because we have some module
parameters changing it is API breaking..

> So I've pulled this, since hey, "maintainer preference" and me not
> really having a lot of reason to *care*, but when I get this kind of
> pure rename pull request, I just have to pipe up about how silly and
> pointless the new name seems to be.

We can start to fix it up. I'm working on splitting that file up
further right now, so I will name the new ones container.c, group.c
and then we can resonably rename what is left as device.c - this would
logically match the object structure in the code at least.

Newer parts are already using non-prefix'd names:

$ ls drivers/vfio/pci/mlx5/
cmd.c  cmd.h  Kconfig  main.c  Makefile

> And if you don't use autocomplete, and actually type things out fully,
> doesn't that double redundant 'vtio' bother you even *more*?

Honestly, not really.. It is is just yet another tab in the bash
autocomplete, and vscode's filename searcher makes it irrelevant.

I'll watch out for it in future, eg I see I just merged
drivers/infiniband/hw/erdma/ and they used prefixes too.

Thanks,
Jason

Re: [GIT PULL] VFIO updates for v6.0-rc1 (part 2)

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 2379 bytes --]

On Fri, Aug 12, 2022 at 9:58 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> I think this is partly a historical artifact because each of the files
> in the directory are compiling to actual kernel modules so the file
> name has to have vfio_ in it to fit into the global module namespace.
>
> Now, there is no reason I can see for all these files to be different
> modules, but that is how it is, and because we have some module
> parameters changing it is API breaking..

Oh, the module name issue is absolutely real.

But we actually have good tools for that in the kernel build system,
because we've had that issue for so long, and because it's not at all
uncommon that one single kernel module needs to be built up from
multiple different object files.

I guess it does require an extra lines in the Makefile. Maybe we could
improve on that, but that extra line does end up having real
advantages in that it makes for a lot of flexibility (see below).

Attached is a truly *stupid* patch just to show the concept. I
literally just picked one vfio file at random to convert. The point
being that this approach

 (a) makes it very easy to have the file naming you like

 (b) makes it *very* easy to have common helper libraries that get
linked into the modules

 (c) also means that it's now basically trivial to split any of these
drivers into multiple files, exactly because the file name isn't tied
to the module name

where that extra line is exactly what makes (b) and (c) so trivial.

Now, don't get me wrong: this patch is *not* meant to be a "please do
this". If people really like the current odd file naming policy, it's
really not a lot of skin off my nose.

So the attached patch is literally meant to be a "look, if you want to
do this, it's really this simple".

And you can easily do it one driver at a time, possibly when  you have
a "oops, this one driver is getting a bit large, so I'd like to split
it up".

Also, it should go without mention that I've not actually *tested*
this patch. I did do a full build, and that full build results in a
vfio_iommu_type1.ko module as expected, but there might be something I
overlooked.

There's also some build overhead from the indirection, but hey, what
else is new. Our Makefiles are actually quite powerful, but they do
make 'GNU make' spend a *lot* of time doing various string tricks.

                    Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 567 bytes --]

diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 1a32357592e3..41ed528c7587 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -3,6 +3,8 @@ vfio_virqfd-y := virqfd.o
 
 vfio-y += vfio_main.o
 
+vfio_iommu_type1-y = iommu_type1.o
+
 obj-$(CONFIG_VFIO) += vfio.o
 obj-$(CONFIG_VFIO_VIRQFD) += vfio_virqfd.o
 obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/iommu_type1.c
similarity index 100%
rename from drivers/vfio/vfio_iommu_type1.c
rename to drivers/vfio/iommu_type1.c

Re: [GIT PULL] VFIO updates for v6.0-rc1 (part 2)

[pr-tracker-bot]

The pull request you sent on Thu, 11 Aug 2022 15:36:32 -0600:

> https://github.com/awilliam/linux-vfio.git tags/vfio-v6.0-rc1pt2

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/d16b418fac3de0d2ac854b3a9a1a59a0ebf2a0e9

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html