Bug 8798

Oops.

Anyway, the problem is:

# ls -alis /proc/acpi/video
total 0
4026532491 0 dr-xr-xr-x  4 root root 0 2007-07-23 23:20 .
4026531915 0 dr-xr-xr-x 12 root root 0 2007-07-23 23:13 ..
4026532513 0 dr-xr-xr-x  5 root root 0 2007-07-23 23:25 VID
4026532513 0 dr-xr-xr-x  5 root root 0 2007-07-23 23:25 VID

The VID/ directory is appearing in the directory listing more than once.

Created attachment 12109 [details]
dmesg output

Hmm,
First, the device name we export in /proc is from BIOS. We can't promise that the name is unique. So we should use something else rather than the string from ACPI namespace.
Second, proc_mkdir() doesn't inspect for duplicated names. Anyway, this is another bug and I'll send out a patch.

Created attachment 12131 [details]
file names must be different in the same directory.

Created attachment 12132 [details]
use device bus_id in procfs

This will change the /proc/acpi a lot.
But we need something else as the device name rather than the arbitrary stings from BIOS.
IMO, this is the most convient and reasonable way to fix it.
And I don't see there are any user space tools relying on the old device names.

Chris,
Does patch in comment #5 work for you?

Created attachment 12149 [details]
patch vs 2.6.23-rc1 from comment #5

applied to acpi-test

Yes, that patch has had the desired effect (I think):

# ls -alsi /proc/acpi/video/
total 0
4026532136 0 dr-xr-xr-x  4 root root 0 2007-07-26 22:32 .
4026531915 0 dr-xr-xr-x 12 root root 0 2007-07-26 22:10 ..
4026532137 0 dr-xr-xr-x  5 root root 0 2007-07-26 22:35 video:00
4026532158 0 dr-xr-xr-x  5 root root 0 2007-07-26 22:35 video:01

# cat /sys/bus/acpi/devices/video\:0?/device\:0?/path
\_SB_.PCI0.VID_.LCD0
\_SB_.PCI0.VID_.CRT0
\_SB_.PCI0.VID_.DVI0
\_SB_.PCI0.AGP_.VID_.LCD0
\_SB_.PCI0.AGP_.VID_.CRT0
\_SB_.PCI0.AGP_.VID_.DVI0

Why is there an AGP video device? I thought this laptop used PCI-Express.

Len,
please drop the patch in comment #5 from acpi-test.
It breaks the sbs driver.

Created attachment 12212 [details]
patch: use unique ACPI device names in procfs

re: patch in comment #4 to error-out in generic procfs code
on registration of files with duplicate names.
Maybe this is correct, but as we are abandoning procfs
and as it doesn't really fix the issue at hand where we presumably
need two different files, so lets not apply this one.

re: patch in comment #5 to change the names in all of /proc/acpi/*/*
Again, probably technically correct, and this one actually should
fix the problem at hand, but as we want /proc/acpi/
to become legacy, lets not stir the pot by changing the strings
on everybody.  It will probably just upset somebody
who will have to make a userspace change.  Instead, lets save
that pain for their migration to sysfs.  Also, per comment #9,
this patch causes sbs to fail because it was counting on the
fixed size of these strings:-(


Re: comment #8 why AGP_
Chris, these strings are completely arbitrary, it could just
as well be called ABCD -- it is up to the BIOS developer to
choose (or in this case, probably inherit) whatever strings
they want.  Part of the migration from procfs to sysfs is to
stop exposing (and confusing) user-space with these internal
AML variable names -- they make for a really poor user/kernel API.

re: patch in comment #10 to change all /proc/acpi/*/*
Again, too disruptive for a legacy interface.

re: what to do
The problem at hand is two /proc/acpi/video/VID files
We should make the late-comer /proc/acpi/video/VID:1 or something
to make this "instance" of it unique.  It would be ideal if the
solution applies to all of /proc/acpi/*/* -- but I'd settle
for a modification to the video.c file that solves just
this instance of perhaps a more general problem -- since we've
not seen the problem elsewhere, and we want to leave procfs
behind with as few changes to it as possible.

Created attachment 12285 [details]
patch-use-unique-device-name-in-procfs

Created attachment 12286 [details]
patch-video-driver-use-unique-device-name-in-procfs

Patches in comment#4 #12 are needed as well.

Len,
This can be a generic solution for all ACPI devices,
which need only few change in the ACPI driver code.

BTW: IMO, allowing duplicate name in procfs is a bug,
the patch in comment#4 fix this bug in procfs generic code,
and I think it should be sent to linux-fsdevel, do you think so?

yes, please send the patch in comment #4 directly upstream.

Re: patch in comment #12 -- the code to
generate the new name should be in a single macro or routine,
not duplicated in multiple places.  Also, it strikes me as a lot
of code to handle a corner-case in a feature we are trying to delete.

I think it would be better to ship patch in comment #4 
and fail to create the duplicate, and focus on getting
this driver properly supported in sysfs.  Either that,
or some really small hack that fixes the problem at hand
(say, only for the video driver), but is ifdef'd to be
deleted with the proc code.

Created attachment 12467 [details]
patch: hack the duplicate AML name "VID" problem on T61

patch in comment #15 shipped in 2.6.23-rc3-git9,
closed.