Commit af6e2b5b authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'ptp-attribute-cleanup' 



Dmitry Torokhov says:

====================
PTP attribute handling cleanup

PTP core was creating some attributes, such as "period" and "fifo", and the
entire "pins" attribute group, after creating class deevice, which creates
a race for userspace: uevent may arrive before all attributes are created.

This series of patches switches PTP to use is_visible() to control
visibility of attributes in a group, and device_create_with_groups() to
ensure that attributes are created before we notify userspace of a new
device.

v2:
- added Richard's acked-by to patch #1
- removed use of kmalloc_array in favor of kcalloc in patch #2 at
  Richard's request
- added a cover letter

v1:
- initial patch set
====================

Acked-by: default avatarRichard Cochran <richardcochran@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 01f0f425 85a66e55

drivers/ptp/ptp_clock.c

+11 −11
Original line number Diff line number Diff line
@@ -221,18 +221,17 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info, 
	mutex_init(&ptp->pincfg_mux);

	init_waitqueue_head(&ptp->tsev_wq);



	err = ptp_populate_pin_groups(ptp);

	if (err)

		goto no_pin_groups;



	/* Create a new device in our class. */

	ptp->dev = device_create(ptp_class, parent, ptp->devid, ptp,

	ptp->dev = device_create_with_groups(ptp_class, parent, ptp->devid,

					     ptp, ptp->pin_attr_groups,

					     "ptp%d", ptp->index);

	if (IS_ERR(ptp->dev))

		goto no_device;



	dev_set_drvdata(ptp->dev, ptp);



	err = ptp_populate_sysfs(ptp);

	if (err)

		goto no_sysfs;



	/* Register a new PPS source. */

	if (info->pps) {

		struct pps_source_info pps;
@@ -260,10 +259,10 @@ no_clock: 
	if (ptp->pps_source)

		pps_unregister_source(ptp->pps_source);

no_pps:

	ptp_cleanup_sysfs(ptp);

no_sysfs:

	device_destroy(ptp_class, ptp->devid);

no_device:

	ptp_cleanup_pin_groups(ptp);

no_pin_groups:

	mutex_destroy(&ptp->tsevq_mux);

	mutex_destroy(&ptp->pincfg_mux);

	ida_simple_remove(&ptp_clocks_map, index);
@@ -282,8 +281,9 @@ int ptp_clock_unregister(struct ptp_clock *ptp) 
	/* Release the clock's resources. */

	if (ptp->pps_source)

		pps_unregister_source(ptp->pps_source);

	ptp_cleanup_sysfs(ptp);



	device_destroy(ptp_class, ptp->devid);

	ptp_cleanup_pin_groups(ptp);



	posix_clock_unregister(&ptp->clock);

	return 0;

drivers/ptp/ptp_private.h

+4 −3
Original line number Diff line number Diff line
@@ -54,6 +54,8 @@ struct ptp_clock { 
	struct device_attribute *pin_dev_attr;

	struct attribute **pin_attr;

	struct attribute_group pin_attr_group;

	/* 1st entry is a pointer to the real group, 2nd is NULL terminator */

	const struct attribute_group *pin_attr_groups[2];

};



/*
@@ -94,8 +96,7 @@ uint ptp_poll(struct posix_clock *pc, 


extern const struct attribute_group *ptp_groups[];



int ptp_cleanup_sysfs(struct ptp_clock *ptp);



int ptp_populate_sysfs(struct ptp_clock *ptp);

int ptp_populate_pin_groups(struct ptp_clock *ptp);

void ptp_cleanup_pin_groups(struct ptp_clock *ptp);



#endif

drivers/ptp/ptp_sysfs.c

+65 −102
Original line number Diff line number Diff line
@@ -46,27 +46,6 @@ PTP_SHOW_INT(n_periodic_outputs, n_per_out); 
PTP_SHOW_INT(n_programmable_pins, n_pins);

PTP_SHOW_INT(pps_available, pps);



static struct attribute *ptp_attrs[] = {

	&dev_attr_clock_name.attr,

	&dev_attr_max_adjustment.attr,

	&dev_attr_n_alarms.attr,

	&dev_attr_n_external_timestamps.attr,

	&dev_attr_n_periodic_outputs.attr,

	&dev_attr_n_programmable_pins.attr,

	&dev_attr_pps_available.attr,

	NULL,

};



static const struct attribute_group ptp_group = {

	.attrs = ptp_attrs,

};



const struct attribute_group *ptp_groups[] = {

	&ptp_group,

	NULL,

};





static ssize_t extts_enable_store(struct device *dev,

				  struct device_attribute *attr,

				  const char *buf, size_t count)
@@ -91,6 +70,7 @@ static ssize_t extts_enable_store(struct device *dev, 
out:

	return err;

}

static DEVICE_ATTR(extts_enable, 0220, NULL, extts_enable_store);



static ssize_t extts_fifo_show(struct device *dev,

			       struct device_attribute *attr, char *page)
@@ -124,6 +104,7 @@ out: 
	mutex_unlock(&ptp->tsevq_mux);

	return cnt;

}

static DEVICE_ATTR(fifo, 0444, extts_fifo_show, NULL);



static ssize_t period_store(struct device *dev,

			    struct device_attribute *attr,
@@ -151,6 +132,7 @@ static ssize_t period_store(struct device *dev, 
out:

	return err;

}

static DEVICE_ATTR(period, 0220, NULL, period_store);



static ssize_t pps_enable_store(struct device *dev,

				struct device_attribute *attr,
@@ -177,6 +159,57 @@ static ssize_t pps_enable_store(struct device *dev, 
out:

	return err;

}

static DEVICE_ATTR(pps_enable, 0220, NULL, pps_enable_store);



static struct attribute *ptp_attrs[] = {

	&dev_attr_clock_name.attr,



	&dev_attr_max_adjustment.attr,

	&dev_attr_n_alarms.attr,

	&dev_attr_n_external_timestamps.attr,

	&dev_attr_n_periodic_outputs.attr,

	&dev_attr_n_programmable_pins.attr,

	&dev_attr_pps_available.attr,



	&dev_attr_extts_enable.attr,

	&dev_attr_fifo.attr,

	&dev_attr_period.attr,

	&dev_attr_pps_enable.attr,

	NULL

};



static umode_t ptp_is_attribute_visible(struct kobject *kobj,

					struct attribute *attr, int n)

{

	struct device *dev = kobj_to_dev(kobj);

	struct ptp_clock *ptp = dev_get_drvdata(dev);

	struct ptp_clock_info *info = ptp->info;

	umode_t mode = attr->mode;



	if (attr == &dev_attr_extts_enable.attr ||

	    attr == &dev_attr_fifo.attr) {

		if (!info->n_ext_ts)

			mode = 0;

	} else if (attr == &dev_attr_period.attr) {

		if (!info->n_per_out)

			mode = 0;

	} else if (attr == &dev_attr_pps_enable.attr) {

		if (!info->pps)

			mode = 0;

	}



	return mode;

}



static const struct attribute_group ptp_group = {

	.is_visible	= ptp_is_attribute_visible,

	.attrs		= ptp_attrs,

};



const struct attribute_group *ptp_groups[] = {

	&ptp_group,

	NULL

};



static int ptp_pin_name2index(struct ptp_clock *ptp, const char *name)

{
@@ -235,47 +268,20 @@ static ssize_t ptp_pin_store(struct device *dev, struct device_attribute *attr, 
	return count;

}



static DEVICE_ATTR(extts_enable, 0220, NULL, extts_enable_store);

static DEVICE_ATTR(fifo,         0444, extts_fifo_show, NULL);

static DEVICE_ATTR(period,       0220, NULL, period_store);

static DEVICE_ATTR(pps_enable,   0220, NULL, pps_enable_store);



int ptp_cleanup_sysfs(struct ptp_clock *ptp)

int ptp_populate_pin_groups(struct ptp_clock *ptp)

{

	struct device *dev = ptp->dev;

	struct ptp_clock_info *info = ptp->info;

	int err = -ENOMEM, i, n_pins = info->n_pins;



	if (info->n_ext_ts) {

		device_remove_file(dev, &dev_attr_extts_enable);

		device_remove_file(dev, &dev_attr_fifo);

	}

	if (info->n_per_out)

		device_remove_file(dev, &dev_attr_period);



	if (info->pps)

		device_remove_file(dev, &dev_attr_pps_enable);



	if (info->n_pins) {

		sysfs_remove_group(&dev->kobj, &ptp->pin_attr_group);

		kfree(ptp->pin_attr);

		kfree(ptp->pin_dev_attr);

	}

	if (!n_pins)

		return 0;

}



static int ptp_populate_pins(struct ptp_clock *ptp)

{

	struct device *dev = ptp->dev;

	struct ptp_clock_info *info = ptp->info;

	int err = -ENOMEM, i, n_pins = info->n_pins;



	ptp->pin_dev_attr = kzalloc(n_pins * sizeof(*ptp->pin_dev_attr),

	ptp->pin_dev_attr = kcalloc(n_pins, sizeof(*ptp->pin_dev_attr),

				    GFP_KERNEL);

	if (!ptp->pin_dev_attr)

		goto no_dev_attr;



	ptp->pin_attr = kzalloc((1 + n_pins) * sizeof(struct attribute *),

				GFP_KERNEL);

	ptp->pin_attr = kcalloc(1 + n_pins, sizeof(*ptp->pin_attr), GFP_KERNEL);

	if (!ptp->pin_attr)

		goto no_pin_attr;
@@ -292,61 +298,18 @@ static int ptp_populate_pins(struct ptp_clock *ptp) 
	ptp->pin_attr_group.name = "pins";

	ptp->pin_attr_group.attrs = ptp->pin_attr;



	err = sysfs_create_group(&dev->kobj, &ptp->pin_attr_group);

	if (err)

		goto no_group;

	ptp->pin_attr_groups[0] = &ptp->pin_attr_group;



	return 0;



no_group:

	kfree(ptp->pin_attr);

no_pin_attr:

	kfree(ptp->pin_dev_attr);

no_dev_attr:

	return err;

}



int ptp_populate_sysfs(struct ptp_clock *ptp)

void ptp_cleanup_pin_groups(struct ptp_clock *ptp)

{

	struct device *dev = ptp->dev;

	struct ptp_clock_info *info = ptp->info;

	int err;



	if (info->n_ext_ts) {

		err = device_create_file(dev, &dev_attr_extts_enable);

		if (err)

			goto out1;

		err = device_create_file(dev, &dev_attr_fifo);

		if (err)

			goto out2;

	}

	if (info->n_per_out) {

		err = device_create_file(dev, &dev_attr_period);

		if (err)

			goto out3;

	}

	if (info->pps) {

		err = device_create_file(dev, &dev_attr_pps_enable);

		if (err)

			goto out4;

	}

	if (info->n_pins) {

		err = ptp_populate_pins(ptp);

		if (err)

			goto out5;

	}

	return 0;

out5:

	if (info->pps)

		device_remove_file(dev, &dev_attr_pps_enable);

out4:

	if (info->n_per_out)

		device_remove_file(dev, &dev_attr_period);

out3:

	if (info->n_ext_ts)

		device_remove_file(dev, &dev_attr_fifo);

out2:

	if (info->n_ext_ts)

		device_remove_file(dev, &dev_attr_extts_enable);

out1:

	return err;

	kfree(ptp->pin_attr);

	kfree(ptp->pin_dev_attr);

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE