[dm-devel] [PATCH 6/6] block: unexport bio_clone_bioset
Now only used by the bounce code, so move it there and mark the function
static.
Signed-off-by: Christoph Hellwig
---
block/bio.c | 77 -
block/bounce.c | 69 +++-
include/linux/bio.h | 1 -
3 files changed, 68 insertions(+), 79 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 9710e275f230..43698bcff737 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -644,83 +644,6 @@ struct bio *bio_clone_fast(struct bio *bio, gfp_t
gfp_mask, struct bio_set *bs)
}
EXPORT_SYMBOL(bio_clone_fast);
-/**
- * bio_clone_bioset - clone a bio
- * @bio_src: bio to clone
- * @gfp_mask: allocation priority
- * @bs: bio_set to allocate from
- *
- * Clone bio. Caller will own the returned bio, but not the actual data it
- * points to. Reference count of returned bio will be one.
- */
-struct bio *bio_clone_bioset(struct bio *bio_src, gfp_t gfp_mask,
-struct bio_set *bs)
-{
- struct bvec_iter iter;
- struct bio_vec bv;
- struct bio *bio;
-
- /*
-* Pre immutable biovecs, __bio_clone() used to just do a memcpy from
-* bio_src->bi_io_vec to bio->bi_io_vec.
-*
-* We can't do that anymore, because:
-*
-* - The point of cloning the biovec is to produce a bio with a biovec
-*the caller can modify: bi_idx and bi_bvec_done should be 0.
-*
-* - The original bio could've had more than BIO_MAX_PAGES biovecs; if
-*we tried to clone the whole thing bio_alloc_bioset() would fail.
-*But the clone should succeed as long as the number of biovecs we
-*actually need to allocate is fewer than BIO_MAX_PAGES.
-*
-* - Lastly, bi_vcnt should not be looked at or relied upon by code
-*that does not own the bio - reason being drivers don't use it for
-*iterating over the biovec anymore, so expecting it to be kept up
-*to date (i.e. for clones that share the parent biovec) is just
-*asking for trouble and would force extra work on
-*__bio_clone_fast() anyways.
-*/
-
- bio = bio_alloc_bioset(gfp_mask, bio_segments(bio_src), bs);
- if (!bio)
- return NULL;
- bio->bi_disk= bio_src->bi_disk;
- bio->bi_opf = bio_src->bi_opf;
- bio->bi_write_hint = bio_src->bi_write_hint;
- bio->bi_iter.bi_sector = bio_src->bi_iter.bi_sector;
- bio->bi_iter.bi_size= bio_src->bi_iter.bi_size;
-
- switch (bio_op(bio)) {
- case REQ_OP_DISCARD:
- case REQ_OP_SECURE_ERASE:
- case REQ_OP_WRITE_ZEROES:
- break;
- case REQ_OP_WRITE_SAME:
- bio->bi_io_vec[bio->bi_vcnt++] = bio_src->bi_io_vec[0];
- break;
- default:
- bio_for_each_segment(bv, bio_src, iter)
- bio->bi_io_vec[bio->bi_vcnt++] = bv;
- break;
- }
-
- if (bio_integrity(bio_src)) {
- int ret;
-
- ret = bio_integrity_clone(bio, bio_src, gfp_mask);
- if (ret < 0) {
- bio_put(bio);
- return NULL;
- }
- }
-
- bio_clone_blkcg_association(bio, bio_src);
-
- return bio;
-}
-EXPORT_SYMBOL(bio_clone_bioset);
-
/**
* bio_add_pc_page - attempt to add page to bio
* @q: the target queue
diff --git a/block/bounce.c b/block/bounce.c
index fd31347b7836..bc63b3a2d18c 100644
--- a/block/bounce.c
+++ b/block/bounce.c
@@ -195,6 +195,73 @@ static void bounce_end_io_read_isa(struct bio *bio)
__bounce_end_io_read(bio, _page_pool);
}
+static struct bio *bounce_clone_bio(struct bio *bio_src, gfp_t gfp_mask,
+ struct bio_set *bs)
+{
+ struct bvec_iter iter;
+ struct bio_vec bv;
+ struct bio *bio;
+
+ /*
+* Pre immutable biovecs, __bio_clone() used to just do a memcpy from
+* bio_src->bi_io_vec to bio->bi_io_vec.
+*
+* We can't do that anymore, because:
+*
+* - The point of cloning the biovec is to produce a bio with a biovec
+*the caller can modify: bi_idx and bi_bvec_done should be 0.
+*
+* - The original bio could've had more than BIO_MAX_PAGES biovecs; if
+*we tried to clone the whole thing bio_alloc_bioset() would fail.
+*But the clone should succeed as long as the number of biovecs we
+*actually need to allocate is fewer than BIO_MAX_PAGES.
+*
+* - Lastly, bi_vcnt should not be looked at or relied upon by code
+*that does not own the bio - reason being drivers don't use it for
+*iterating over the biovec anymore, so expecting it to be kept up
+*to date (i.e. for clones that share the parent biovec) is just
+
[dm-devel] [PATCH 3/6] exofs: use bio_clone_fast in _write_mirror
The mirroring code never changes the bio data or biovecs. This means
we can reuse the biovec allocation easily instead of duplicating it.
Signed-off-by: Christoph Hellwig
---
fs/exofs/ore.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/exofs/ore.c b/fs/exofs/ore.c
index 1b8b44637e70..5331a15a61f1 100644
--- a/fs/exofs/ore.c
+++ b/fs/exofs/ore.c
@@ -873,8 +873,8 @@ static int _write_mirror(struct ore_io_state *ios, int
cur_comp)
struct bio *bio;
if (per_dev != master_dev) {
- bio = bio_clone_kmalloc(master_dev->bio,
- GFP_KERNEL);
+ bio = bio_clone_fast(master_dev->bio,
+GFP_KERNEL, NULL);
if (unlikely(!bio)) {
ORE_DBGMSG(
"Failed to allocate BIO
size=%u\n",
--
2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [RFC] kill bio_clone_kmalloc and bio_clone_bioset
Hi all, this series removes all but one users of the traditional deep bio clone, and then moves bio_clone_bioset to its only caller so that we get rid of the deep bio cloning in the block layer API. Patch 1 is already in the device mapper queue for 4.18, so we should wait for that to go in before the series is applied. The series is inteded as preparation work for the multi-page biovecs so that it doesn't have to deal with the difference between single page or larger bio vecs for cloning. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 1/6] dm: use bio_split() when splitting out the already processed bio
From: Mike Snitzer
Use of bio_clone_bioset() is inefficient if there is no need to clone
the original bio's bio_vec array. Best to use the bio_clone_fast()
variant. Also, just using bio_advance() is only part of what is needed
to properly setup the clone -- it doesn't account for the various
bio_integrity() related work that also needs to be performed (see
bio_split).
Address both of these issues by switching from bio_clone_bioset() to
bio_split().
Fixes: 18a25da8 ("dm: ensure bio submission follows a depth-first tree walk")
Cc: sta...@vger.kernel.org # 4.15+, requires removal of '&' before
md->queue->bio_split
Reported-by: Christoph Hellwig
Reviewed-by: NeilBrown
Signed-off-by: Mike Snitzer
---
drivers/md/dm.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index e65429a29c06..a3b103e8e3ce 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1606,10 +1606,9 @@ static blk_qc_t __split_and_process_bio(struct
mapped_device *md,
* the usage of io->orig_bio in
dm_remap_zone_report()
* won't be affected by this reassignment.
*/
- struct bio *b = bio_clone_bioset(bio, GFP_NOIO,
-
>queue->bio_split);
+ struct bio *b = bio_split(bio, bio_sectors(bio)
- ci.sector_count,
+ GFP_NOIO,
>queue->bio_split);
ci.io->orig_bio = b;
- bio_advance(bio, (bio_sectors(bio) -
ci.sector_count) << 9);
bio_chain(b, bio);
ret = generic_make_request(bio);
break;
--
2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 5/6] md: remove a bogus comment
The function name mentioned doesn't exist, and the code next to it
doesn't match the description either.
Signed-off-by: Christoph Hellwig
---
drivers/md/md.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 29b0cd9ec951..81f458514ac0 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -204,10 +204,6 @@ static int start_readonly;
*/
static bool create_on_open = true;
-/* bio_clone_mddev
- * like bio_clone_bioset, but with a local bio set
- */
-
struct bio *bio_alloc_mddev(gfp_t gfp_mask, int nr_iovecs,
struct mddev *mddev)
{
--
2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 2/6] bcache: don't clone bio in bch_data_verify
We immediately overwrite the biovec array, so instead just allocate
a new bio and copy over the disk, setor and size.
Signed-off-by: Christoph Hellwig
---
drivers/md/bcache/debug.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/md/bcache/debug.c b/drivers/md/bcache/debug.c
index d030ce3025a6..04d146711950 100644
--- a/drivers/md/bcache/debug.c
+++ b/drivers/md/bcache/debug.c
@@ -110,11 +110,15 @@ void bch_data_verify(struct cached_dev *dc, struct bio
*bio)
struct bio_vec bv, cbv;
struct bvec_iter iter, citer = { 0 };
- check = bio_clone_kmalloc(bio, GFP_NOIO);
+ check = bio_kmalloc(GFP_NOIO, bio_segments(bio));
if (!check)
return;
+ check->bi_disk = bio->bi_disk;
check->bi_opf = REQ_OP_READ;
+ check->bi_iter.bi_sector = bio->bi_iter.bi_sector;
+ check->bi_iter.bi_size = bio->bi_iter.bi_size;
+ bch_bio_map(check, NULL);
if (bch_bio_alloc_pages(check, GFP_NOIO))
goto out_put;
--
2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 4/6] block: remove bio_clone_kmalloc
Unused now.
Signed-off-by: Christoph Hellwig
---
include/linux/bio.h | 6 --
1 file changed, 6 deletions(-)
diff --git a/include/linux/bio.h b/include/linux/bio.h
index f08f5fe7bd08..430807f9f44b 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -443,12 +443,6 @@ static inline struct bio *bio_kmalloc(gfp_t gfp_mask,
unsigned int nr_iovecs)
return bio_alloc_bioset(gfp_mask, nr_iovecs, NULL);
}
-static inline struct bio *bio_clone_kmalloc(struct bio *bio, gfp_t gfp_mask)
-{
- return bio_clone_bioset(bio, gfp_mask, NULL);
-
-}
-
extern blk_qc_t submit_bio(struct bio *);
extern void bio_endio(struct bio *);
--
2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v3 3/5] bitmap: Add bitmap_alloc(), bitmap_zalloc() and bitmap_free()
On Mon, 18 Jun 2018 15:01:43 -0700 Dmitry Torokhov
wrote:
> > > +unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags)
> > > +{
> > > + return kmalloc_array(BITS_TO_LONGS(nbits), sizeof(unsigned long),
> > > flags);
> > > +}
> > > +EXPORT_SYMBOL(bitmap_alloc);
> > > +
> > > +unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags)
> > > +{
> > > + return bitmap_alloc(nbits, flags | __GFP_ZERO);
> > > +}
> > > +EXPORT_SYMBOL(bitmap_zalloc);
> > > +
> > > +void bitmap_free(const unsigned long *bitmap)
> > > +{
> > > + kfree(bitmap);
> > > +}
> > > +EXPORT_SYMBOL(bitmap_free);
> > > +
> >
> > I suggest these functions are small and simple enough to justify
> > inlining them.
> >
>
> We can't as we end up including bitmap.h (by the way of cpumask.h)
> form slab.h, so we gen circular dependency.
That info should have been in the changelog, and probably a code
comment.
> Maybe if we removed memcg
> stuff from slab.h so we do not need to include workqueue.h...
Or move the basic slab API stuff out of slab.h into a new header. Or
create a new, standalone work_struct.h - that looks pretty simple.
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] dm bufio: Reduce dm_bufio_lock contention
On Fri, 15 Jun 2018, Michal Hocko wrote: > On Fri 15-06-18 08:47:52, Mikulas Patocka wrote: > > > > > > On Fri, 15 Jun 2018, Michal Hocko wrote: > > > > > On Fri 15-06-18 07:35:07, Mikulas Patocka wrote: > > > > > > > > Because mempool uses it. Mempool uses allocations with "GFP_NOIO | > > > > __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN". An so dm-bufio uses > > > > these flags too. dm-bufio is just a big mempool. > > > > > > This doesn't answer my question though. Somebody else is doing it is not > > > an explanation. Prior to your 41c73a49df31 there was no GFP_NOIO > > > allocation AFAICS. So why do you really need it now? Why cannot you > > > > dm-bufio always used "GFP_NOIO | __GFP_NORETRY | __GFP_NOMEMALLOC | > > __GFP_NOWARN" since the kernel 3.2 when it was introduced. > > > > In the kernel 4.10, dm-bufio was changed so that it does GFP_NOWAIT > > allocation, then drops the lock and does GFP_NOIO with the dropped lock > > (because someone was likely experiencing the same issue that is reported > > in this thread) - there are two commits that change it - 9ea61cac0 and > > 41c73a49df31. > > OK, I see. Is there any fundamental reason why this path has to do one > round of GFP_IO or it can keep NOWAIT, drop the lock, sleep and retry > again? If the process is woken up, there was some buffer added to the freelist, or refcount of some buffer was dropped to 0. In this case, we don't want to drop the lock and use GFP_NOIO, because the freed buffer may disappear when we drop the lock. > [...] > > > is the same class of problem, honestly, I dunno. And I've already said > > > that stalling __GFP_NORETRY might be a good way around that but that > > > needs much more consideration and existing users examination. I am not > > > aware anybody has done that. Doing changes like that based on a single > > > user is certainly risky. > > > > Why don't you set any rules how these flags should be used? > > It is really hard to change rules during the game. You basically have to > examine all existing users and that is well beyond my time scope. I've > tried that where it was possible. E.g. __GFP_REPEAT and turned it into a > well defined semantic. __GFP_NORETRY is a bigger beast. > > Anyway, I believe that it would be much safer to look at the problem > from a highlevel perspective. You seem to be focused on __GFP_NORETRY > little bit too much IMHO. We are not throttling callers which explicitly > do not want to or cannot - see current_may_throttle. Is it possible that > both your md and mempool allocators can either (ab)use PF_LESS_THROTTLE > or use other means? E.g. do you have backing_dev_info at that time? > -- > Michal Hocko > SUSE Labs I grepped the kernel for __GFP_NORETRY and triaged them. I found 16 cases without a fallback - those are bugs that make various functions randomly return -ENOMEM. Most of the callers provide callback. There is another strange flag - __GFP_RETRY_MAYFAIL - it provides two different functions - if the allocation is larger than PAGE_ALLOC_COSTLY_ORDER, it retries the allocation as if it were smaller. If the allocations is smaller than PAGE_ALLOC_COSTLY_ORDER, __GFP_RETRY_MAYFAIL will avoid the oom killer (larger order allocations don't trigger the oom killer at all). So, perhaps __GFP_RETRY_MAYFAIL could be used instead of __GFP_NORETRY in the cases where the caller wants to avoid trigerring the oom killer (the problem is that __GFP_NORETRY causes random failure even in no-oom situations but __GFP_RETRY_MAYFAIL doesn't). So my suggestion is - fix these obvious bugs when someone allocates memory with __GFP_NORETRY without any fallback - and then, __GFP_NORETRY could be just changed to return NULL instead of sleeping. arch/arm/mm/dma-mapping.c - fallback to a smaller size without __GFP_NORETRY arch/mips/mm/dma-default.c - says that it uses __GFP_NORETRY to avoid the oom killer, provides no fallback - it seems to be a BUG arch/sparc/mm/tsb.c - fallback to a smaller size without __GFP_NORETRY arch/x86/include/asm/floppy.h - __GFP_NORETRY doesn't seem to serve any purpose, it may cause random failures during initialization, can be removed - BUG arch/powerpc/mm/mmu_context_iommu.c - uses it just during moving pages, there's no problem with failure arch/powerpc/platforms/pseries/cmm.c - a vm balloon driver, no problem with failure block/bio.c - falls back to mempool block/blk-mq.c - errorneous use of __GFP_NORETRY during initialization, it falls back to a smaller size, but doesn't drop the __GFP_NORETRY flag (BUG) drivers/gpu/drm/i915/i915_gem.c - it starts with __GFP_NORETRY and on failure, it ORs it with __GFP_RETRY_MAYFAIL (which of these conflicting flags wins?) drivers/gpu/drm/i915/i915_gem_gtt.c - __GFP_NORETRY is used during initialization (BUG), it shouldn't be used drivers/gpu/drm/i915/i915_gem_execbuffer.c - fallback to a smaller size without __GFP_NORETRY drivers/gpu/drm/i915/i915_gem_internal.c - fallback
Re: [dm-devel] [PATCH] dm: writecache: Use 2-factor allocator arguments
On Mon, Jun 18, 2018 at 2:12 PM, Mikulas Patocka wrote:
>
>
> On Mon, 18 Jun 2018, Kees Cook wrote:
>
>> This adjusts the allocator calls to use the 2-factor argument style, as
>> already done treewide for better defense against allocator overflows.
>> Additionally adjusts style nit to avoid assignments in test expressions.
>>
>> Signed-off-by: Kees Cook
>> ---
>> drivers/md/dm-writecache.c | 16 ++--
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
>> index 5961c7794ef3..7773f4c75701 100644
>> --- a/drivers/md/dm-writecache.c
>> +++ b/drivers/md/dm-writecache.c
>> @@ -259,7 +259,7 @@ static int persistent_memory_claim(struct dm_writecache
>> *wc)
>> if (da != p) {
>> long i;
>> wc->memory_map = NULL;
>> - pages = kvmalloc(p * sizeof(struct page *), GFP_KERNEL);
>> + pages = kvmalloc_array(p, sizeof(struct page *), GFP_KERNEL);
>> if (!pages) {
>> r = -ENOMEM;
>> goto err2;
>> @@ -859,7 +859,8 @@ static int writecache_alloc_entries(struct dm_writecache
>> *wc)
>>
>> if (wc->entries)
>> return 0;
>> - wc->entries = vmalloc(sizeof(struct wc_entry) * wc->n_blocks);
>> + wc->entries = vmalloc(array_size(sizeof(struct wc_entry),
>> + wc->n_blocks));
>> if (!wc->entries)
>> return -ENOMEM;
>> for (b = 0; b < wc->n_blocks; b++) {
>> @@ -1480,10 +1481,13 @@ static void __writecache_writeback_pmem(struct
>> dm_writecache *wc, struct writeba
>> bio_set_dev(>bio, wc->dev->bdev);
>> wb->bio.bi_iter.bi_sector = read_original_sector(wc, e);
>> wb->page_offset = PAGE_SIZE;
>> - if (max_pages <= WB_LIST_INLINE ||
>> - unlikely(!(wb->wc_list = kmalloc(max_pages * sizeof(struct
>> wc_entry *),
>> - GFP_NOIO | __GFP_NORETRY |
>> - __GFP_NOMEMALLOC |
>> __GFP_NOWARN {
>> + if (max_pages > WB_LIST_INLINE)
>> + wb->wc_list = kmalloc_array(max_pages,
>> + sizeof(struct wc_entry *),
>> + GFP_NOIO | __GFP_NORETRY |
>> + __GFP_NOMEMALLOC |
>> + __GFP_NOWARN);
>> + if (max_pages <= WB_LIST_INLINE || !wb->wc_list) {
>
> The rest of patch is OK - but you shouldn't duplicate the comparison
> against WB_LIST_INLINE.
I couldn't find a better way to avoid an assignment in a test... open
to suggestions! :)
-Kees
--
Kees Cook
Pixel Security
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 00/28] multipath-tools: improve config file handling
On Fri, Jun 08, 2018 at 12:20:13PM +0200, Martin Wilck wrote:
Thanks for cleaning this up.
ACK for the whole series 00-30
-Ben
> This patch series fixes a number of small but important issues in
> the way multipath-tools handle configuration files, in particular the
> handling of multiple "device" and "blacklist" sections matching the
> same device. Besides eliminating inconsistencies, the goal of the series is
> to be able to use "multipath -t" or "multipathd show config" ouput as
> configuration for multipath-tools, and obtain exactly the same results
> as with the original configuration.
>
> Late in the series, two new commands "multipath -T" and "multipath show config
> local" are added, which avoid the lengthyness of the full configuration
> dump and produce output more suitable as a local configuration template.
>
> By far the largest part of the series is a new unit test (hwtable.c) for
> configuration file handling. The commit message of the patch introducing
> this unit test ("tests/hwtable: tests for config file handling and hwentry
> merging") explains the problems / inconsistencies that the series is supposed
> to fix. Tests that fail in the first place are marked with the BROKEN macro.
> While reviewing the 2000 LOC of the new unit test is certainly a pain, I'd
> like to ask reviewers to *pay special attention to the tests marked BROKEN*,
> because that's where follow-up patches will change the behavior of
> multipath-tools in user-noticeable ways. I believe it's for the better, but
> I'd like to make sure we all agree. For the rest of the unit test, reviewers
> might appreciate the fact that tests succeed for the current code, and are
> intended as regeression tests for the follow-up patches.
>
> Patch 01-07 are just simple fixes and, as usual, "const" additions.
> Patch 08-10 introduce the new unit test.
> Patch 11-17 fix some basic problems which need to be fixed for dump/reload.
> Patch 18-24 add the ability for dumping the configuration, using the output
> as new configuration input ("reload"), and testing / comparing the results.
> Patch 23-24 implement "multipathd show config local" and "multipath -T" on
> these
> grounds.
> Patch 23-27 fix more problems that surfaced in the dump/reload test.
> Patch 28 clarifies the documentation of multipath.conf.
>
> Martin Wilck (28):
> kpartx: no need to use FREE_CONST
> libmultipath: fix memory leak in process_config_dir()
> libmultipath: remove superfluous conditionals in load_config()
> libmultipath/structs.c: constify some functions
> libmultipath: some const usage in hwentry handling
> libmultipath: change prototypes of hwe_regmatch() and find_hwe()
> libmultipath/prio: constify simple getters
> tests/Makefile: autogenerate list of symbols to be wrapped
> tests/test-lib: cmocka helpers to simulate path and map discovery
> tests/hwtable: tests for config file handling and hwentry merging
> libmultipath: add debug messages to hwentry lookup/merging code
> libmultipath: use vector for for pp->hwe and mp->hwe
> libmultipath: allow more than one hwentry
> libmultipath: don't merge hwentries by regex
> libmultipath: merge hwentries inside a conf file
> libmultipath/hwtable: remove inherited props from ONTAP NVMe
> libmultipath: don't merge by regex in setup_default_blist()
> multipath, multipathd: consolidate config dumping
> tests/hwtable: implement configuration dump + reload test
> libmultipath: allow dumping only "local" hwtable in snprint_config
> tests/hwtable: add test for local configuration dump
> libmultipath: allow printing local maps in snprint_config
> multipathd: implement "show config local"
> multipath: implement "multipath -T"
> libmultipath: merge "multipath" config sections by wwid
> libmultipath: implement and use blacklist merging
> libmultipath: escape '"' chars while dumping config
> multipath.conf(5): various corrections and clarifications
>
> kpartx/devmapper.c | 10 +-
> libmultipath/blacklist.c | 125 ++-
> libmultipath/blacklist.h |2 +
> libmultipath/config.c | 208 +++--
> libmultipath/config.h |5 +-
> libmultipath/dict.c| 40 +-
> libmultipath/discovery.c | 10 +-
> libmultipath/hwtable.c |3 -
> libmultipath/print.c | 128 ++-
> libmultipath/print.h |9 +-
> libmultipath/prio.c|8 +-
> libmultipath/prio.h|8 +-
> libmultipath/propsel.c | 53 +-
> libmultipath/structs.c | 26 +-
> libmultipath/structs.h | 24 +-
> libmultipath/structs_vec.c | 19 +
> libmultipath/structs_vec.h |1 +
> libmultipath/vector.c | 12 +
> libmultipath/vector.h |1 +
> multipath/main.c | 92 +-
> multipath/multipath.8 |8 +-
> multipath/multipath.conf.5 | 174 ++--
> multipathd/cli.c |2 +
> multipathd/cli.h |2 +
> multipathd/cli_handlers.c | 80 +-
> multipathd/cli_handlers.h |1 +
Re: [dm-devel] [PATCH v3 3/5] bitmap: Add bitmap_alloc(), bitmap_zalloc() and bitmap_free()
On Mon, 18 Jun 2018 16:10:01 +0300 Andy Shevchenko
wrote:
> A lot of code become ugly because of open coding allocations for bitmaps.
>
> Introduce three helpers to allow users be more clear of intention
> and keep their code neat.
>
> ...
>
> +unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags)
> +{
> + return kmalloc_array(BITS_TO_LONGS(nbits), sizeof(unsigned long),
> flags);
> +}
> +EXPORT_SYMBOL(bitmap_alloc);
> +
> +unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags)
> +{
> + return bitmap_alloc(nbits, flags | __GFP_ZERO);
> +}
> +EXPORT_SYMBOL(bitmap_zalloc);
> +
> +void bitmap_free(const unsigned long *bitmap)
> +{
> + kfree(bitmap);
> +}
> +EXPORT_SYMBOL(bitmap_free);
> +
I suggest these functions are small and simple enough to justify
inlining them.
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] dm: writecache: Use 2-factor allocator arguments
On Mon, 18 Jun 2018, Kees Cook wrote:
> This adjusts the allocator calls to use the 2-factor argument style, as
> already done treewide for better defense against allocator overflows.
> Additionally adjusts style nit to avoid assignments in test expressions.
>
> Signed-off-by: Kees Cook
> ---
> drivers/md/dm-writecache.c | 16 ++--
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
> index 5961c7794ef3..7773f4c75701 100644
> --- a/drivers/md/dm-writecache.c
> +++ b/drivers/md/dm-writecache.c
> @@ -259,7 +259,7 @@ static int persistent_memory_claim(struct dm_writecache
> *wc)
> if (da != p) {
> long i;
> wc->memory_map = NULL;
> - pages = kvmalloc(p * sizeof(struct page *), GFP_KERNEL);
> + pages = kvmalloc_array(p, sizeof(struct page *), GFP_KERNEL);
> if (!pages) {
> r = -ENOMEM;
> goto err2;
> @@ -859,7 +859,8 @@ static int writecache_alloc_entries(struct dm_writecache
> *wc)
>
> if (wc->entries)
> return 0;
> - wc->entries = vmalloc(sizeof(struct wc_entry) * wc->n_blocks);
> + wc->entries = vmalloc(array_size(sizeof(struct wc_entry),
> + wc->n_blocks));
> if (!wc->entries)
> return -ENOMEM;
> for (b = 0; b < wc->n_blocks; b++) {
> @@ -1480,10 +1481,13 @@ static void __writecache_writeback_pmem(struct
> dm_writecache *wc, struct writeba
> bio_set_dev(>bio, wc->dev->bdev);
> wb->bio.bi_iter.bi_sector = read_original_sector(wc, e);
> wb->page_offset = PAGE_SIZE;
> - if (max_pages <= WB_LIST_INLINE ||
> - unlikely(!(wb->wc_list = kmalloc(max_pages * sizeof(struct
> wc_entry *),
> - GFP_NOIO | __GFP_NORETRY |
> - __GFP_NOMEMALLOC |
> __GFP_NOWARN {
> + if (max_pages > WB_LIST_INLINE)
> + wb->wc_list = kmalloc_array(max_pages,
> + sizeof(struct wc_entry *),
> + GFP_NOIO | __GFP_NORETRY |
> + __GFP_NOMEMALLOC |
> + __GFP_NOWARN);
> + if (max_pages <= WB_LIST_INLINE || !wb->wc_list) {
The rest of patch is OK - but you shouldn't duplicate the comparison
against WB_LIST_INLINE.
Mikulas
> wb->wc_list = wb->wc_list_inline;
> max_pages = WB_LIST_INLINE;
> }
> --
> 2.17.0
>
>
> --
> Kees Cook
> Pixel Security
>
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 5/5] Input: evdev - Switch to bitmap_zalloc()
On Mon, 2018-06-18 at 22:56 +0300, Andy Shevchenko wrote: > On Mon, Jun 18, 2018 at 6:49 PM, Joe Perches wrote: > > Perhaps bitmap_dup_user [or some better name] could or should > > be one of the helpers. > > Can you help with estimation how many existing users need this kind of > functionality? One of them evdev, which has an open coded variant. My estimation via cocci script below is 1 existing user, so it's almost certainly not worthwhile. $ cat copy_from_user.cocci @@ unsigned long *p; expression e1, e2; @@ * copy_from_user(p, e1, e2) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] dm: writecache: Use 2-factor allocator arguments
This adjusts the allocator calls to use the 2-factor argument style, as
already done treewide for better defense against allocator overflows.
Additionally adjusts style nit to avoid assignments in test expressions.
Signed-off-by: Kees Cook
---
drivers/md/dm-writecache.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
index 5961c7794ef3..7773f4c75701 100644
--- a/drivers/md/dm-writecache.c
+++ b/drivers/md/dm-writecache.c
@@ -259,7 +259,7 @@ static int persistent_memory_claim(struct dm_writecache *wc)
if (da != p) {
long i;
wc->memory_map = NULL;
- pages = kvmalloc(p * sizeof(struct page *), GFP_KERNEL);
+ pages = kvmalloc_array(p, sizeof(struct page *), GFP_KERNEL);
if (!pages) {
r = -ENOMEM;
goto err2;
@@ -859,7 +859,8 @@ static int writecache_alloc_entries(struct dm_writecache
*wc)
if (wc->entries)
return 0;
- wc->entries = vmalloc(sizeof(struct wc_entry) * wc->n_blocks);
+ wc->entries = vmalloc(array_size(sizeof(struct wc_entry),
+wc->n_blocks));
if (!wc->entries)
return -ENOMEM;
for (b = 0; b < wc->n_blocks; b++) {
@@ -1480,10 +1481,13 @@ static void __writecache_writeback_pmem(struct
dm_writecache *wc, struct writeba
bio_set_dev(>bio, wc->dev->bdev);
wb->bio.bi_iter.bi_sector = read_original_sector(wc, e);
wb->page_offset = PAGE_SIZE;
- if (max_pages <= WB_LIST_INLINE ||
- unlikely(!(wb->wc_list = kmalloc(max_pages * sizeof(struct
wc_entry *),
-GFP_NOIO | __GFP_NORETRY |
-__GFP_NOMEMALLOC |
__GFP_NOWARN {
+ if (max_pages > WB_LIST_INLINE)
+ wb->wc_list = kmalloc_array(max_pages,
+ sizeof(struct wc_entry *),
+ GFP_NOIO | __GFP_NORETRY |
+ __GFP_NOMEMALLOC |
+ __GFP_NOWARN);
+ if (max_pages <= WB_LIST_INLINE || !wb->wc_list) {
wb->wc_list = wb->wc_list_inline;
max_pages = WB_LIST_INLINE;
}
--
2.17.0
--
Kees Cook
Pixel Security
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] x86: optimize memcpy_flushcache
On Mon, Jun 18, 2018 at 5:50 AM, Mikulas Patocka wrote:
> Hi Mike
>
> Could you please push this patch to the kernel 4.18-rc? Dan Williams said
> that he will submit it, but he forgot about it.
...to be clear I acked it and asked Ingo to take it. Will need a
resubmit for 4.19.
Ingo, see below for a patch to pick up into -tip when you have a chance.
>
> Without this patch, dm-writecache is suffering 2% penalty because of
> memcpy_flushcache overhead.
>
> Mikulas
>
>
>
> From: Mikulas Patocka
>
> I use memcpy_flushcache in my persistent memory driver for metadata
> updates and it turns out that the overhead of memcpy_flushcache causes 2%
> performance degradation compared to "movnti" instruction explicitly coded
> using inline assembler.
>
> This patch recognizes memcpy_flushcache calls with constant short length
> and turns them into inline assembler - so that I don't have to use inline
> assembler in the driver.
>
> Signed-off-by: Mikulas Patocka
>
> ---
> arch/x86/include/asm/string_64.h | 20 +++-
> arch/x86/lib/usercopy_64.c |4 ++--
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> Index: linux-2.6/arch/x86/include/asm/string_64.h
> ===
> --- linux-2.6.orig/arch/x86/include/asm/string_64.h
> +++ linux-2.6/arch/x86/include/asm/string_64.h
> @@ -149,7 +149,25 @@ memcpy_mcsafe(void *dst, const void *src
>
> #ifdef CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE
> #define __HAVE_ARCH_MEMCPY_FLUSHCACHE 1
> -void memcpy_flushcache(void *dst, const void *src, size_t cnt);
> +void __memcpy_flushcache(void *dst, const void *src, size_t cnt);
> +static __always_inline void memcpy_flushcache(void *dst, const void *src,
> size_t cnt)
> +{
> + if (__builtin_constant_p(cnt)) {
> + switch (cnt) {
> + case 4:
> + asm ("movntil %1, %0" : "=m"(*(u32 *)dst) :
> "r"(*(u32 *)src));
> + return;
> + case 8:
> + asm ("movntiq %1, %0" : "=m"(*(u64 *)dst) :
> "r"(*(u64 *)src));
> + return;
> + case 16:
> + asm ("movntiq %1, %0" : "=m"(*(u64 *)dst) :
> "r"(*(u64 *)src));
> + asm ("movntiq %1, %0" : "=m"(*(u64 *)(dst +
> 8)) : "r"(*(u64 *)(src + 8)));
> + return;
> + }
> + }
> + __memcpy_flushcache(dst, src, cnt);
> +}
> #endif
>
> #endif /* __KERNEL__ */
> Index: linux-2.6/arch/x86/lib/usercopy_64.c
> ===
> --- linux-2.6.orig/arch/x86/lib/usercopy_64.c
> +++ linux-2.6/arch/x86/lib/usercopy_64.c
> @@ -153,7 +153,7 @@ long __copy_user_flushcache(void *dst, c
> return rc;
> }
>
> -void memcpy_flushcache(void *_dst, const void *_src, size_t size)
> +void __memcpy_flushcache(void *_dst, const void *_src, size_t size)
> {
> unsigned long dest = (unsigned long) _dst;
> unsigned long source = (unsigned long) _src;
> @@ -216,7 +216,7 @@ void memcpy_flushcache(void *_dst, const
> clean_cache_range((void *) dest, size);
> }
> }
> -EXPORT_SYMBOL_GPL(memcpy_flushcache);
> +EXPORT_SYMBOL_GPL(__memcpy_flushcache);
>
> void memcpy_page_flushcache(char *to, struct page *page, size_t offset,
> size_t len)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 5/5] Input: evdev - Switch to bitmap_zalloc()
On Mon, 2018-06-18 at 15:02 +0300, Andy Shevchenko wrote: > On Sat, 2018-06-16 at 12:16 -0700, Joe Perches wrote: > > On Sat, 2018-06-16 at 21:45 +0300, Andy Shevchenko wrote: > > > On Sat, Jun 16, 2018 at 12:46 AM Yury Norov > > om> wrote: > > > > On Fri, Jun 15, 2018 at 04:20:17PM +0300, Andy Shevchenko wrote: > > > > > Switch to bitmap_zalloc() to show clearly what we are > > > > > allocating. > > > > > Besides that it returns pointer of bitmap type instead of opaque > > > > > void *. > > > > > > > > > > > + mem = bitmap_alloc(maxbit, GFP_KERNEL); > > > > > if (!mem) > > > > > return -ENOMEM; > > > > > > > > But in commit message you say you switch to bitmap_zalloc(). IIUC > > > > bitmap_alloc() is OK here. But could you please update comment to > > > > avoid confusing. > > > > > > There are two places, one with alloc, another with zalloc. > > > I will clarify this in commit message of next version. > > > > > > > > + mask = bitmap_zalloc(cnt, GFP_KERNEL); > > > > > if (!mask) > > > > > return -ENOMEM; > > > > > > > > > > error = bits_from_user(mask, cnt - 1, codes_size, codes, > > > > > compat); > > > > > > > > If my understanding of bits_from_user() correct, here you can also > > > > use > > > > bitmap_alloc(), true? > > > > Also it might be useful to have a separate bitmap_from_user > > to alloc and copy. > > Maybe. I didn't check if there are such users except this driver. > > Anyway, it's out of scope of the series. That seems incorrect as you are introducing alloc/free helpers. Perhaps bitmap_dup_user [or some better name] could or should be one of the helpers. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v2 RESEND] x86: optimize memcpy_flushcache
From: Mikulas Patocka
Subject: [PATCH v2] x86: optimize memcpy_flushcache
In the context of constant short length stores to persistent memory,
memcpy_flushcache suffers from a 2% performance degradation compared to
explicitly using the "movnti" instruction.
Optimize 4, 8, and 16 byte memcpy_flushcache calls to explicitly use the
movnti instruction with inline assembler.
Signed-off-by: Mikulas Patocka
Reviewed-by: Dan Williams
Signed-off-by: Mike Snitzer
---
arch/x86/include/asm/string_64.h | 28 +++-
arch/x86/lib/usercopy_64.c | 4 ++--
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/string_64.h
index 533f74c300c2..aaba83478cdc 100644
--- a/arch/x86/include/asm/string_64.h
+++ b/arch/x86/include/asm/string_64.h
@@ -147,7 +147,33 @@ memcpy_mcsafe(void *dst, const void *src, size_t cnt)
#ifdef CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE
#define __HAVE_ARCH_MEMCPY_FLUSHCACHE 1
-void memcpy_flushcache(void *dst, const void *src, size_t cnt);
+void __memcpy_flushcache(void *dst, const void *src, size_t cnt);
+static __always_inline void memcpy_flushcache(void *dst, const void *src,
size_t cnt)
+{
+ if (__builtin_constant_p(cnt)) {
+ switch (cnt) {
+ case 4:
+ asm volatile("movntil %1, %0"
+: "=m" (*(u32 *)dst)
+: "r" (*(u32 *)src));
+ return;
+ case 8:
+ asm volatile("movntiq %1, %0"
+: "=m" (*(u64 *)dst)
+: "r" (*(u64 *)src));
+ return;
+ case 16:
+ asm volatile("movntiq %1, %0"
+: "=m" (*(u64 *)dst)
+: "r" (*(u64 *)src));
+ asm volatile("movntiq %1, %0"
+: "=m" (*(u64 *)(dst + 8))
+: "r" (*(u64 *)(src + 8)));
+ return;
+ }
+ }
+ __memcpy_flushcache(dst, src, cnt);
+}
#endif
#endif /* __KERNEL__ */
diff --git a/arch/x86/lib/usercopy_64.c b/arch/x86/lib/usercopy_64.c
index 75d3776123cc..26f515aa3529 100644
--- a/arch/x86/lib/usercopy_64.c
+++ b/arch/x86/lib/usercopy_64.c
@@ -133,7 +133,7 @@ long __copy_user_flushcache(void *dst, const void __user
*src, unsigned size)
return rc;
}
-void memcpy_flushcache(void *_dst, const void *_src, size_t size)
+void __memcpy_flushcache(void *_dst, const void *_src, size_t size)
{
unsigned long dest = (unsigned long) _dst;
unsigned long source = (unsigned long) _src;
@@ -196,7 +196,7 @@ void memcpy_flushcache(void *_dst, const void *_src, size_t
size)
clean_cache_range((void *) dest, size);
}
}
-EXPORT_SYMBOL_GPL(memcpy_flushcache);
+EXPORT_SYMBOL_GPL(__memcpy_flushcache);
void memcpy_page_flushcache(char *to, struct page *page, size_t offset,
size_t len)
--
2.15.0
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] x86: optimize memcpy_flushcache
On Mon, Jun 18 2018 at 8:50am -0400,
Mikulas Patocka wrote:
> Hi Mike
>
> Could you please push this patch to the kernel 4.18-rc? Dan Williams said
> that he will submit it, but he forgot about it.
>
> Without this patch, dm-writecache is suffering 2% penalty because of
> memcpy_flushcache overhead.
I cannot send this to Linus directly, it needs to go through the x86
tree.
I already tried to get a slightly revised version of this upstream, see:
https://www.redhat.com/archives/dm-devel/2018-May/msg00080.html
I'll try a resend.. but the 4.18 merge window is now closed.
Mike
> From: Mikulas Patocka
>
> I use memcpy_flushcache in my persistent memory driver for metadata
> updates and it turns out that the overhead of memcpy_flushcache causes 2%
> performance degradation compared to "movnti" instruction explicitly coded
> using inline assembler.
>
> This patch recognizes memcpy_flushcache calls with constant short length
> and turns them into inline assembler - so that I don't have to use inline
> assembler in the driver.
>
> Signed-off-by: Mikulas Patocka
>
> ---
> arch/x86/include/asm/string_64.h | 20 +++-
> arch/x86/lib/usercopy_64.c |4 ++--
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> Index: linux-2.6/arch/x86/include/asm/string_64.h
> ===
> --- linux-2.6.orig/arch/x86/include/asm/string_64.h
> +++ linux-2.6/arch/x86/include/asm/string_64.h
> @@ -149,7 +149,25 @@ memcpy_mcsafe(void *dst, const void *src
>
> #ifdef CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE
> #define __HAVE_ARCH_MEMCPY_FLUSHCACHE 1
> -void memcpy_flushcache(void *dst, const void *src, size_t cnt);
> +void __memcpy_flushcache(void *dst, const void *src, size_t cnt);
> +static __always_inline void memcpy_flushcache(void *dst, const void *src,
> size_t cnt)
> +{
> + if (__builtin_constant_p(cnt)) {
> + switch (cnt) {
> + case 4:
> + asm ("movntil %1, %0" : "=m"(*(u32 *)dst) :
> "r"(*(u32 *)src));
> + return;
> + case 8:
> + asm ("movntiq %1, %0" : "=m"(*(u64 *)dst) :
> "r"(*(u64 *)src));
> + return;
> + case 16:
> + asm ("movntiq %1, %0" : "=m"(*(u64 *)dst) :
> "r"(*(u64 *)src));
> + asm ("movntiq %1, %0" : "=m"(*(u64 *)(dst + 8))
> : "r"(*(u64 *)(src + 8)));
> + return;
> + }
> + }
> + __memcpy_flushcache(dst, src, cnt);
> +}
> #endif
>
> #endif /* __KERNEL__ */
> Index: linux-2.6/arch/x86/lib/usercopy_64.c
> ===
> --- linux-2.6.orig/arch/x86/lib/usercopy_64.c
> +++ linux-2.6/arch/x86/lib/usercopy_64.c
> @@ -153,7 +153,7 @@ long __copy_user_flushcache(void *dst, c
> return rc;
> }
>
> -void memcpy_flushcache(void *_dst, const void *_src, size_t size)
> +void __memcpy_flushcache(void *_dst, const void *_src, size_t size)
> {
> unsigned long dest = (unsigned long) _dst;
> unsigned long source = (unsigned long) _src;
> @@ -216,7 +216,7 @@ void memcpy_flushcache(void *_dst, const
> clean_cache_range((void *) dest, size);
> }
> }
> -EXPORT_SYMBOL_GPL(memcpy_flushcache);
> +EXPORT_SYMBOL_GPL(__memcpy_flushcache);
>
> void memcpy_page_flushcache(char *to, struct page *page, size_t offset,
> size_t len)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] x86: optimize memcpy_flushcache
Hi Mike
Could you please push this patch to the kernel 4.18-rc? Dan Williams said
that he will submit it, but he forgot about it.
Without this patch, dm-writecache is suffering 2% penalty because of
memcpy_flushcache overhead.
Mikulas
From: Mikulas Patocka
I use memcpy_flushcache in my persistent memory driver for metadata
updates and it turns out that the overhead of memcpy_flushcache causes 2%
performance degradation compared to "movnti" instruction explicitly coded
using inline assembler.
This patch recognizes memcpy_flushcache calls with constant short length
and turns them into inline assembler - so that I don't have to use inline
assembler in the driver.
Signed-off-by: Mikulas Patocka
---
arch/x86/include/asm/string_64.h | 20 +++-
arch/x86/lib/usercopy_64.c |4 ++--
2 files changed, 21 insertions(+), 3 deletions(-)
Index: linux-2.6/arch/x86/include/asm/string_64.h
===
--- linux-2.6.orig/arch/x86/include/asm/string_64.h
+++ linux-2.6/arch/x86/include/asm/string_64.h
@@ -149,7 +149,25 @@ memcpy_mcsafe(void *dst, const void *src
#ifdef CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE
#define __HAVE_ARCH_MEMCPY_FLUSHCACHE 1
-void memcpy_flushcache(void *dst, const void *src, size_t cnt);
+void __memcpy_flushcache(void *dst, const void *src, size_t cnt);
+static __always_inline void memcpy_flushcache(void *dst, const void *src,
size_t cnt)
+{
+ if (__builtin_constant_p(cnt)) {
+ switch (cnt) {
+ case 4:
+ asm ("movntil %1, %0" : "=m"(*(u32 *)dst) :
"r"(*(u32 *)src));
+ return;
+ case 8:
+ asm ("movntiq %1, %0" : "=m"(*(u64 *)dst) :
"r"(*(u64 *)src));
+ return;
+ case 16:
+ asm ("movntiq %1, %0" : "=m"(*(u64 *)dst) :
"r"(*(u64 *)src));
+ asm ("movntiq %1, %0" : "=m"(*(u64 *)(dst + 8))
: "r"(*(u64 *)(src + 8)));
+ return;
+ }
+ }
+ __memcpy_flushcache(dst, src, cnt);
+}
#endif
#endif /* __KERNEL__ */
Index: linux-2.6/arch/x86/lib/usercopy_64.c
===
--- linux-2.6.orig/arch/x86/lib/usercopy_64.c
+++ linux-2.6/arch/x86/lib/usercopy_64.c
@@ -153,7 +153,7 @@ long __copy_user_flushcache(void *dst, c
return rc;
}
-void memcpy_flushcache(void *_dst, const void *_src, size_t size)
+void __memcpy_flushcache(void *_dst, const void *_src, size_t size)
{
unsigned long dest = (unsigned long) _dst;
unsigned long source = (unsigned long) _src;
@@ -216,7 +216,7 @@ void memcpy_flushcache(void *_dst, const
clean_cache_range((void *) dest, size);
}
}
-EXPORT_SYMBOL_GPL(memcpy_flushcache);
+EXPORT_SYMBOL_GPL(__memcpy_flushcache);
void memcpy_page_flushcache(char *to, struct page *page, size_t offset,
size_t len)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 30/30] fixup "libmultipath: merge hwentries inside a conf file"
The previous patch "libmultipath: merge hwentries inside a conf file"
meant to enable checking for duplicate entries not only between different
"sections" of the configuration (i.e. built-in hwtable, multipath.conf,
config dir files), but also inside every section except the built-in
table. This patch fixes a bug to actually implement this new behavior.
Signed-off-by: Martin Wilck
---
libmultipath/config.c | 3 ---
tests/hwtable.c | 3 +--
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index 28b76dd2..50826a1d 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -527,9 +527,6 @@ factorize_hwtable (vector hw, int n, const char *table_desc)
restart:
vector_foreach_slot(hw, hwe1, i) {
- if (i == n)
- break;
- j = n;
/* drop invalid device configs */
if (i >= n && (!hwe1->vendor || !hwe1->product)) {
condlog(0, "device config in %s missing vendor or
product parameter",
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 3488640d..42127adf 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -661,8 +661,7 @@ static void test_broken_hwe(const struct hwt_state *hwt)
/* foo:bar doesn't match, as hwentry is ignored */
pp = mock_path(vnd_foo.value, prd_bar.value);
- TEST_PROP_BROKEN("prio", prio_name(>prio), prio_emc.value,
-DEFAULT_PRIO);
+ TEST_PROP(prio_name(>prio), DEFAULT_PRIO);
/* boo:bar doesn't match */
pp = mock_path(vnd_boo.value, prd_bar.value);
--
2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 29/30] tests/hwtable: add test for broken hwentry filtering
Broken hwentries weren't correctly filtered after patch
"libmultipath: merge hwentries inside a conf file". Add a test
case for that.
Signed-off-by: Martin Wilck
---
tests/hwtable.c | 50 +
1 file changed, 50 insertions(+)
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 9db79391..3488640d 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -651,6 +651,52 @@ static int setup_string_hwe(void **state)
return 0;
}
+/*
+ * Device section with a broken entry (no product)
+ * It should be ignored.
+ */
+static void test_broken_hwe(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:bar doesn't match, as hwentry is ignored */
+ pp = mock_path(vnd_foo.value, prd_bar.value);
+ TEST_PROP_BROKEN("prio", prio_name(>prio), prio_emc.value,
+DEFAULT_PRIO);
+
+ /* boo:bar doesn't match */
+ pp = mock_path(vnd_boo.value, prd_bar.value);
+ TEST_PROP(prio_name(>prio), DEFAULT_PRIO);
+}
+
+static int setup_broken_hwe(void **state)
+{
+ struct hwt_state *hwt = CHECK_STATE(state);
+ const struct key_value kv[] = { vnd_foo, prio_emc };
+
+ WRITE_ONE_DEVICE(hwt, kv);
+ SET_TEST_FUNC(hwt, test_broken_hwe);
+ return 0;
+}
+
+/*
+ * Like test_broken_hwe, but in config_dir file.
+ */
+static int setup_broken_hwe_dir(void **state)
+{
+ struct hwt_state *hwt = CHECK_STATE(state);
+ const struct key_value kv[] = { vnd_foo, prio_emc };
+
+ begin_config(hwt);
+ begin_section_all(hwt, "devices");
+ write_device(hwt->conf_dir_file[0], ARRAY_SIZE(kv), kv);
+ end_section_all(hwt);
+ finish_config(hwt);
+ hwt->test = test_broken_hwe;
+ hwt->test_name = "test_broken_hwe_dir";
+ return 0;
+}
+
/*
* Device section with a single regex entry ("^.foo:(bar|baz|ba\.)$")
*/
@@ -1628,6 +1674,8 @@ static int setup_multipath_config_3(void **state)
}
define_test(string_hwe)
+define_test(broken_hwe)
+define_test(broken_hwe_dir)
define_test(quoted_hwe)
define_test(internal_nvme)
define_test(regex_hwe)
@@ -1666,6 +1714,8 @@ static int test_hwtable(void)
cmocka_unit_test(test_sanity_globals),
test_entry(internal_nvme),
test_entry(string_hwe),
+ test_entry(broken_hwe),
+ test_entry(broken_hwe_dir),
test_entry(quoted_hwe),
test_entry(regex_hwe),
test_entry(regex_string_hwe),
--
2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 4/5] Input: gpio-keys - Switch to bitmap_zalloc()
On Fri, Jun 15, 2018 at 04:20:16PM +0300, Andy Shevchenko wrote: > External Email > > Switch to bitmap_zalloc() to show clearly what we are allocating. > Besides that it returns pointer of bitmap type instead of opaque void *. Nit. There's no special type for bitmaps, bitmap_zalloc() returns long *. Is it less opaque than void *, not sure. > Acked-by: Dmitry Torokhov > Signed-off-by: Andy Shevchenko > --- > drivers/input/keyboard/gpio_keys.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/input/keyboard/gpio_keys.c > b/drivers/input/keyboard/gpio_keys.c > index 052e37675086..492a971b95b5 100644 > --- a/drivers/input/keyboard/gpio_keys.c > +++ b/drivers/input/keyboard/gpio_keys.c > @@ -196,7 +196,7 @@ static ssize_t gpio_keys_attr_show_helper(struct > gpio_keys_drvdata *ddata, > ssize_t ret; > int i; > > - bits = kcalloc(BITS_TO_LONGS(n_events), sizeof(*bits), GFP_KERNEL); > + bits = bitmap_zalloc(n_events, GFP_KERNEL); > if (!bits) > return -ENOMEM; > > @@ -216,7 +216,7 @@ static ssize_t gpio_keys_attr_show_helper(struct > gpio_keys_drvdata *ddata, > buf[ret++] = '\n'; > buf[ret] = '\0'; > > - kfree(bits); > + bitmap_free(bits); > > return ret; > } > @@ -240,7 +240,7 @@ static ssize_t gpio_keys_attr_store_helper(struct > gpio_keys_drvdata *ddata, > ssize_t error; > int i; > > - bits = kcalloc(BITS_TO_LONGS(n_events), sizeof(*bits), GFP_KERNEL); > + bits = bitmap_zalloc(n_events, GFP_KERNEL); > if (!bits) > return -ENOMEM; > > @@ -284,7 +284,7 @@ static ssize_t gpio_keys_attr_store_helper(struct > gpio_keys_drvdata *ddata, > mutex_unlock(>disable_lock); > > out: > - kfree(bits); > + bitmap_free(bits); > return error; > } > > -- > 2.17.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v2 3/5] bitmap: Add bitmap_alloc(), bitmap_zalloc() and bitmap_free()
A lot of code become ugly because of open coding allocations for bitmaps.
Introduce three helpers to allow users be more clear of intention
and keep their code neat.
Signed-off-by: Andy Shevchenko
---
include/linux/bitmap.h | 8
lib/bitmap.c | 19 +++
2 files changed, 27 insertions(+)
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 1ee46f492267..acf5e8df3504 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -104,6 +104,14 @@
* contain all bit positions from 0 to 'bits' - 1.
*/
+/*
+ * Allocation and deallocation of bitmap.
+ * Provided in lib/bitmap.c to avoid circular dependency.
+ */
+extern unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags);
+extern unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags);
+extern void bitmap_free(const unsigned long *bitmap);
+
/*
* lib/bitmap.c provides these functions:
*/
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 33e95cd359a2..09acf2fd6a35 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1125,6 +1126,24 @@ void bitmap_copy_le(unsigned long *dst, const unsigned
long *src, unsigned int n
EXPORT_SYMBOL(bitmap_copy_le);
#endif
+unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags)
+{
+ return kmalloc_array(BITS_TO_LONGS(nbits), sizeof(unsigned long),
flags);
+}
+EXPORT_SYMBOL(bitmap_alloc);
+
+unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags)
+{
+ return bitmap_alloc(nbits, flags | __GFP_ZERO);
+}
+EXPORT_SYMBOL(bitmap_zalloc);
+
+void bitmap_free(const unsigned long *bitmap)
+{
+ kfree(bitmap);
+}
+EXPORT_SYMBOL(bitmap_free);
+
#if BITS_PER_LONG == 64
/**
* bitmap_from_arr32 - copy the contents of u32 array of bits to bitmap
--
2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 3/5] bitmap: Add bitmap_alloc(), bitmap_zalloc() and bitmap_free()
On Sat, Jun 16, 2018 at 1:24 AM Yury Norov wrote: > On Fri, Jun 15, 2018 at 04:20:15PM +0300, Andy Shevchenko wrote: > > A lot of code become ugly because of open coding allocations for bitmaps. > > > > Introduce three helpers to allow users be more clear of intention > > and keep their code neat. > > I like the idea. But in following patches you switch to new API only > couple of drivers. I have converted much more, indeed. I published only for couple of drivers as an example. This is I mentioned in cover letter. > I think, it worth to switch more, especially core > users to make new API visible for developers. Brief grepping for > candidates showse only 17 suspected places: Yes, but not in this series. Since it covers many subsystems and drivers the applying that kind of series would take ages. That's why I have decided to submit for one subsystem only. -- With Best Regards, Andy Shevchenko -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 4/5] Input: gpio-keys - Switch to bitmap_zalloc()
On Sat, Jun 16, 2018 at 1:08 AM Yury Norov wrote: > > On Fri, Jun 15, 2018 at 04:20:16PM +0300, Andy Shevchenko wrote: > > External Email > > > > Switch to bitmap_zalloc() to show clearly what we are allocating. > > Besides that it returns pointer of bitmap type instead of opaque void *. > > Nit. There's no special type for bitmaps, bitmap_zalloc() returns long *. > Is it less opaque than void *, not sure. Yes, it enables type checking by compiler, which is good thing. You can't supply other pointer to this API, like unsigned char *bar = kmalloc(BITS_PER_LONGS(nbits) * sizeof(unsigned long), GFP_KERNEL); ... bitmap_free(bar); //<< compiler will not pass this silently! Or other way around: unsigned char *foo = bitmap_alloc(nbits, GFP_KERNEL); //<< wrong types! -- With Best Regards, Andy Shevchenko -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 1/5] md: Avoid namespace collision with bitmap API
On Fri, 2018-06-15 at 23:09 +0800, kbuild test robot wrote:
> Hi Andy,
>
> I love your patch! Yet something to improve:
Thanks!
I fixed that locally (definitely that module wasn't compiled by
default). Though I would wait to gather more comments before sending v3.
>
> [auto build test ERROR on next-20180615]
> [also build test ERROR on v4.17]
> [cannot apply to md/for-next linus/master dm/for-next v4.17 v4.17-rc7
> v4.17-rc6]
> [if your patch is applied to the wrong git tree, please drop us a note
> to help improve the system]
>
> url:https://github.com/0day-ci/linux/commits/Andy-Shevchenko/bitma
> p-Introduce-alloc-free-helpers/20180615-214724
> config: i386-randconfig-b0-06141412 (attached as .config)
> compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
> > > drivers/md/persistent-data/dm-space-map-common.c:111:23: error:
> > > 'bitmap_prepare_for_write' undeclared here (not in a function)
>
> .prepare_for_write = bitmap_prepare_for_write,
> ^
> > > drivers/md/persistent-data/dm-space-map-common.c:112:11: error:
> > > 'bitmap_check' undeclared here (not in a function)
>
> .check = bitmap_check
> ^
>drivers/md/persistent-data/dm-space-map-common.c:72:13: warning:
> 'md_bitmap_prepare_for_write' defined but not used [-Wunused-function]
> static void md_bitmap_prepare_for_write(struct dm_block_validator
> *v,
> ^
>drivers/md/persistent-data/dm-space-map-common.c:84:12: warning:
> 'md_bitmap_check' defined but not used [-Wunused-function]
> static int md_bitmap_check(struct dm_block_validator *v,
>^
>
> vim +/bitmap_prepare_for_write +111 drivers/md/persistent-data/dm-
> space-map-common.c
>
> 3241b1d3 Joe Thornber 2011-10-31 108
> 3241b1d3 Joe Thornber 2011-10-31 109 static struct
> dm_block_validator dm_sm_bitmap_validator = {
> 3241b1d3 Joe Thornber 2011-10-31 110 .name = "sm_bitmap",
> 3241b1d3 Joe Thornber 2011-10-31 @111 .prepare_for_write =
> bitmap_prepare_for_write,
> 3241b1d3 Joe Thornber 2011-10-31 @112 .check = bitmap_check
> 3241b1d3 Joe Thornber 2011-10-31 113 };
> 3241b1d3 Joe Thornber 2011-10-31 114
>
> :: The code at line 111 was first introduced by commit
> :: 3241b1d3e0aaafbfcd320f4d71ade629728cc4f4 dm: add persistent
> data library
>
> :: TO: Joe Thornber
> :: CC: Alasdair G Kergon
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology
> Center
> https://lists.01.org/pipermail/kbuild-all Intel
> Corporation
--
Andy Shevchenko
Intel Finland Oy
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 3/5] bitmap: Add bitmap_alloc(), bitmap_zalloc() and bitmap_free()
Hi Andy,
On Fri, Jun 15, 2018 at 04:20:15PM +0300, Andy Shevchenko wrote:
> A lot of code become ugly because of open coding allocations for bitmaps.
>
> Introduce three helpers to allow users be more clear of intention
> and keep their code neat.
I like the idea. But in following patches you switch to new API only
couple of drivers. I think, it worth to switch more, especially core
users to make new API visible for developers. Brief grepping for
candidates showse only 17 suspected places:
yury:linux$ git grep BITS_TO_LONGS | grep alloc | grep -v drivers | grep -v arch
kernel/events/uprobes.c:1188: area->bitmap =
kzalloc(BITS_TO_LONGS(UINSNS_PER_PAGE) * sizeof(long), GFP_KERNEL);
kernel/sysctl.c:3004: tmp_bitmap = kzalloc(BITS_TO_LONGS(bitmap_len)
* sizeof(unsigned long),
lib/genalloc.c:188: BITS_TO_LONGS(nbits) *
sizeof(long);
lib/test_printf.c:378: unsigned long *bits = kcalloc(BITS_TO_LONGS(nbits),
sizeof(long), GFP_KERNEL);
mm/percpu.c:1109: chunk->alloc_map =
memblock_virt_alloc(BITS_TO_LONGS(region_bits) *
mm/percpu.c:: chunk->bound_map =
memblock_virt_alloc(BITS_TO_LONGS(region_bits + 1) *
mm/percpu.c:1170: chunk->alloc_map =
pcpu_mem_zalloc(BITS_TO_LONGS(region_bits) *
mm/percpu.c:1175: chunk->bound_map =
pcpu_mem_zalloc(BITS_TO_LONGS(region_bits + 1) *
mm/slub.c:3668: unsigned long *map = kzalloc(BITS_TO_LONGS(page->objects) *
mm/slub.c:4435: unsigned long *map = kmalloc(BITS_TO_LONGS(oo_objects(s->max)) *
mm/slub.c:4596: unsigned long *map = kmalloc(BITS_TO_LONGS(oo_objects(s->max)) *
mm/swapfile.c:3219: frontswap_map =
kvzalloc(BITS_TO_LONGS(maxpages) * sizeof(long),
net/bridge/br_if.c:328: inuse = kcalloc(BITS_TO_LONGS(BR_MAX_PORTS),
sizeof(unsigned long),
net/bridge/br_vlan.c:836: changed = kcalloc(BITS_TO_LONGS(BR_MAX_PORTS),
sizeof(unsigned long),
net/mac80211/mesh_plink.c:465: aid_map =
kcalloc(BITS_TO_LONGS(IEEE80211_MAX_AID + 1),
net/sched/cls_u32.c:737:unsigned long *bitmap =
kzalloc(BITS_TO_LONGS(NR_U32_NODE) * sizeof(unsigned long),
tools/include/linux/bitmap.h:105: return calloc(1, BITS_TO_LONGS(nbits) *
sizeof(unsigned long));
Yury
>
> Signed-off-by: Andy Shevchenko
> ---
> include/linux/bitmap.h | 8
> lib/bitmap.c | 19 +++
> 2 files changed, 27 insertions(+)
>
> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> index 1ee46f492267..acf5e8df3504 100644
> --- a/include/linux/bitmap.h
> +++ b/include/linux/bitmap.h
> @@ -104,6 +104,14 @@
> * contain all bit positions from 0 to 'bits' - 1.
> */
>
> +/*
> + * Allocation and deallocation of bitmap.
> + * Provided in lib/bitmap.c to avoid circular dependency.
> + */
> +extern unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags);
> +extern unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags);
> +extern void bitmap_free(const unsigned long *bitmap);
> +
> /*
> * lib/bitmap.c provides these functions:
> */
> diff --git a/lib/bitmap.c b/lib/bitmap.c
> index 33e95cd359a2..09acf2fd6a35 100644
> --- a/lib/bitmap.c
> +++ b/lib/bitmap.c
> @@ -13,6 +13,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -1125,6 +1126,24 @@ void bitmap_copy_le(unsigned long *dst, const unsigned
> long *src, unsigned int n
> EXPORT_SYMBOL(bitmap_copy_le);
> #endif
>
> +unsigned long *bitmap_alloc(unsigned int nbits, gfp_t flags)
> +{
> + return kmalloc_array(BITS_TO_LONGS(nbits), sizeof(unsigned long),
> flags);
> +}
> +EXPORT_SYMBOL(bitmap_alloc);
> +
> +unsigned long *bitmap_zalloc(unsigned int nbits, gfp_t flags)
> +{
> + return bitmap_alloc(nbits, flags | __GFP_ZERO);
> +}
> +EXPORT_SYMBOL(bitmap_zalloc);
> +
> +void bitmap_free(const unsigned long *bitmap)
> +{
> + kfree(bitmap);
> +}
> +EXPORT_SYMBOL(bitmap_free);
> +
> #if BITS_PER_LONG == 64
> /**
> * bitmap_from_arr32 - copy the contents of u32 array of bits to bitmap
> --
> 2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] How to reattach a disconnected device with active device-mapper mappings on top
I have a hard disk connected to my laptop via an usb-sata bridge that somehow got disconnected. There is a stack of device-mapper mappings and a mounted filesystem on top of it that are currently failing all I/O. "dmsetup deps" reports "1 dependencies : (8, 16)". The same HDD is now reattached, but as /dev/sdc (8,32) and refuses to be attached as /dev/sdb, which is now a permanently vacant slot. How do I force this hard disk to attach as (8,16) so the whole device-mapper stack and the filesystem on top of it can resume working? Thank you, Fedja -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v2 1/5] md: Avoid namespace collision with bitmap API
bitmap API (include/linux/bitmap.h) has 'bitmap' prefix for its methods.
On the other hand MD bitmap API is special case.
Adding 'md' prefix to it to avoid name space collision.
Signed-off-by: Andy Shevchenko
---
drivers/md/dm-raid.c | 6 +-
drivers/md/md-bitmap.c| 301 +-
drivers/md/md-bitmap.h| 46 +--
drivers/md/md-cluster.c | 16 +-
drivers/md/md.c | 44 +--
.../md/persistent-data/dm-space-map-common.c | 8 +-
drivers/md/raid1.c| 20 +-
drivers/md/raid10.c | 26 +-
drivers/md/raid5-cache.c | 2 +-
drivers/md/raid5.c| 24 +-
10 files changed, 242 insertions(+), 251 deletions(-)
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index ab13fcec3fca..71d158560e69 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -3859,7 +3859,7 @@ static int __load_dirty_region_bitmap(struct raid_set *rs)
/* Try loading the bitmap unless "raid0", which does not have one */
if (!rs_is_raid0(rs) &&
!test_and_set_bit(RT_FLAG_RS_BITMAP_LOADED, >runtime_flags)) {
- r = bitmap_load(>md);
+ r = md_bitmap_load(>md);
if (r)
DMERR("Failed to load bitmap");
}
@@ -3987,8 +3987,8 @@ static int raid_preresume(struct dm_target *ti)
/* Resize bitmap to adjust to changed region size (aka MD bitmap
chunksize) */
if (test_bit(RT_FLAG_RS_BITMAP_LOADED, >runtime_flags) &&
mddev->bitmap &&
mddev->bitmap_info.chunksize !=
to_bytes(rs->requested_bitmap_chunk_sectors)) {
- r = bitmap_resize(mddev->bitmap, mddev->dev_sectors,
- to_bytes(rs->requested_bitmap_chunk_sectors),
0);
+ r = md_bitmap_resize(mddev->bitmap, mddev->dev_sectors,
+
to_bytes(rs->requested_bitmap_chunk_sectors), 0);
if (r)
DMERR("Failed to resize bitmap");
}
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index f983c3fdf204..307e7a90d78f 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -46,8 +46,8 @@ static inline char *bmname(struct bitmap *bitmap)
* if we find our page, we increment the page's refcount so that it stays
* allocated while we're using it
*/
-static int bitmap_checkpage(struct bitmap_counts *bitmap,
- unsigned long page, int create, int no_hijack)
+static int md_bitmap_checkpage(struct bitmap_counts *bitmap,
+ unsigned long page, int create, int no_hijack)
__releases(bitmap->lock)
__acquires(bitmap->lock)
{
@@ -115,7 +115,7 @@ __acquires(bitmap->lock)
/* if page is completely empty, put it back on the free list, or dealloc it */
/* if page was hijacked, unmark the flag so it might get alloced next time */
/* Note: lock should be held when calling this */
-static void bitmap_checkfree(struct bitmap_counts *bitmap, unsigned long page)
+static void md_bitmap_checkfree(struct bitmap_counts *bitmap, unsigned long
page)
{
char *ptr;
@@ -280,7 +280,7 @@ static int write_sb_page(struct bitmap *bitmap, struct page
*page, int wait)
return -EINVAL;
}
-static void bitmap_file_kick(struct bitmap *bitmap);
+static void md_bitmap_file_kick(struct bitmap *bitmap);
/*
* write out a page to a file
*/
@@ -310,7 +310,7 @@ static void write_page(struct bitmap *bitmap, struct page
*page, int wait)
atomic_read(>pending_writes)==0);
}
if (test_bit(BITMAP_WRITE_ERROR, >flags))
- bitmap_file_kick(bitmap);
+ md_bitmap_file_kick(bitmap);
}
static void end_bitmap_write(struct buffer_head *bh, int uptodate)
@@ -421,11 +421,11 @@ static int read_page(struct file *file, unsigned long
index,
*/
/*
- * bitmap_wait_writes() should be called before writing any bitmap
+ * md_bitmap_wait_writes() should be called before writing any bitmap
* blocks, to ensure previous writes, particularly from
- * bitmap_daemon_work(), have completed.
+ * md_bitmap_daemon_work(), have completed.
*/
-static void bitmap_wait_writes(struct bitmap *bitmap)
+static void md_bitmap_wait_writes(struct bitmap *bitmap)
{
if (bitmap->storage.file)
wait_event(bitmap->write_wait,
@@ -443,7 +443,7 @@ static void bitmap_wait_writes(struct bitmap *bitmap)
/* update the event counter and sync the superblock to disk */
-void bitmap_update_sb(struct bitmap *bitmap)
+void md_bitmap_update_sb(struct bitmap *bitmap)
{
bitmap_super_t *sb;
@@ -476,10 +476,10 @@ void bitmap_update_sb(struct bitmap *bitmap)
kunmap_atomic(sb);
write_page(bitmap, bitmap->storage.sb_page, 1);
}
-EXPORT_SYMBOL(bitmap_update_sb);
[dm-devel] [PATCH v2 4/5] Input: gpio-keys - Switch to bitmap_zalloc()
Switch to bitmap_zalloc() to show clearly what we are allocating. Besides that it returns pointer of bitmap type instead of opaque void *. Acked-by: Dmitry Torokhov Signed-off-by: Andy Shevchenko --- drivers/input/keyboard/gpio_keys.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index 052e37675086..492a971b95b5 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -196,7 +196,7 @@ static ssize_t gpio_keys_attr_show_helper(struct gpio_keys_drvdata *ddata, ssize_t ret; int i; - bits = kcalloc(BITS_TO_LONGS(n_events), sizeof(*bits), GFP_KERNEL); + bits = bitmap_zalloc(n_events, GFP_KERNEL); if (!bits) return -ENOMEM; @@ -216,7 +216,7 @@ static ssize_t gpio_keys_attr_show_helper(struct gpio_keys_drvdata *ddata, buf[ret++] = '\n'; buf[ret] = '\0'; - kfree(bits); + bitmap_free(bits); return ret; } @@ -240,7 +240,7 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata, ssize_t error; int i; - bits = kcalloc(BITS_TO_LONGS(n_events), sizeof(*bits), GFP_KERNEL); + bits = bitmap_zalloc(n_events, GFP_KERNEL); if (!bits) return -ENOMEM; @@ -284,7 +284,7 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata, mutex_unlock(>disable_lock); out: - kfree(bits); + bitmap_free(bits); return error; } -- 2.17.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 5/5] Input: evdev - Switch to bitmap_zalloc()
On Sat, Jun 16, 2018 at 12:46 AM Yury Norov wrote: > On Fri, Jun 15, 2018 at 04:20:17PM +0300, Andy Shevchenko wrote: > > Switch to bitmap_zalloc() to show clearly what we are allocating. > > Besides that it returns pointer of bitmap type instead of opaque void *. > > + mem = bitmap_alloc(maxbit, GFP_KERNEL); > > if (!mem) > > return -ENOMEM; > > But in commit message you say you switch to bitmap_zalloc(). IIUC > bitmap_alloc() is OK here. But could you please update comment to > avoid confusing. There are two places, one with alloc, another with zalloc. I will clarify this in commit message of next version. > > + mask = bitmap_zalloc(cnt, GFP_KERNEL); > > if (!mask) > > return -ENOMEM; > > > > error = bits_from_user(mask, cnt - 1, codes_size, codes, compat); > > If my understanding of bits_from_user() correct, here you can also use > bitmap_alloc(), true? While it might be true, it's a material for separate change. Original code uses zalloc version. -- With Best Regards, Andy Shevchenko -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v2 0/5] bitmap: Introduce alloc/free helpers
A lot of code is using allocation of bitmaps using BITS_PER_LONG() macro and sizeof(unsigned long) operator. The readability suffers because of this. The series introduces three helpers, i.e. bitmap_alloc(), bitmap_zalloc() and bitmap_free(), to make it more cleaner. Patch 1 is a preparatory to avoid namespace collisions between bitmap API and MD bitmap. No functional changes intended. Patch 2 is just orphaned from previous release cycle. Patch 3 introduces new helpers. Patches 4 and 5 is just an example how to use new helpers. Locally I have like dozen of them against different subsystems and drivers. Ideally it would go through Input subsystem, thus, needs an Ack from MD maintainer(s). Since v2: - added namespace fix patch against MD bitmap API - moved functions to lib/bitmap.c to avoid circular dependencies - appended Dmitry's tags Andy Shevchenko (5): md: Avoid namespace collision with bitmap API bitmap: Drop unnecessary 0 check for u32 array operations bitmap: Add bitmap_alloc(), bitmap_zalloc() and bitmap_free() Input: gpio-keys - Switch to bitmap_zalloc() Input: evdev - Switch to bitmap_zalloc() drivers/input/evdev.c | 16 +- drivers/input/keyboard/gpio_keys.c| 8 +- drivers/md/dm-raid.c | 6 +- drivers/md/md-bitmap.c| 301 +- drivers/md/md-bitmap.h| 46 +-- drivers/md/md-cluster.c | 16 +- drivers/md/md.c | 44 +-- .../md/persistent-data/dm-space-map-common.c | 8 +- drivers/md/raid1.c| 20 +- drivers/md/raid10.c | 26 +- drivers/md/raid5-cache.c | 2 +- drivers/md/raid5.c| 24 +- include/linux/bitmap.h| 8 + lib/bitmap.c | 28 +- 14 files changed, 281 insertions(+), 272 deletions(-) -- 2.17.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v2 5/5] Input: evdev - Switch to bitmap_zalloc()
Switch to bitmap_zalloc() to show clearly what we are allocating.
Besides that it returns pointer of bitmap type instead of opaque void *.
Acked-by: Dmitry Torokhov
Signed-off-by: Andy Shevchenko
---
drivers/input/evdev.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index c81c79d01d93..370206f987f9 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -481,7 +481,7 @@ static int evdev_release(struct inode *inode, struct file
*file)
evdev_detach_client(evdev, client);
for (i = 0; i < EV_CNT; ++i)
- kfree(client->evmasks[i]);
+ bitmap_free(client->evmasks[i]);
kvfree(client);
@@ -925,17 +925,15 @@ static int evdev_handle_get_val(struct evdev_client
*client,
{
int ret;
unsigned long *mem;
- size_t len;
- len = BITS_TO_LONGS(maxbit) * sizeof(unsigned long);
- mem = kmalloc(len, GFP_KERNEL);
+ mem = bitmap_alloc(maxbit, GFP_KERNEL);
if (!mem)
return -ENOMEM;
spin_lock_irq(>event_lock);
spin_lock(>buffer_lock);
- memcpy(mem, bits, len);
+ bitmap_copy(mem, bits, maxbit);
spin_unlock(>event_lock);
@@ -947,7 +945,7 @@ static int evdev_handle_get_val(struct evdev_client *client,
if (ret < 0)
evdev_queue_syn_dropped(client);
- kfree(mem);
+ bitmap_free(mem);
return ret;
}
@@ -1003,13 +1001,13 @@ static int evdev_set_mask(struct evdev_client *client,
if (!cnt)
return 0;
- mask = kcalloc(sizeof(unsigned long), BITS_TO_LONGS(cnt), GFP_KERNEL);
+ mask = bitmap_zalloc(cnt, GFP_KERNEL);
if (!mask)
return -ENOMEM;
error = bits_from_user(mask, cnt - 1, codes_size, codes, compat);
if (error < 0) {
- kfree(mask);
+ bitmap_free(mask);
return error;
}
@@ -1018,7 +1016,7 @@ static int evdev_set_mask(struct evdev_client *client,
client->evmasks[type] = mask;
spin_unlock_irqrestore(>buffer_lock, flags);
- kfree(oldmask);
+ bitmap_free(oldmask);
return 0;
}
--
2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 5/5] Input: evdev - Switch to bitmap_zalloc()
Hi Andy,
On Fri, Jun 15, 2018 at 04:20:17PM +0300, Andy Shevchenko wrote:
> Switch to bitmap_zalloc() to show clearly what we are allocating.
> Besides that it returns pointer of bitmap type instead of opaque void *.
>
> Acked-by: Dmitry Torokhov
> Signed-off-by: Andy Shevchenko
> ---
> drivers/input/evdev.c | 16 +++-
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index c81c79d01d93..370206f987f9 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -481,7 +481,7 @@ static int evdev_release(struct inode *inode, struct file
> *file)
> evdev_detach_client(evdev, client);
>
> for (i = 0; i < EV_CNT; ++i)
> - kfree(client->evmasks[i]);
> + bitmap_free(client->evmasks[i]);
>
> kvfree(client);
>
> @@ -925,17 +925,15 @@ static int evdev_handle_get_val(struct evdev_client
> *client,
> {
> int ret;
> unsigned long *mem;
> - size_t len;
>
> - len = BITS_TO_LONGS(maxbit) * sizeof(unsigned long);
> - mem = kmalloc(len, GFP_KERNEL);
> + mem = bitmap_alloc(maxbit, GFP_KERNEL);
> if (!mem)
> return -ENOMEM;
But in commit message you say you switch to bitmap_zalloc(). IIUC
bitmap_alloc() is OK here. But could you please update comment to
avoid confusing.
>
> spin_lock_irq(>event_lock);
> spin_lock(>buffer_lock);
>
> - memcpy(mem, bits, len);
> + bitmap_copy(mem, bits, maxbit);
>
> spin_unlock(>event_lock);
>
> @@ -947,7 +945,7 @@ static int evdev_handle_get_val(struct evdev_client
> *client,
> if (ret < 0)
> evdev_queue_syn_dropped(client);
>
> - kfree(mem);
> + bitmap_free(mem);
>
> return ret;
> }
> @@ -1003,13 +1001,13 @@ static int evdev_set_mask(struct evdev_client *client,
> if (!cnt)
> return 0;
>
> - mask = kcalloc(sizeof(unsigned long), BITS_TO_LONGS(cnt), GFP_KERNEL);
> + mask = bitmap_zalloc(cnt, GFP_KERNEL);
> if (!mask)
> return -ENOMEM;
>
> error = bits_from_user(mask, cnt - 1, codes_size, codes, compat);
If my understanding of bits_from_user() correct, here you can also use
bitmap_alloc(), true?
> if (error < 0) {
> - kfree(mask);
> + bitmap_free(mask);
> return error;
> }
>
> @@ -1018,7 +1016,7 @@ static int evdev_set_mask(struct evdev_client *client,
> client->evmasks[type] = mask;
> spin_unlock_irqrestore(>buffer_lock, flags);
>
> - kfree(oldmask);
> + bitmap_free(oldmask);
>
> return 0;
> }
> --
> 2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v2 2/5] bitmap: Drop unnecessary 0 check for u32 array operations
The nbits == 0 is safe to be supplied to the function body, so,
remove unnecessary checks in bitmap_to_arr32() and bitmap_from_arr32().
Acked-by: Yury Norov
Signed-off-by: Andy Shevchenko
---
lib/bitmap.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 58f9750e49c6..33e95cd359a2 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -1132,14 +1132,10 @@ EXPORT_SYMBOL(bitmap_copy_le);
* @buf: array of u32 (in host byte order), the source bitmap
* @nbits: number of bits in @bitmap
*/
-void bitmap_from_arr32(unsigned long *bitmap, const u32 *buf,
- unsigned int nbits)
+void bitmap_from_arr32(unsigned long *bitmap, const u32 *buf, unsigned int
nbits)
{
unsigned int i, halfwords;
- if (!nbits)
- return;
-
halfwords = DIV_ROUND_UP(nbits, 32);
for (i = 0; i < halfwords; i++) {
bitmap[i/2] = (unsigned long) buf[i];
@@ -1163,9 +1159,6 @@ void bitmap_to_arr32(u32 *buf, const unsigned long
*bitmap, unsigned int nbits)
{
unsigned int i, halfwords;
- if (!nbits)
- return;
-
halfwords = DIV_ROUND_UP(nbits, 32);
for (i = 0; i < halfwords; i++) {
buf[i] = (u32) (bitmap[i/2] & UINT_MAX);
--
2.17.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 15/28] libmultipath: merge hwentries inside a conf file
Hi Ben,
On Fri, 2018-06-15 at 13:03 -0500, Benjamin Marzinski wrote:
> On Fri, Jun 08, 2018 at 12:20:28PM +0200, Martin Wilck wrote:
> > Merging hwentries with identical vendor/product/revision is still
> > useful,
> > although it's not strictly necessary any more. But such identical
> > entries
> > should always be merged, not only if they appear in different
> > configuration
> > files. This requires changing the logic of factorize_hwtable.
>
> Sorry for my slowness in reviewing these. This one looks wrong to me
Thanks for the thorough review. Given the size of the series, I don't
think this was slow.
> > By setting -DCHECK_BUILTIN_HWTABLE at compile time, the built-in
> > hwtable
> > can be checked against duplicates as well.
> >
> > Signed-off-by: Martin Wilck
> > ---
> > libmultipath/config.c | 43 +
> > --
> > 1 file changed, 21 insertions(+), 22 deletions(-)
> >
> > diff --git a/libmultipath/config.c b/libmultipath/config.c
> > index 713ac7f3..fb41d620 100644
> > --- a/libmultipath/config.c
> > +++ b/libmultipath/config.c
> > @@ -452,7 +452,7 @@ out:
> > }
> >
> > static void
> > -factorize_hwtable (vector hw, int n)
> > +factorize_hwtable (vector hw, int n, const char *table_desc)
> > {
> > struct hwentry *hwe1, *hwe2;
> > int i, j;
> > @@ -462,22 +462,26 @@ restart:
> > if (i == n)
> > break;
> > j = n;
> > + /* drop invalid device configs */
> > + if (i >= n && (!hwe1->vendor || !hwe1->product)) {
>
> How can i >= n? vector_foreach_slot() starts i at 0, and then next
> lines are
>
> if (i == n)
> break;
The above two lines, and the line "j = n" after that, were meant to be
deleted. That slipped through in my rebasing work, sorry about that.
When these lines are gone, I believe your issues are resolved, see
below.
n denotes the number of hwentries present before the current config
file was parsed. Before my patch series, "factorize_hwtable" would only
check for duplicates between the "old" (i < n) and "new" (i >= n)
sections, and would not remove the dups inside either section. Now
duplicates are also merged if they occur in the same section (except
for the built-in hwtable, for which the "inside" check is only done
with -DCHECK_BUILTIN_HWTABLE).
Well - that's what I intended to do, but by by forgetting to delete the
above lines, I failed :-(
In practice, this error only breaks the "broken hwentry" and "dup
removal inside a section" features - application of "good" hwentries to
paths and multipaths isn't affected, and thus the test cases pass
despite the bug. I'll post a fix, plus new test cases covering it.
> > + condlog(0, "device config in %s missing
> > vendor or product parameter",
> > + table_desc);
> > + vector_del_slot(hw, i--);
>
> shouldn't we also decrement n here. I realize that doesn't work well
> for
> the CHECK_BUILTIN_HWTABLE check, where n is 0, but as I said above,
> I'm
> pretty sure that will never get here.
No, see above. The check for broken entries is only done for "new" ones
(i > n), as we assume the entries up to n to be checked already. So
when we reach this condition, is is really always above n.
>
> > + free_hwe(hwe1);
> > + continue;
> > + }
> > + j = n > i + 1 ? n : i + 1;
>
> again, n must always be at least i + 1 to get here, so j is always n.
See above.
>
> > vector_foreach_slot_after(hw, hwe2, j) {
> > - /* drop invalid device configs */
> > - if (!hwe2->vendor || !hwe2->product) {
> > - condlog(0, "device config missing
> > vendor or product parameter");
> > - vector_del_slot(hw, j--);
> > - free_hwe(hwe2);
> > - continue;
> > - }
> > if (hwe_strmatch(hwe2, hwe1) == 0) {
> > - condlog(4, "%s: removing hwentry
> > %s:%s:%s",
> > + condlog(i >= n ? 1 : 3,
>
> this check also looks wrong
The intention is not to log at prio 1 if a new section (read in most
cases: multipath.conf) contains a duplicate to a previous section (read
in most cases: built-in hwtable). Such use is perfectly valid, whereas
two hwentries with the same vendor and product in one config file are
suspicious and should be logged.
>
> > + "%s: duplicate device
> > section for %s:%s:%s in %s",
> > __func__, hwe1->vendor,
> > hwe1->product,
> > - hwe1->revision);
> > + hwe1->revision,
> > table_desc);
> > vector_del_slot(hw, i);
> > merge_hwe(hwe2, hwe1);
> >
