[PATCH 3/5] ocfs2: dir, refcounttree, xattr: replace swap functions with built-in one
Replace dx_leaf_sort_swap, swap_refcount_rec and swap_xe functions
with built-in one, because they do only a simple byte to byte swap.
Signed-off-by: Andrey Abramov
---
fs/ocfs2/dir.c | 13 +
fs/ocfs2/refcounttree.c | 13 +++--
fs/ocfs2/xattr.c| 15 +++
3 files changed, 7 insertions(+), 34 deletions(-)
diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index c121abbdfc7d..4b86b181df0a 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -3529,16 +3529,6 @@ static int dx_leaf_sort_cmp(const void *a, const void *b)
return 0;
}
-static void dx_leaf_sort_swap(void *a, void *b, int size)
-{
- struct ocfs2_dx_entry *entry1 = a;
- struct ocfs2_dx_entry *entry2 = b;
-
- BUG_ON(size != sizeof(*entry1));
-
- swap(*entry1, *entry2);
-}
-
static int ocfs2_dx_leaf_same_major(struct ocfs2_dx_leaf *dx_leaf)
{
struct ocfs2_dx_entry_list *dl_list = _leaf->dl_list;
@@ -3799,8 +3789,7 @@ static int ocfs2_dx_dir_rebalance(struct ocfs2_super
*osb, struct inode *dir,
* This block is changing anyway, so we can sort it in place.
*/
sort(dx_leaf->dl_list.de_entries, num_used,
-sizeof(struct ocfs2_dx_entry), dx_leaf_sort_cmp,
-dx_leaf_sort_swap);
+sizeof(struct ocfs2_dx_entry), dx_leaf_sort_cmp, NULL);
ocfs2_journal_dirty(handle, dx_leaf_bh);
diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
index 1dc9a08e8bdc..7bbc94d23a0c 100644
--- a/fs/ocfs2/refcounttree.c
+++ b/fs/ocfs2/refcounttree.c
@@ -1400,13 +1400,6 @@ static int cmp_refcount_rec_by_cpos(const void *a, const
void *b)
return 0;
}
-static void swap_refcount_rec(void *a, void *b, int size)
-{
- struct ocfs2_refcount_rec *l = a, *r = b;
-
- swap(*l, *r);
-}
-
/*
* The refcount cpos are ordered by their 64bit cpos,
* But we will use the low 32 bit to be the e_cpos in the b-tree.
@@ -1482,7 +1475,7 @@ static int ocfs2_divide_leaf_refcount_block(struct
buffer_head *ref_leaf_bh,
*/
sort(>rl_recs, le16_to_cpu(rl->rl_used),
sizeof(struct ocfs2_refcount_rec),
-cmp_refcount_rec_by_low_cpos, swap_refcount_rec);
+cmp_refcount_rec_by_low_cpos, NULL);
ret = ocfs2_find_refcount_split_pos(rl, , _index);
if (ret) {
@@ -1507,11 +1500,11 @@ static int ocfs2_divide_leaf_refcount_block(struct
buffer_head *ref_leaf_bh,
sort(>rl_recs, le16_to_cpu(rl->rl_used),
sizeof(struct ocfs2_refcount_rec),
-cmp_refcount_rec_by_cpos, swap_refcount_rec);
+cmp_refcount_rec_by_cpos, NULL);
sort(_rl->rl_recs, le16_to_cpu(new_rl->rl_used),
sizeof(struct ocfs2_refcount_rec),
-cmp_refcount_rec_by_cpos, swap_refcount_rec);
+cmp_refcount_rec_by_cpos, NULL);
*split_cpos = cpos;
return 0;
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 3a24ce3deb01..b3e6f42baf78 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -4175,15 +4175,6 @@ static int cmp_xe(const void *a, const void *b)
return 0;
}
-static void swap_xe(void *a, void *b, int size)
-{
- struct ocfs2_xattr_entry *l = a, *r = b, tmp;
-
- tmp = *l;
- memcpy(l, r, sizeof(struct ocfs2_xattr_entry));
- memcpy(r, , sizeof(struct ocfs2_xattr_entry));
-}
-
/*
* When the ocfs2_xattr_block is filled up, new bucket will be created
* and all the xattr entries will be moved to the new bucket.
@@ -4249,7 +4240,7 @@ static void ocfs2_cp_xattr_block_to_bucket(struct inode
*inode,
trace_ocfs2_cp_xattr_block_to_bucket_end(offset, size, off_change);
sort(target + offset, count, sizeof(struct ocfs2_xattr_entry),
-cmp_xe, swap_xe);
+cmp_xe, NULL);
}
/*
@@ -,7 +4435,7 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
*/
sort(entries, le16_to_cpu(xh->xh_count),
sizeof(struct ocfs2_xattr_entry),
-cmp_xe_offset, swap_xe);
+cmp_xe_offset, NULL);
/* Move all name/values to the end of the bucket. */
xe = xh->xh_entries;
@@ -4486,7 +4477,7 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
/* sort the entries by their name_hash. */
sort(entries, le16_to_cpu(xh->xh_count),
sizeof(struct ocfs2_xattr_entry),
-cmp_xe, swap_xe);
+cmp_xe, NULL);
buf = bucket_buf;
for (i = 0; i < bucket->bu_blocks; i++, buf += blocksize)
--
2.21.0
[PATCH 1/5] arch/arc: unwind.c: replace swap function with built-in one
Replace swap_eh_frame_hdr_table_entries with built-in one, because
swap_eh_frame_hdr_table_entries does a simple byte to byte swap.
Signed-off-by: Andrey Abramov
---
arch/arc/kernel/unwind.c | 20 ++--
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/arch/arc/kernel/unwind.c b/arch/arc/kernel/unwind.c
index 271e9fafa479..7610fe84afea 100644
--- a/arch/arc/kernel/unwind.c
+++ b/arch/arc/kernel/unwind.c
@@ -248,20 +248,6 @@ static int cmp_eh_frame_hdr_table_entries(const void *p1,
const void *p2)
return (e1->start > e2->start) - (e1->start < e2->start);
}
-static void swap_eh_frame_hdr_table_entries(void *p1, void *p2, int size)
-{
- struct eh_frame_hdr_table_entry *e1 = p1;
- struct eh_frame_hdr_table_entry *e2 = p2;
- unsigned long v;
-
- v = e1->start;
- e1->start = e2->start;
- e2->start = v;
- v = e1->fde;
- e1->fde = e2->fde;
- e2->fde = v;
-}
-
static void init_unwind_hdr(struct unwind_table *table,
void *(*alloc) (unsigned long))
{
@@ -354,10 +340,8 @@ static void init_unwind_hdr(struct unwind_table *table,
}
WARN_ON(n != header->fde_count);
- sort(header->table,
-n,
-sizeof(*header->table),
-cmp_eh_frame_hdr_table_entries, swap_eh_frame_hdr_table_entries);
+ sort(header->table, n,
+sizeof(*header->table), cmp_eh_frame_hdr_table_entries, NULL);
table->hdrsz = hdrSize;
smp_wmb();
--
2.21.0
[PATCH 2/5] powerpc: module_[32|64].c: replace swap function with built-in one
Replace relaswap with built-in one, because of relaswap
does a simple byte to byte swap.
Signed-off-by: Andrey Abramov
---
arch/powerpc/kernel/module_32.c | 17 +
arch/powerpc/kernel/module_64.c | 17 +
2 files changed, 2 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/kernel/module_32.c b/arch/powerpc/kernel/module_32.c
index 88d83771f462..c311e8575d10 100644
--- a/arch/powerpc/kernel/module_32.c
+++ b/arch/powerpc/kernel/module_32.c
@@ -79,21 +79,6 @@ static int relacmp(const void *_x, const void *_y)
return 0;
}
-static void relaswap(void *_x, void *_y, int size)
-{
- uint32_t *x, *y, tmp;
- int i;
-
- y = (uint32_t *)_x;
- x = (uint32_t *)_y;
-
- for (i = 0; i < sizeof(Elf32_Rela) / sizeof(uint32_t); i++) {
- tmp = x[i];
- x[i] = y[i];
- y[i] = tmp;
- }
-}
-
/* Get the potential trampolines size required of the init and
non-init sections */
static unsigned long get_plt_size(const Elf32_Ehdr *hdr,
@@ -130,7 +115,7 @@ static unsigned long get_plt_size(const Elf32_Ehdr *hdr,
*/
sort((void *)hdr + sechdrs[i].sh_offset,
sechdrs[i].sh_size / sizeof(Elf32_Rela),
-sizeof(Elf32_Rela), relacmp, relaswap);
+sizeof(Elf32_Rela), relacmp, NULL);
ret += count_relocs((void *)hdr
+ sechdrs[i].sh_offset,
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 8661eea78503..0c833d7f36f1 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -231,21 +231,6 @@ static int relacmp(const void *_x, const void *_y)
return 0;
}
-static void relaswap(void *_x, void *_y, int size)
-{
- uint64_t *x, *y, tmp;
- int i;
-
- y = (uint64_t *)_x;
- x = (uint64_t *)_y;
-
- for (i = 0; i < sizeof(Elf64_Rela) / sizeof(uint64_t); i++) {
- tmp = x[i];
- x[i] = y[i];
- y[i] = tmp;
- }
-}
-
/* Get size of potential trampolines required. */
static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
const Elf64_Shdr *sechdrs)
@@ -269,7 +254,7 @@ static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
*/
sort((void *)sechdrs[i].sh_addr,
sechdrs[i].sh_size / sizeof(Elf64_Rela),
-sizeof(Elf64_Rela), relacmp, relaswap);
+sizeof(Elf64_Rela), relacmp, NULL);
relocs += count_relocs((void *)sechdrs[i].sh_addr,
sechdrs[i].sh_size
--
2.21.0
[PATCH 0/5] simple sort swap function usage improvements
This is the logical continuation of sort improvements by George Spelvin "lib/sort & lib/list_sort: faster and smaller" series (added to the linux-next really recently). Patches from 1 to 4 replace simple swap functions with the built-in (which is now much faster) and grouped by subsystem (after that only 3 files implement custom swap - arch/x86/kernel/unwind_orc.c, kernel/jump_label.c and lib/extable.c). Patch #5 replaces the int type with the size_t type of the size argument in the swap function. Andrey Abramov (5): arch/arc: unwind.c: replace swap function with built-in one powerpc: module_[32|64].c: replace swap function with built-in one ocfs2: dir,refcounttree,xattr: replace swap functions with built-in one ubifs: find.c: replace swap function with built-in one Lib: sort.h: replace int size with size_t size in the swap function arch/arc/kernel/unwind.c| 20 ++-- arch/powerpc/kernel/module_32.c | 17 + arch/powerpc/kernel/module_64.c | 17 + arch/x86/kernel/unwind_orc.c| 2 +- fs/ocfs2/dir.c | 13 + fs/ocfs2/refcounttree.c | 13 +++-- fs/ocfs2/xattr.c| 15 +++ fs/ubifs/find.c | 9 + include/linux/sort.h| 2 +- kernel/jump_label.c | 2 +- lib/extable.c | 2 +- lib/sort.c | 6 +++--- 12 files changed, 19 insertions(+), 99 deletions(-) -- 2.21.0
[PATCH 5/5] Lib: sort.h: replace int size with size_t size in the swap function
Replace int type with size_t type of the size argument
in the swap function, also affect all its dependencies.
Signed-off-by: Andrey Abramov
---
arch/x86/kernel/unwind_orc.c | 2 +-
include/linux/sort.h | 2 +-
kernel/jump_label.c | 2 +-
lib/extable.c| 2 +-
lib/sort.c | 6 +++---
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 89be1be1790c..1078c287198c 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -176,7 +176,7 @@ static struct orc_entry *orc_find(unsigned long ip)
return orc_ftrace_find(ip);
}
-static void orc_sort_swap(void *_a, void *_b, int size)
+static void orc_sort_swap(void *_a, void *_b, size_t size)
{
struct orc_entry *orc_a, *orc_b;
struct orc_entry orc_tmp;
diff --git a/include/linux/sort.h b/include/linux/sort.h
index 2b99a5dd073d..aea39d552ff7 100644
--- a/include/linux/sort.h
+++ b/include/linux/sort.h
@@ -6,6 +6,6 @@
void sort(void *base, size_t num, size_t size,
int (*cmp)(const void *, const void *),
- void (*swap)(void *, void *, int));
+ void (*swap)(void *, void *, size_t));
#endif
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index bad96b476eb6..340b788571fb 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -45,7 +45,7 @@ static int jump_label_cmp(const void *a, const void *b)
return 0;
}
-static void jump_label_swap(void *a, void *b, int size)
+static void jump_label_swap(void *a, void *b, size_t size)
{
long delta = (unsigned long)a - (unsigned long)b;
struct jump_entry *jea = a;
diff --git a/lib/extable.c b/lib/extable.c
index f54996fdd0b8..db2888342cd7 100644
--- a/lib/extable.c
+++ b/lib/extable.c
@@ -28,7 +28,7 @@ static inline unsigned long ex_to_insn(const struct
exception_table_entry *x)
#ifndef ARCH_HAS_RELATIVE_EXTABLE
#define swap_exNULL
#else
-static void swap_ex(void *a, void *b, int size)
+static void swap_ex(void *a, void *b, size_t size)
{
struct exception_table_entry *x = a, *y = b, tmp;
int delta = b - a;
diff --git a/lib/sort.c b/lib/sort.c
index 50855ea8c262..60fbbc29104a 100644
--- a/lib/sort.c
+++ b/lib/sort.c
@@ -114,7 +114,7 @@ static void swap_bytes(void *a, void *b, size_t n)
} while (n);
}
-typedef void (*swap_func_t)(void *a, void *b, int size);
+typedef void (*swap_func_t)(void *a, void *b, size_t size);
/*
* The values are arbitrary as long as they can't be confused with
@@ -138,7 +138,7 @@ static void do_swap(void *a, void *b, size_t size,
swap_func_t swap_func)
else if (swap_func == SWAP_BYTES)
swap_bytes(a, b, size);
else
- swap_func(a, b, (int)size);
+ swap_func(a, b, size);
}
/**
@@ -187,7 +187,7 @@ static size_t parent(size_t i, unsigned int lsbit, size_t
size)
*/
void sort(void *base, size_t num, size_t size,
int (*cmp_func)(const void *, const void *),
- void (*swap_func)(void *, void *, int size))
+ void (*swap_func)(void *, void *, size_t size))
{
/* pre-scale counters for performance */
size_t n = num * size, a = (num/2) * size;
--
2.21.0
[PATCH 4/5] ubifs: find.c: replace swap function with built-in one
Replace swap_dirty_idx function with built-in one,
because swap_dirty_idx does only a simple byte to byte swap.
Signed-off-by: Andrey Abramov
---
fs/ubifs/find.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/fs/ubifs/find.c b/fs/ubifs/find.c
index f9646835b026..5deaae7fcead 100644
--- a/fs/ubifs/find.c
+++ b/fs/ubifs/find.c
@@ -747,12 +747,6 @@ static int cmp_dirty_idx(const struct ubifs_lprops **a,
return lpa->dirty + lpa->free - lpb->dirty - lpb->free;
}
-static void swap_dirty_idx(struct ubifs_lprops **a, struct ubifs_lprops **b,
- int size)
-{
- swap(*a, *b);
-}
-
/**
* ubifs_save_dirty_idx_lnums - save an array of the most dirty index LEB nos.
* @c: the UBIFS file-system description object
@@ -772,8 +766,7 @@ int ubifs_save_dirty_idx_lnums(struct ubifs_info *c)
sizeof(void *) * c->dirty_idx.cnt);
/* Sort it so that the dirtiest is now at the end */
sort(c->dirty_idx.arr, c->dirty_idx.cnt, sizeof(void *),
-(int (*)(const void *, const void *))cmp_dirty_idx,
-(void (*)(void *, void *, int))swap_dirty_idx);
+(int (*)(const void *, const void *))cmp_dirty_idx, NULL);
dbg_find("found %d dirty index LEBs", c->dirty_idx.cnt);
if (c->dirty_idx.cnt)
dbg_find("dirtiest index LEB is %d with dirty %d and free %d",
--
2.21.0
Re: CONFIG_ARCH_SUPPORTS_INT128: Why not mips, s390, powerpc, and alpha?
On Sat, Mar 30, 2019 at 10:30:23AM +, George Spelvin wrote: > For s390, that was added on 24 March 2017 by > https://gcc.gnu.org/viewcvs/gcc?view=revision=246457 > which is part of GCC 7. > > It also only applies to TARGET_ARCH12, which I am guessing > corresponds to HAVE_MARCH_ZEC12_FEATURES. zEC12 is arch10, while z14 is arch12. See https://sourceware.org/binutils/docs-2.32/as/s390-Options.html#s390-Options for example; it lists the correspondences, and states "The processor names starting with arch refer to the edition number in the Principle of Operations manual. They can be used as alternate processor names and have been added for compatibility with the IBM XL compiler." Newer GCC does not use the somewhat confusing TARGET_ARCH12 name anymore; see https://gcc.gnu.org/r264796 . Segher
Re: CONFIG_ARCH_SUPPORTS_INT128: Why not mips, s390, powerpc, and alpha?
On Sat, Mar 30, 2019 at 11:28:21AM +, George Spelvin wrote: > >> I like that the MIPS code leaves the high half of the product in > >> the hi register until it tests the low half; I wish PowerPC would > >> similarly move the mulhdu *after* the loop, > > > The MIPS code has the multiplication inside the loop as well, and even > > the mfhi I think: MIPS has delay slots. > > Yes, it's in the delay slot, which is fine (the branch is unlikely, > after all). But it does the compare (sltu) before accessing %hi, which > is good as %hi often has a longer latency than %lo. (On out-of-order > cores, of course, none of this matters.) Yes. But it does the mfhi on every iteration, while it only needs it for the last one (or after the last one). This may not be more expensive for the actual hardware, but it is for GCC's cost model > > GCC treats the int128 as one register until it has expanded to RTL, and it > > does not do such loop optimisations after that, apparently. > > > > File a PR please? https://gcc.gnu.org/bugzilla/ > > Er... about what? The fact that the PowerPC code is not > >> PowerPC: > >> .L9: > >>bl get_random_u64 > >>nop > >>mulld 9,3,31 > >>cmpld 7,30,9 > >>bgt 7,.L9 > >>mulhdu 3,3,31 > > I'm not sure quite how to explain it in gcc-ese. Yeah, exactly, like that. This transformation is called "loop sinking" usually: if anything that is set within a loop is only used after the loop, it can be set after the loop (provided you keep the set's sources alive). Segher
Re: CONFIG_ARCH_SUPPORTS_INT128: Why not mips, s390, powerpc, and alpha?
On Sat, Mar 30, 2019 at 09:00:15AM +1300, Michael Cree wrote: > It does move the umulh inside the loop but that seems sensible since > the use of unlikely() implies that the loop is unlikely to be taken > so on average it would be a good bet to start the calculation of > umulh earlier since it has a few cycles latency to get the result, > and it is pipelined so it can be calculated in the shadow of the > mulq instruction on the same execution unit. That may make sense, but it is not what happens, sorry. It _starts off_ as part of the loop, and it is never moved outside. The only difference between a likely loop and an unlikely loop here I've seen (on all targets I tried) is that with a likely loop the loop target is aligned, while with an unlikely loop it isn't. > On the older CPUs > (before EV6 which are not out-of-order execution) having the umulh > inside the loop may be a net gain. Yes. Similarly, on Power you can often calculate the high mul at the same time as the low mul, for no extra cost. This may be true on many archs. Segher
Re: [PATCH 5/5] Lib: sort.h: replace int size with size_t size in the swap function
On Sat, Mar 30, 2019 at 07:43:53PM +0300, Andrey Abramov wrote: > Replace int type with size_t type of the size argument > in the swap function, also affect all its dependencies. This says _what_ the patch does, but it gives no clue as to _why_ you are doing this. Neither did your 0/5 patch :( Why make this change? Nothing afterward depends on it from what I can tell, so why is it needed? thanks, greg k-h
Re: [PATCH 0/5] simple sort swap function usage improvements
On Sat, Mar 30, 2019 at 6:39 PM Andrey Abramov wrote: > > This is the logical continuation of sort improvements by George Spelvin > "lib/sort & lib/list_sort: faster and smaller" series > (added to the linux-next really recently). > > Patches from 1 to 4 replace simple swap functions with the built-in > (which is now much faster) and grouped by subsystem (after that only > 3 files implement custom swap - arch/x86/kernel/unwind_orc.c, > kernel/jump_label.c and lib/extable.c). > > Patch #5 replaces the int type with the size_t type of the size argument > in the swap function. > Andrey Abramov (5): > arch/arc: unwind.c: replace swap function with built-in one > powerpc: module_[32|64].c: replace swap function with built-in one > ocfs2: dir,refcounttree,xattr: replace swap functions with built-in > one > ubifs: find.c: replace swap function with built-in one > Lib: sort.h: replace int size with size_t size in the swap function You have to do something about Cc list. 50 people is complete overkill. Seems as your series is a set of independent fixes. Why not to split and Cc based on individual case? -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/5] simple sort swap function usage improvements
Great work; that is indeed a logical follow-on. Reviewed by: George Spelvin I you feel even more ambitious, you could try impementing Rasmus Villemoes' idea of having generic *compare* functions. (It's on my to-do list, but I haven't made meaningful progress yet, and I'm happy to pawn it off.) A significant fraction of the time, the sort key is a 4- or 8-byte integer field in a structure at a small offset from the base or list_head. A function pointer < 4096 could be interpreted as encoding: - Key size (1 bit) - Key signedness (1 bit) - Sort direction (1 bit) - Offset (9 bits; +/-256 words = +/-1024 bytes, or 0..511 words from start) With the correct level of preprocessor hackery, SIMPLE_CMP_ASCENDING(struct type, key_field) SIMPLE_LIST_CMP_ASCENDING(struct type, list_field, key_field) SIMPLE_CMP_DESCENDING(struct type, key_field) SIMPLE_LIST_CMP_DESCENDING(struct type, list_field, key_field) could encode all that and cause a compile-time error if the key is the wrong type or the offset is out of range.
Re: [RFC PATCH] drivers/dax: Allow to include DEV_DAX_PMEM as builtin
On Fri, Mar 29, 2019 at 10:42 PM Aneesh Kumar K.V wrote: > > This move the dependency to DEV_DAX_PMEM_COMPAT such that only > if DEV_DAX_PMEM is built as module we can allow the compat support. > > This allows to test the new code easily in a emulation setup where we > often build things without module support. > > Signed-off-by: Aneesh Kumar K.V > --- > drivers/dax/Kconfig | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/dax/Kconfig b/drivers/dax/Kconfig > index 5ef624fe3934..e582e088b48c 100644 > --- a/drivers/dax/Kconfig > +++ b/drivers/dax/Kconfig > @@ -23,7 +23,6 @@ config DEV_DAX > config DEV_DAX_PMEM > tristate "PMEM DAX: direct access to persistent memory" > depends on LIBNVDIMM && NVDIMM_DAX && DEV_DAX > - depends on m # until we can kill DEV_DAX_PMEM_COMPAT > default DEV_DAX > help > Support raw access to persistent memory. Note that this > @@ -50,7 +49,7 @@ config DEV_DAX_KMEM > > config DEV_DAX_PMEM_COMPAT > tristate "PMEM DAX: support the deprecated /sys/class/dax interface" > - depends on DEV_DAX_PMEM > + depends on DEV_DAX_PMEM=m Looks ok, just also a needs a "depends on m" here, because DEV_DAX_PMEM_COMPAT=y is an invalid config.
Re: CONFIG_ARCH_SUPPORTS_INT128: Why not mips, s390, powerpc, and alpha?
I've been tracking down when "umulditi3" support was added to various gcc platforms. So far, I've found: ia64: 2005-07-28, gcc 4.1.0 https://gcc.gnu.org/viewcvs/gcc?view=revision=102463 mips: 2008-06-09, gcc 4.4.0 https://gcc.gnu.org/viewcvs/gcc?view=revision=136600 alpha: 2013-02-01, gcc 4.8.0 https://gcc.gnu.org/viewcvs/gcc?view=revision=195668 s390: 2011-10-07, gcc 4.7.0 https://gcc.gnu.org/viewcvs/gcc?view=revision=179647 ppc64: 2013-02-01, gcc 4.8.0 https://gcc.gnu.org/viewcvs/gcc?view=revision=195667 Here's a revised s390 patch. >From e8ea8c0a5d618385049248649b8c13717b598a42 Mon Sep 17 00:00:00 2001 From: George Spelvin Date: Sat, 30 Mar 2019 10:27:14 + Subject: [PATCH v2] s390: Enable CONFIG_ARCH_SUPPORTS_INT128 on 64-bit builds If a platform supports a 64x64->128-bit widening multiply, that allows more efficient scaling of 64-bit values in various parts of the kernel. GCC advertises __int128 support with the __INT128__ #define, but we care about efficient inline support, so this is a separate flag. For s390, that was added on 2011-10-07 by https://gcc.gnu.org/viewcvs/gcc?view=revision=179647 which is part of GCC 4.7. Signed-off-by: George Spelvin --- arch/s390/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index ed554b09eb3f..6ddaee6573f4 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -372,6 +372,7 @@ endchoice config 64BIT def_bool y + select ARCH_SUPPORTS_INT128 if GCC_VERSION >= 40700 || CC_IS_CLANG config COMPAT def_bool y -- 2.20.1
Re: CONFIG_ARCH_SUPPORTS_INT128: Why not mips, s390, powerpc, and alpha?
General update:
I've since found the reason for the GCC version check.
It's not *broken* support (apologies for impugning GCC),
but *inefficient* support via a muditi3 helper function.
The version check is the version which added in-line code generation.
For example, s390 support was added in March of 2017 and is part
of the 7.1 release.
I hvave to do some digging through gcc version history to
find when it was added to various architectures.
(And MIPS64v6 support is still lacking in gcc 9.)
On Fri, 29 Mar 2019 at 15:25:58 -0500, Segher Boessenkool wrote:
> On Fri, Mar 29, 2019 at 01:07:07PM +, George Spelvin wrote:
>> I don't have easy access to an Alpha cross-compiler to test, but
>> as it has UMULH, I suspect it would work, too.
>
> https://mirrors.edge.kernel.org/pub/tools/crosstool/
Thanks for the pointer; I have a working Alpha cross-compiler now.
>> u64 get_random_u64(void);
>> u64 get_random_max64(u64 range, u64 lim)
>> {
>> unsigned __int128 prod;
>> do {
>> prod = (unsigned __int128)get_random_u64() * range;
>> } while (unlikely((u64)prod < lim));
>> return prod >> 64;
>> }
>
>> MIPS:
>> .L7:
>> jal get_random_u64
>> nop
>> dmultu $2,$17
>> mflo$3
>> sltu$4,$3,$16
>> bne $4,$0,.L7
>> mfhi$2
>>
>> PowerPC:
>> .L9:
>> bl get_random_u64
>> nop
>> mulld 9,3,31
>> mulhdu 3,3,31
>> cmpld 7,30,9
>> bgt 7,.L9
>>
>> I like that the MIPS code leaves the high half of the product in
>> the hi register until it tests the low half; I wish PowerPC would
>> similarly move the mulhdu *after* the loop,
> The MIPS code has the multiplication inside the loop as well, and even
> the mfhi I think: MIPS has delay slots.
Yes, it's in the delay slot, which is fine (the branch is unlikely,
after all). But it does the compare (sltu) before accessing %hi, which
is good as %hi often has a longer latency than %lo. (On out-of-order
cores, of course, none of this matters.)
> GCC treats the int128 as one register until it has expanded to RTL, and it
> does not do such loop optimisations after that, apparently.
>
> File a PR please? https://gcc.gnu.org/bugzilla/
Er... about what? The fact that the PowerPC code is not
>> PowerPC:
>> .L9:
>> bl get_random_u64
>> nop
>> mulld 9,3,31
>> cmpld 7,30,9
>> bgt 7,.L9
>> mulhdu 3,3,31
I'm not sure quite how to explain it in gcc-ese.
Re: CONFIG_ARCH_SUPPORTS_INT128: Why not mips, s390, powerpc, and alpha?
On Sat, 30 Mar 2019 at 09:43:47 +0100, Heiko Carstens wrote: > It hasn't been enabled on s390 simply because at least I wasn't aware > of this config option. Feel free to send a patch, otherwise I will > enable this. Whatever you prefer. > > Thanks for pointing this out! Here's a draft patch, but obviously it should be tested! >From 6f3cc608c49dba33a38e81232a2fd46e8fa8dbcd Mon Sep 17 00:00:00 2001 From: George Spelvin Date: Sat, 30 Mar 2019 10:27:14 + Subject: [PATCH] s390: Enable CONFIG_ARCH_SUPPORTS_INT128 on 64-bit builds If a platform supports a 64x64->128-bit widening multiply, that allows more efficient scaling of 64-bit values in various parts of the kernel. GCC advertises __int128 support with the __INT128__ #define, but we care about efficient inline support, so this is a separate flag. For s390, that was added on 24 March 2017 by https://gcc.gnu.org/viewcvs/gcc?view=revision=246457 which is part of GCC 7. It also only applies to TARGET_ARCH12, which I am guessing corresponds to HAVE_MARCH_ZEC12_FEATURES. clang support is pure guesswork. Signed-off-by: George Spelvin --- arch/s390/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index ed554b09eb3f..43e6dc677f7d 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -372,6 +372,7 @@ endchoice config 64BIT def_bool y + select ARCH_SUPPORTS_INT128 if GCC_VERSION >= 7 && HAVE_MARCH_ZEC12_FEATURES || CC_IS_CLANG config COMPAT def_bool y -- 2.20.1
Re: [PATCH -next] crypto: nx842 - remove set but not used variables 'dpadding' and 'max_sync_size'
On 3/30/2019 11:24 AM, Yue Haibing wrote:
From: YueHaibing
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/crypto/nx/nx-842.c: In function 'decompress':
drivers/crypto/nx/nx-842.c:356:25: warning: variable 'dpadding' set but not
used [-Wunused-but-set-variable]
drivers/crypto/nx/nx-842-pseries.c: In function 'nx842_pseries_compress':
drivers/crypto/nx/nx-842-pseries.c:299:15: warning: variable 'max_sync_size'
set but not used [-Wunused-but-set-variable]
drivers/crypto/nx/nx-842-pseries.c: In function 'nx842_pseries_decompress':
drivers/crypto/nx/nx-842-pseries.c:430:15: warning: variable 'max_sync_size'
set but not used [-Wunused-but-set-variable]
They are not used any more and can be removed.
Signed-off-by: YueHaibing
Reviewed-by: Mukesh Ojha
Cheers,
-Mukesh
---
drivers/crypto/nx/nx-842-pseries.c | 6 ++
drivers/crypto/nx/nx-842.c | 3 +--
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/crypto/nx/nx-842-pseries.c
b/drivers/crypto/nx/nx-842-pseries.c
index 6686997..5793284 100644
--- a/drivers/crypto/nx/nx-842-pseries.c
+++ b/drivers/crypto/nx/nx-842-pseries.c
@@ -296,7 +296,7 @@ static int nx842_pseries_compress(const unsigned char *in,
unsigned int inlen,
struct nx842_workmem *workmem;
struct nx842_scatterlist slin, slout;
struct nx_csbcpb *csbcpb;
- int ret = 0, max_sync_size;
+ int ret = 0;
unsigned long inbuf, outbuf;
struct vio_pfo_op op = {
.done = NULL,
@@ -319,7 +319,6 @@ static int nx842_pseries_compress(const unsigned char *in,
unsigned int inlen,
rcu_read_unlock();
return -ENODEV;
}
- max_sync_size = local_devdata->max_sync_size;
dev = local_devdata->dev;
/* Init scatterlist */
@@ -427,7 +426,7 @@ static int nx842_pseries_decompress(const unsigned char
*in, unsigned int inlen,
struct nx842_workmem *workmem;
struct nx842_scatterlist slin, slout;
struct nx_csbcpb *csbcpb;
- int ret = 0, max_sync_size;
+ int ret = 0;
unsigned long inbuf, outbuf;
struct vio_pfo_op op = {
.done = NULL,
@@ -451,7 +450,6 @@ static int nx842_pseries_decompress(const unsigned char
*in, unsigned int inlen,
rcu_read_unlock();
return -ENODEV;
}
- max_sync_size = local_devdata->max_sync_size;
dev = local_devdata->dev;
workmem = PTR_ALIGN(wmem, WORKMEM_ALIGN);
diff --git a/drivers/crypto/nx/nx-842.c b/drivers/crypto/nx/nx-842.c
index d94e25d..f06565d 100644
--- a/drivers/crypto/nx/nx-842.c
+++ b/drivers/crypto/nx/nx-842.c
@@ -353,7 +353,7 @@ static int decompress(struct nx842_crypto_ctx *ctx,
unsigned int adj_slen = slen;
u8 *src = p->in, *dst = p->out;
u16 padding = be16_to_cpu(g->padding);
- int ret, spadding = 0, dpadding = 0;
+ int ret, spadding = 0;
ktime_t timeout;
if (!slen || !required_len)
@@ -413,7 +413,6 @@ static int decompress(struct nx842_crypto_ctx *ctx,
spadding = 0;
dst = p->out;
dlen = p->oremain;
- dpadding = 0;
if (dlen < required_len) { /* have ignore bytes */
dst = ctx->dbounce;
dlen = BOUNCE_BUFFER_SIZE;
Re: [PATCH 2/2] arch: add pidfd and io_uring syscalls everywhere
On Mon, Mar 25, 2019 at 03:47:37PM +0100, Arnd Bergmann wrote: > Add the io_uring and pidfd_send_signal system calls to all architectures. > > These system calls are designed to handle both native and compat tasks, > so all entries are the same across architectures, only arm-compat and > the generic tale still use an old format. > > Signed-off-by: Arnd Bergmann > diff --git a/arch/s390/kernel/syscalls/syscall.tbl > b/arch/s390/kernel/syscalls/syscall.tbl > index 02579f95f391..3eb56e639b96 100644 > --- a/arch/s390/kernel/syscalls/syscall.tbl > +++ b/arch/s390/kernel/syscalls/syscall.tbl > @@ -426,3 +426,7 @@ > 421 32 rt_sigtimedwait_time64 - > compat_sys_rt_sigtimedwait_time64 > 422 32 futex_time64- > sys_futex > 423 32 sched_rr_get_interval_time64- > sys_sched_rr_get_interval > +424 common pidfd_send_signal sys_pidfd_send_signal > +425 common io_uring_setup sys_io_uring_setup > +426 common io_uring_enter sys_io_uring_enter > +427 common io_uring_register sys_io_uring_register I was just about to write that io_uring_enter is missing compat handling, but your first patch actually fixes that. Would have been good to be cc'ed on both patches :) For s390: Acked-by: Heiko Carstens
Re: CONFIG_ARCH_SUPPORTS_INT128: Why not mips, s390, powerpc, and alpha?
On Fri, Mar 29, 2019 at 01:07:07PM +, George Spelvin wrote: > (Cross-posted in case there are generic issues; please trim if > discussion wanders into single-architecture details.) > > I was working on some scaling code that can benefit from 64x64->128-bit > multiplies. GCC supports an __int128 type on processors with hardware > support (including z/Arch and MIPS64), but the support was broken on > early compilers, so it's gated behind CONFIG_ARCH_SUPPORTS_INT128. > > Currently, of the ten 64-bit architectures Linux supports, that's > only enabled on x86, ARM, and RISC-V. > > SPARC and HP-PA don't have support. > > But that leaves Alpha, Mips, PowerPC, and S/390x. > > Current mips64, powerpc64, and s390x gcc seems to generate sensible code > for mul_u64_u64_shr() in if I cross-compile them. > > I don't have easy access to an Alpha cross-compiler to test, but > as it has UMULH, I suspect it would work, too. > > Is there a reason it hasn't been enabled on these platforms? It hasn't been enabled on s390 simply because at least I wasn't aware of this config option. Feel free to send a patch, otherwise I will enable this. Whatever you prefer. Thanks for pointing this out!
Re: CONFIG_ARCH_SUPPORTS_INT128: Why not mips, s390, powerpc, and alpha?
Hi!
On Fri, Mar 29, 2019 at 01:07:07PM +, George Spelvin wrote:
> I was working on some scaling code that can benefit from 64x64->128-bit
> multiplies. GCC supports an __int128 type on processors with hardware
> support (including z/Arch and MIPS64), but the support was broken on
> early compilers, so it's gated behind CONFIG_ARCH_SUPPORTS_INT128.
>
> Currently, of the ten 64-bit architectures Linux supports, that's
> only enabled on x86, ARM, and RISC-V.
>
> SPARC and HP-PA don't have support.
>
> But that leaves Alpha, Mips, PowerPC, and S/390x.
>
> Current mips64, powerpc64, and s390x gcc seems to generate sensible code
> for mul_u64_u64_shr() in if I cross-compile them.
Yup.
> I don't have easy access to an Alpha cross-compiler to test, but
> as it has UMULH, I suspect it would work, too.
https://mirrors.edge.kernel.org/pub/tools/crosstool/
> u64 get_random_u64(void);
> u64 get_random_max64(u64 range, u64 lim)
> {
> unsigned __int128 prod;
> do {
> prod = (unsigned __int128)get_random_u64() * range;
> } while (unlikely((u64)prod < lim));
> return prod >> 64;
> }
> Which turns into these inner loops:
> MIPS:
> .L7:
> jal get_random_u64
> nop
> dmultu $2,$17
> mflo$3
> sltu$4,$3,$16
> bne $4,$0,.L7
> mfhi$2
>
> PowerPC:
> .L9:
> bl get_random_u64
> nop
> mulld 9,3,31
> mulhdu 3,3,31
> cmpld 7,30,9
> bgt 7,.L9
>
> s/390:
> .L13:
> brasl %r14,get_random_u64@PLT
> lgr %r5,%r2
> mlgr%r4,%r10
> lgr %r2,%r4
> clgr%r11,%r5
> jh .L13
>
> I like that the MIPS code leaves the high half of the product in
> the hi register until it tests the low half; I wish PowerPC would
> similarly move the mulhdu *after* the loop,
The MIPS code has the multiplication inside the loop as well, and even
the mfhi I think: MIPS has delay slots.
GCC treats the int128 as one register until it has expanded to RTL, and it
does not do such loop optimisations after that, apparently.
File a PR please? https://gcc.gnu.org/bugzilla/
Segher
Re: [PATCH] powerpc/64s/radix: Fix radix segment exception handling
Nicholas Piggin writes:
> Commit 48e7b76957 ("powerpc/64s/hash: Convert SLB miss handlers to C")
> broke the radix-mode segment exception handler. In radix mode, this is
> exception is not an SLB miss, rather it signals that the EA is outside
> the range translated by any page table.
>
> The commit lost the radix feature alternate code patch, which can
> cause faults to some EAs to kernel BUG at arch/powerpc/mm/slb.c:639!
>
> The original radix code would send faults to slb_miss_large_addr,
> which would end up faulting due to slb_addr_limit being 0. This patch
> sends radix directly to do_bad_slb_fault, which is a bit clearer.
>
Reviewed-by: Aneesh Kumar K.V
> Fixes: 48e7b76957 ("powerpc/64s/hash: Convert SLB miss handlers to C")
> Cc: Aneesh Kumar K.V
> Reported-by: Anton Blanchard
> Signed-off-by: Nicholas Piggin
> ---
> arch/powerpc/kernel/exceptions-64s.S | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S
> b/arch/powerpc/kernel/exceptions-64s.S
> index a5b8fbae56a0..9481a117e242 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -656,11 +656,17 @@ EXC_COMMON_BEGIN(data_access_slb_common)
> ld r4,PACA_EXSLB+EX_DAR(r13)
> std r4,_DAR(r1)
> addir3,r1,STACK_FRAME_OVERHEAD
> +BEGIN_MMU_FTR_SECTION
> + /* HPT case, do SLB fault */
> bl do_slb_fault
> cmpdi r3,0
> bne-1f
> b fast_exception_return
> 1: /* Error case */
> +MMU_FTR_SECTION_ELSE
> + /* Radix case, access is outside page table range */
> + li r3,-EFAULT
> +ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
> std r3,RESULT(r1)
> bl save_nvgprs
> RECONCILE_IRQ_STATE(r10, r11)
> @@ -705,11 +711,17 @@ EXC_COMMON_BEGIN(instruction_access_slb_common)
> EXCEPTION_PROLOG_COMMON(0x480, PACA_EXSLB)
> ld r4,_NIP(r1)
> addir3,r1,STACK_FRAME_OVERHEAD
> +BEGIN_MMU_FTR_SECTION
> + /* HPT case, do SLB fault */
> bl do_slb_fault
> cmpdi r3,0
> bne-1f
> b fast_exception_return
> 1: /* Error case */
> +MMU_FTR_SECTION_ELSE
> + /* Radix case, access is outside page table range */
> + li r3,-EFAULT
> +ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
> std r3,RESULT(r1)
> bl save_nvgprs
> RECONCILE_IRQ_STATE(r10, r11)
> --
> 2.20.1
