libsysfs/udev hang

Hi,

while writing an ataraid replacement on top of device mapper, I stumbled upon a freeze in sysfs_close_device which only happens if udev is NOT compiled with DEBUG=true.

Test routine (modified udevinfo function) follows:
static int print_device_chain(const char *path)

{
        struct sysfs_class_device *class_dev;
        struct sysfs_class_device *class_dev_parent;
        struct sysfs_attribute *attr;
        struct sysfs_device *sysfs_dev;
        struct sysfs_device *sysfs_dev_parent;
        struct dlist *cur_attrs;
        int retval = 0;
        unsigned int vendor,device,subsystem_vendor,subsystem_device;
        unsigned long devsize;
        unsigned char *buf;

        /*  get the class dev */
        class_dev = sysfs_open_class_device_path(path);
        if (class_dev = NULL) {
                printf("couldn't get the class device\n");
                return -1;
        }

        cur_attrs = sysfs_get_classdev_attributes(class_dev);

        /* get device size in 512 byte blocks from 'size' */
        buf = sysfs_get_value_from_attributes(cur_attrs, "size");
        devsize = strtoul(buf, NULL, 10);

        /* read the 'dev' file for major/minor*/
        buf = sysfs_get_value_from_attributes(cur_attrs, "dev");
        printf("device '%s' has size %lu blocks and major:minor %s\n",

class_dev->path, devsize, buf);

        sysfs_close_list(cur_attrs);

        /* get the device link (if parent exists look here) */
        class_dev_parent = sysfs_get_classdev_parent(class_dev);
        if (class_dev_parent != NULL) {
                class_dev = class_dev_parent;
        }
        sysfs_dev = sysfs_get_classdev_device(class_dev);
        if (sysfs_dev != NULL)
                printf("follow the class device's \"device\"\n");

        /* look the device chain upwards */
        while (sysfs_dev != NULL) {

                printf("point1\n");
                /* open sysfs device directory and print all attributes */
                if (strcmp(sysfs_dev->bus, "pci") = 0) {
                        cur_attrs = sysfs_get_device_attributes(sysfs_dev);
                        buf = sysfs_get_value_from_attributes(cur_attrs,

"vendor");
vendor = (unsigned int)strtoul(buf + 2, NULL, 16); buf = sysfs_get_value_from_attributes(cur_attrs,
"device");
device = (unsigned int)strtoul(buf + 2, NULL, 16); buf = sysfs_get_value_from_attributes(cur_attrs,
"subsystem_vendor");
subsystem_vendor = (unsigned int)strtoul(buf + 2,
NULL, 16);
buf = sysfs_get_value_from_attributes(cur_attrs,
"subsystem_device");
subsystem_device = (unsigned int)strtoul(buf + 2,
NULL, 16);
sysfs_close_list(cur_attrs);

printf("device '%s' has vendor %x device %x
subvendor %x subdevice %x\n",

                                 class_dev->path, vendor, device,
subsystem_vendor, subsystem_device);
                        printf("point2\n");
                        break;
                }
                printf("point3\n");

                sysfs_dev_parent = sysfs_get_device_parent(sysfs_dev);
                printf("point4\n");
                if (sysfs_dev_parent = NULL)
                        break;
                printf("point5\n");

                sysfs_dev = sysfs_dev_parent;
                printf("point6\n");
        }
        printf("point7\n");
        sysfs_close_device(sysfs_dev);

        printf("point8\n");
exit:
        return retval;
}

And if I compile udev with DEBUG=true, it works perfectly. If I compile udev without debugging, it hangs between point7 and point8.

Did I make any mistakes in the above routine which trigger the hang? I ask here because the libsysfs version in udev differs from libsysfs 1.0.0.

Regards,
Carl-Daniel

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id70&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

<snip>

At this point in time, sysfs_dev will not point to the oritinal device you got from the sysfs_get_glassdev_device() call, right?

That's probably the bug.

Hope this helps,

greg k-h

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id70&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Right.

Then it has to be fixed in udevinfo.c:print_device_chain, too.

Unfortunately this does not explain why the lockup vanishes once I compile with DEBUG=true. Added debugging should not change the behaviour of a library, right?

Thanks,
Carl-Daniel

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id70&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Probably. Care to send a patch?

It "shouldn't", correct. But who knows :)

Why not ask on the mailing list for libsysfs?

thanks,

greg k-h

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id70&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Hi Carl-Daniel,

First of all, you've opened a class_dev with an "open" call -> sysfs_open_class_device_path. This should have a matching "close" call to clean up the class device you've created. Instead, you assign the parent reference that's returned from a "get" call to class_dev. This isn't a good idea.

Second, you use a "get" function to retrieve a reference to the class_dev's attributes -> sysfs_get_classdev_attributes. Then you close the list using sysfs_close_list() rather than leaving that to the sysfs_close_classdev() function.

Open functions in libsysfs create a device - and the memory for it - and return you the structure. These "open" functions must be followed with a "close" function to clean everything up.

Get functions are used on opened structures to return references to what you'd like to see like attributes or devices. They don't need to be "closed".

Here's where you close a list that you received through a "get" call. You shouldn't call close on references you receive with "get" functions.

Here's where you close a reference you received through a "get" call.

I will create a similar routine, using correct libsysfs formats, and see if I run into a race.

Please let me know if this helps or if I can explain further.

Thanks,

Dan

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id70&alloc_id638&opÌk
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Yes, udevinfo.c print_device_chain is wrong. It doesn't properly use libsysfs mechanisms of "open/close" and "get". I will make a patch, unless someone else beats me to it.

If there's confusion on "open/close" and "get" and how that works with libsysfs, please let me know. I can explain further.

Thanks,

Dan

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id70&alloc_id638&opÌk
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Well, I assumed the udevinfo.c code was correct and took it as a starting point since the libsysfs documentation has many bugs and sports unclear wording (will send a patch for those sections once I've understood what all of those functions do).

Greg: I noticed libsysfs in udev was updated recently, but it seems to differ in a few definitions from libsysfs-1.0.0. Is that intended?

To explain my motivation for the buggy code I posted:
For my ataraid detection code, I need a list of all hard disks attached to the system hanging off a pci ATA/SATA controller. For each disk, I need the size, the ability to read from the correct device, the vendor/device/ subsystem_vendor/subsystem_device of the hard disk controller.

struct harddisk {
	char name[SYSFS_PATH_MAX];
	dev_t device;
	unsigned long size;
	unsigned int vendor;
	unsigned int device;
	unsigned int subsystem_vendor;
	unsigned int subsystem_device;
}

The ataraid detection gets a list of struct harddisk and checks if it can find devices which are the basis of a Promise/Highpoint/whatever RAID.

Greg: Is there some block device enumeration code in udev I could adapt to my purposes?
I was inspired to hack a bit on udev by kpartx (utility to enable partitions on top of dm) which uses the udev build environment quite successfully and am contemplating where ataraid detection from userspace should end up in the long run. Should it be integrated into udev? Separate package?

Dan: Is there a preferred way for getting the above accomplished with libsysfs?

Regards,
Carl-Daniel

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id70&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Yes, it both lags and then jumps ahead of the "main line" libsysfs code at times due to development by the libsysfs developers. That's one reason we contain our own version of libsysfs and don't rely on the system-wide version.

udev doesn't enumerate over a range of devices as it is passed a specific device that it cares about, sorry.

greg k-h

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id70&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel