From db67097aa6f2587b44055f2e16db72a11e17faef Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Thu, 9 Dec 2021 21:31:56 -0700 Subject: pktdvd: stop using bdi congestion framework. The bdi congestion framework isn't widely used and should be deprecated. pktdvd makes use of it to track congestion, but this can be done entirely internally to pktdvd, so it doesn't need to use the framework. So introduce a "congested" flag. When waiting for bio_queue_size to drop, set this flag and a var_waitqueue() to wait for it. When bio_queue_size does drop and this flag is set, clear the flag and call wake_up_var(). We don't use a wait_var_event macro for the waiting as we need to set the flag and drop the spinlock before calling schedule() and while that is possible with __wait_var_event(), result is not easy to read. Reviewed-by: Christoph Hellwig Signed-off-by: NeilBrown Link: https://lore.kernel.org/r/163910843527.9928.857338663717630212@noble.neil.brown.name Signed-off-by: Jens Axboe --- include/linux/pktcdvd.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include') diff --git a/include/linux/pktcdvd.h b/include/linux/pktcdvd.h index 174601554b06..c391e694aa26 100644 --- a/include/linux/pktcdvd.h +++ b/include/linux/pktcdvd.h @@ -183,6 +183,8 @@ struct pktcdvd_device spinlock_t lock; /* Serialize access to bio_queue */ struct rb_root bio_queue; /* Work queue of bios we need to handle */ int bio_queue_size; /* Number of nodes in bio_queue */ + bool congested; /* Someone is waiting for bio_queue_size + * to drop. */ sector_t current_sector; /* Keep track of where the elevator is */ atomic_t scan_queue; /* Set to non-zero when pkt_handle_queue */ /* needs to be run. */ -- cgit v1.2.3 From d5dbcca70182501bed99de85c224cef04c38ed92 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 3 Jan 2022 17:24:08 +0100 Subject: pktcdvd: convert to use attribute groups There is no need to create kobject children of the pktcdvd device just to display a subdirectory name. Instead, use a named attribute group which removes the extra kobjects and also fixes the userspace race where the device is created yet tools like libudev can not see the attributes as they think the subdirectories are some other sort of device. Cc: linux-block@vger.kernel.org Cc: Jens Axboe Signed-off-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20220103162408.742003-1-gregkh@linuxfoundation.org Signed-off-by: Jens Axboe --- drivers/block/pktcdvd.c | 275 +++++++++++++++++++++++------------------------- include/linux/pktcdvd.h | 10 -- 2 files changed, 134 insertions(+), 151 deletions(-) (limited to 'include') diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index 713b7dcf39f9..2b6b70a39e76 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -113,57 +113,10 @@ static sector_t get_zone(sector_t sector, struct pktcdvd_device *pd) return (sector + pd->offset) & ~(sector_t)(pd->settings.size - 1); } -/* - * create and register a pktcdvd kernel object. - */ -static struct pktcdvd_kobj* pkt_kobj_create(struct pktcdvd_device *pd, - const char* name, - struct kobject* parent, - struct kobj_type* ktype) -{ - struct pktcdvd_kobj *p; - int error; - - p = kzalloc(sizeof(*p), GFP_KERNEL); - if (!p) - return NULL; - p->pd = pd; - error = kobject_init_and_add(&p->kobj, ktype, parent, "%s", name); - if (error) { - kobject_put(&p->kobj); - return NULL; - } - kobject_uevent(&p->kobj, KOBJ_ADD); - return p; -} -/* - * remove a pktcdvd kernel object. - */ -static void pkt_kobj_remove(struct pktcdvd_kobj *p) -{ - if (p) - kobject_put(&p->kobj); -} -/* - * default release function for pktcdvd kernel objects. - */ -static void pkt_kobj_release(struct kobject *kobj) -{ - kfree(to_pktcdvdkobj(kobj)); -} - - /********************************************************** - * * sysfs interface for pktcdvd * by (C) 2006 Thomas Maier - * - **********************************************************/ - -#define DEF_ATTR(_obj,_name,_mode) \ - static struct attribute _obj = { .name = _name, .mode = _mode } - -/********************************************************** + /sys/class/pktcdvd/pktcdvd[0-7]/ stat/reset stat/packets_started @@ -176,75 +129,94 @@ static void pkt_kobj_release(struct kobject *kobj) write_queue/congestion_on **********************************************************/ -DEF_ATTR(kobj_pkt_attr_st1, "reset", 0200); -DEF_ATTR(kobj_pkt_attr_st2, "packets_started", 0444); -DEF_ATTR(kobj_pkt_attr_st3, "packets_finished", 0444); -DEF_ATTR(kobj_pkt_attr_st4, "kb_written", 0444); -DEF_ATTR(kobj_pkt_attr_st5, "kb_read", 0444); -DEF_ATTR(kobj_pkt_attr_st6, "kb_read_gather", 0444); - -static struct attribute *kobj_pkt_attrs_stat[] = { - &kobj_pkt_attr_st1, - &kobj_pkt_attr_st2, - &kobj_pkt_attr_st3, - &kobj_pkt_attr_st4, - &kobj_pkt_attr_st5, - &kobj_pkt_attr_st6, - NULL -}; +static ssize_t packets_started_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct pktcdvd_device *pd = dev_get_drvdata(dev); -DEF_ATTR(kobj_pkt_attr_wq1, "size", 0444); -DEF_ATTR(kobj_pkt_attr_wq2, "congestion_off", 0644); -DEF_ATTR(kobj_pkt_attr_wq3, "congestion_on", 0644); + return sysfs_emit(buf, "%lu\n", pd->stats.pkt_started); +} +static DEVICE_ATTR_RO(packets_started); -static struct attribute *kobj_pkt_attrs_wqueue[] = { - &kobj_pkt_attr_wq1, - &kobj_pkt_attr_wq2, - &kobj_pkt_attr_wq3, - NULL -}; +static ssize_t packets_finished_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct pktcdvd_device *pd = dev_get_drvdata(dev); -static ssize_t kobj_pkt_show(struct kobject *kobj, - struct attribute *attr, char *data) + return sysfs_emit(buf, "%lu\n", pd->stats.pkt_ended); +} +static DEVICE_ATTR_RO(packets_finished); + +static ssize_t kb_written_show(struct device *dev, + struct device_attribute *attr, char *buf) { - struct pktcdvd_device *pd = to_pktcdvdkobj(kobj)->pd; - int n = 0; - int v; - if (strcmp(attr->name, "packets_started") == 0) { - n = sprintf(data, "%lu\n", pd->stats.pkt_started); + struct pktcdvd_device *pd = dev_get_drvdata(dev); - } else if (strcmp(attr->name, "packets_finished") == 0) { - n = sprintf(data, "%lu\n", pd->stats.pkt_ended); + return sysfs_emit(buf, "%lu\n", pd->stats.secs_w >> 1); +} +static DEVICE_ATTR_RO(kb_written); - } else if (strcmp(attr->name, "kb_written") == 0) { - n = sprintf(data, "%lu\n", pd->stats.secs_w >> 1); +static ssize_t kb_read_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct pktcdvd_device *pd = dev_get_drvdata(dev); - } else if (strcmp(attr->name, "kb_read") == 0) { - n = sprintf(data, "%lu\n", pd->stats.secs_r >> 1); + return sysfs_emit(buf, "%lu\n", pd->stats.secs_r >> 1); +} +static DEVICE_ATTR_RO(kb_read); - } else if (strcmp(attr->name, "kb_read_gather") == 0) { - n = sprintf(data, "%lu\n", pd->stats.secs_rg >> 1); +static ssize_t kb_read_gather_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct pktcdvd_device *pd = dev_get_drvdata(dev); - } else if (strcmp(attr->name, "size") == 0) { - spin_lock(&pd->lock); - v = pd->bio_queue_size; - spin_unlock(&pd->lock); - n = sprintf(data, "%d\n", v); + return sysfs_emit(buf, "%lu\n", pd->stats.secs_rg >> 1); +} +static DEVICE_ATTR_RO(kb_read_gather); - } else if (strcmp(attr->name, "congestion_off") == 0) { - spin_lock(&pd->lock); - v = pd->write_congestion_off; - spin_unlock(&pd->lock); - n = sprintf(data, "%d\n", v); +static ssize_t reset_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t len) +{ + struct pktcdvd_device *pd = dev_get_drvdata(dev); - } else if (strcmp(attr->name, "congestion_on") == 0) { - spin_lock(&pd->lock); - v = pd->write_congestion_on; - spin_unlock(&pd->lock); - n = sprintf(data, "%d\n", v); + if (len > 0) { + pd->stats.pkt_started = 0; + pd->stats.pkt_ended = 0; + pd->stats.secs_w = 0; + pd->stats.secs_rg = 0; + pd->stats.secs_r = 0; } + return len; +} +static DEVICE_ATTR_WO(reset); + +static struct attribute *pkt_stat_attrs[] = { + &dev_attr_packets_finished.attr, + &dev_attr_packets_started.attr, + &dev_attr_kb_read.attr, + &dev_attr_kb_written.attr, + &dev_attr_kb_read_gather.attr, + &dev_attr_reset.attr, + NULL, +}; + +static const struct attribute_group pkt_stat_group = { + .name = "stat", + .attrs = pkt_stat_attrs, +}; + +static ssize_t size_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct pktcdvd_device *pd = dev_get_drvdata(dev); + int n; + + spin_lock(&pd->lock); + n = sysfs_emit(buf, "%d\n", pd->bio_queue_size); + spin_unlock(&pd->lock); return n; } +static DEVICE_ATTR_RO(size); static void init_write_congestion_marks(int* lo, int* hi) { @@ -263,30 +235,56 @@ static void init_write_congestion_marks(int* lo, int* hi) } } -static ssize_t kobj_pkt_store(struct kobject *kobj, - struct attribute *attr, - const char *data, size_t len) +static ssize_t congestion_off_show(struct device *dev, + struct device_attribute *attr, char *buf) { - struct pktcdvd_device *pd = to_pktcdvdkobj(kobj)->pd; - int val; + struct pktcdvd_device *pd = dev_get_drvdata(dev); + int n; - if (strcmp(attr->name, "reset") == 0 && len > 0) { - pd->stats.pkt_started = 0; - pd->stats.pkt_ended = 0; - pd->stats.secs_w = 0; - pd->stats.secs_rg = 0; - pd->stats.secs_r = 0; + spin_lock(&pd->lock); + n = sysfs_emit(buf, "%d\n", pd->write_congestion_off); + spin_unlock(&pd->lock); + return n; +} + +static ssize_t congestion_off_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + struct pktcdvd_device *pd = dev_get_drvdata(dev); + int val; - } else if (strcmp(attr->name, "congestion_off") == 0 - && sscanf(data, "%d", &val) == 1) { + if (sscanf(buf, "%d", &val) == 1) { spin_lock(&pd->lock); pd->write_congestion_off = val; init_write_congestion_marks(&pd->write_congestion_off, &pd->write_congestion_on); spin_unlock(&pd->lock); + } + return len; +} +static DEVICE_ATTR_RW(congestion_off); + +static ssize_t congestion_on_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct pktcdvd_device *pd = dev_get_drvdata(dev); + int n; - } else if (strcmp(attr->name, "congestion_on") == 0 - && sscanf(data, "%d", &val) == 1) { + spin_lock(&pd->lock); + n = sysfs_emit(buf, "%d\n", pd->write_congestion_on); + spin_unlock(&pd->lock); + return n; +} + +static ssize_t congestion_on_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + struct pktcdvd_device *pd = dev_get_drvdata(dev); + int val; + + if (sscanf(buf, "%d", &val) == 1) { spin_lock(&pd->lock); pd->write_congestion_on = val; init_write_congestion_marks(&pd->write_congestion_off, @@ -295,44 +293,39 @@ static ssize_t kobj_pkt_store(struct kobject *kobj, } return len; } +static DEVICE_ATTR_RW(congestion_on); -static const struct sysfs_ops kobj_pkt_ops = { - .show = kobj_pkt_show, - .store = kobj_pkt_store +static struct attribute *pkt_wq_attrs[] = { + &dev_attr_congestion_on.attr, + &dev_attr_congestion_off.attr, + &dev_attr_size.attr, + NULL, }; -static struct kobj_type kobj_pkt_type_stat = { - .release = pkt_kobj_release, - .sysfs_ops = &kobj_pkt_ops, - .default_attrs = kobj_pkt_attrs_stat + +static const struct attribute_group pkt_wq_group = { + .name = "write_queue", + .attrs = pkt_wq_attrs, }; -static struct kobj_type kobj_pkt_type_wqueue = { - .release = pkt_kobj_release, - .sysfs_ops = &kobj_pkt_ops, - .default_attrs = kobj_pkt_attrs_wqueue + +static const struct attribute_group *pkt_groups[] = { + &pkt_stat_group, + &pkt_wq_group, + NULL, }; static void pkt_sysfs_dev_new(struct pktcdvd_device *pd) { if (class_pktcdvd) { - pd->dev = device_create(class_pktcdvd, NULL, MKDEV(0, 0), NULL, - "%s", pd->name); + pd->dev = device_create_with_groups(class_pktcdvd, NULL, + MKDEV(0, 0), pd, pkt_groups, + "%s", pd->name); if (IS_ERR(pd->dev)) pd->dev = NULL; } - if (pd->dev) { - pd->kobj_stat = pkt_kobj_create(pd, "stat", - &pd->dev->kobj, - &kobj_pkt_type_stat); - pd->kobj_wqueue = pkt_kobj_create(pd, "write_queue", - &pd->dev->kobj, - &kobj_pkt_type_wqueue); - } } static void pkt_sysfs_dev_remove(struct pktcdvd_device *pd) { - pkt_kobj_remove(pd->kobj_stat); - pkt_kobj_remove(pd->kobj_wqueue); if (class_pktcdvd) device_unregister(pd->dev); } diff --git a/include/linux/pktcdvd.h b/include/linux/pktcdvd.h index c391e694aa26..f9c5ac80d59b 100644 --- a/include/linux/pktcdvd.h +++ b/include/linux/pktcdvd.h @@ -152,14 +152,6 @@ struct packet_stacked_data }; #define PSD_POOL_SIZE 64 -struct pktcdvd_kobj -{ - struct kobject kobj; - struct pktcdvd_device *pd; -}; -#define to_pktcdvdkobj(_k) \ - ((struct pktcdvd_kobj*)container_of(_k,struct pktcdvd_kobj,kobj)) - struct pktcdvd_device { struct block_device *bdev; /* dev attached */ @@ -197,8 +189,6 @@ struct pktcdvd_device int write_congestion_on; struct device *dev; /* sysfs pktcdvd[0-7] dev */ - struct pktcdvd_kobj *kobj_stat; /* sysfs pktcdvd[0-7]/stat/ */ - struct pktcdvd_kobj *kobj_wqueue; /* sysfs pktcdvd[0-7]/write_queue/ */ struct dentry *dfs_d_root; /* debugfs: devname directory */ struct dentry *dfs_f_info; /* debugfs: info file */ -- cgit v1.2.3 From 36dacddbf0bdba86cd00f066b4d724157eeb63f1 Mon Sep 17 00:00:00 2001 From: Dirk Müller Date: Wed, 5 Jan 2022 17:38:47 +0100 Subject: lib/raid6: Use strict priority ranking for pq gen() benchmarking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On x86_64, currently 3 variants of AVX512, 3 variants of AVX2 and 3 variants of SSE2 are benchmarked on initialization, taking between 144-153 jiffies. Testing across a hardware pool of various generations of intel cpus I could not find a single case where SSE2 won over AVX2 or AVX512. There are cases where AVX2 wins over AVX512 however. Change "prefer" into an integer priority field (similar to how recov selection works) to have more than one ranking level available, which is backwards compatible with existing behavior. Give AVX2/512 variants higher priority over SSE2 in order to skip SSE testing when AVX is available. in a AVX2/x86_64/HZ=250 case this saves in the order of 200ms of initialization time. Signed-off-by: Dirk Müller Acked-by: Paul Menzel Signed-off-by: Song Liu --- include/linux/raid/pq.h | 2 +- lib/raid6/algos.c | 2 +- lib/raid6/avx2.c | 8 ++++---- lib/raid6/avx512.c | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) (limited to 'include') diff --git a/include/linux/raid/pq.h b/include/linux/raid/pq.h index 154e954b711d..d6e5a1feb947 100644 --- a/include/linux/raid/pq.h +++ b/include/linux/raid/pq.h @@ -81,7 +81,7 @@ struct raid6_calls { void (*xor_syndrome)(int, int, int, size_t, void **); int (*valid)(void); /* Returns 1 if this routine set is usable */ const char *name; /* Name of this routine set */ - int prefer; /* Has special performance attribute */ + int priority; /* Relative priority ranking if non-zero */ }; /* Selected algorithm */ diff --git a/lib/raid6/algos.c b/lib/raid6/algos.c index 9b7e8a837b27..39b74221f4a7 100644 --- a/lib/raid6/algos.c +++ b/lib/raid6/algos.c @@ -151,7 +151,7 @@ static inline const struct raid6_calls *raid6_choose_gen( const struct raid6_calls *best; for (bestgenperf = 0, best = NULL, algo = raid6_algos; *algo; algo++) { - if (!best || (*algo)->prefer >= best->prefer) { + if (!best || (*algo)->priority >= best->priority) { if ((*algo)->valid && !(*algo)->valid()) continue; diff --git a/lib/raid6/avx2.c b/lib/raid6/avx2.c index f299476e1d76..059024234dce 100644 --- a/lib/raid6/avx2.c +++ b/lib/raid6/avx2.c @@ -132,7 +132,7 @@ const struct raid6_calls raid6_avx2x1 = { raid6_avx21_xor_syndrome, raid6_have_avx2, "avx2x1", - 1 /* Has cache hints */ + .priority = 2 /* Prefer AVX2 over priority 1 (SSE2 and others) */ }; /* @@ -262,7 +262,7 @@ const struct raid6_calls raid6_avx2x2 = { raid6_avx22_xor_syndrome, raid6_have_avx2, "avx2x2", - 1 /* Has cache hints */ + .priority = 2 /* Prefer AVX2 over priority 1 (SSE2 and others) */ }; #ifdef CONFIG_X86_64 @@ -465,6 +465,6 @@ const struct raid6_calls raid6_avx2x4 = { raid6_avx24_xor_syndrome, raid6_have_avx2, "avx2x4", - 1 /* Has cache hints */ + .priority = 2 /* Prefer AVX2 over priority 1 (SSE2 and others) */ }; -#endif +#endif /* CONFIG_X86_64 */ diff --git a/lib/raid6/avx512.c b/lib/raid6/avx512.c index bb684d144ee2..9c3e822e1adf 100644 --- a/lib/raid6/avx512.c +++ b/lib/raid6/avx512.c @@ -162,7 +162,7 @@ const struct raid6_calls raid6_avx512x1 = { raid6_avx5121_xor_syndrome, raid6_have_avx512, "avx512x1", - 1 /* Has cache hints */ + .priority = 2 /* Prefer AVX512 over priority 1 (SSE2 and others) */ }; /* @@ -319,7 +319,7 @@ const struct raid6_calls raid6_avx512x2 = { raid6_avx5122_xor_syndrome, raid6_have_avx512, "avx512x2", - 1 /* Has cache hints */ + .priority = 2 /* Prefer AVX512 over priority 1 (SSE2 and others) */ }; #ifdef CONFIG_X86_64 @@ -557,7 +557,7 @@ const struct raid6_calls raid6_avx512x4 = { raid6_avx5124_xor_syndrome, raid6_have_avx512, "avx512x4", - 1 /* Has cache hints */ + .priority = 2 /* Prefer AVX512 over priority 1 (SSE2 and others) */ }; #endif -- cgit v1.2.3