Re: [dm-devel] [PATCH] fixup! libmultipath: use directio checker for LIO targets
On Fri, 2023-03-31 at 08:47 -0500, Benjamin Marzinski wrote: > On Fri, Mar 31, 2023 at 10:28:35AM +0200, mwi...@suse.com wrote: > > From: Martin Wilck > > > > When we set the check to something other than TUR, we need > > to disable checker detection, too. > > > > Signed-off-by: Martin Wilck > > Reviewed-by: Benjamin Marzinski Thanks! I am going to squash this in. Martin -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] dm raid: remove unused d variable
On Thu, Mar 30, 2023 at 11:28 PM Tom Rix wrote:
> clang with W=1 reports
> drivers/md/dm-raid.c:2212:15: error: variable
> 'd' set but not used [-Werror,-Wunused-but-set-variable]
> unsigned int d;
> ^
> This variable is not used so remove it.
>
> Signed-off-by: Tom Rix
> ---
> drivers/md/dm-raid.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index 60632b409b80..2dfd51509647 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -2209,7 +2209,6 @@ static int super_load(struct md_rdev *rdev, struct
> md_rdev *refdev)
> static int super_init_validation(struct raid_set *rs, struct md_rdev
> *rdev)
> {
> int role;
> - unsigned int d;
> struct mddev *mddev = &rs->md;
> uint64_t events_sb;
> uint64_t failed_devices[DISKS_ARRAY_ELEMS];
> @@ -2324,7 +2323,6 @@ static int super_init_validation(struct raid_set
> *rs, struct md_rdev *rdev)
> *to provide capacity for redundancy or during reshape
> *to add capacity to grow the raid set.
> */
> - d = 0;
> rdev_for_each(r, mddev) {
> if (test_bit(Journal, &rdev->flags))
> continue;
> @@ -2340,8 +2338,6 @@ static int super_init_validation(struct raid_set
> *rs, struct md_rdev *rdev)
> if (test_bit(FirstUse, &r->flags))
> rebuild_and_new++;
> }
> -
> - d++;
> }
>
> if (new_devs == rs->raid_disks || !rebuilds) {
> --
> 2.27.0
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://listman.redhat.com/mailman/listinfo/dm-devel
>
>
Acked
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] fixup! libmultipath: use directio checker for LIO targets
On Fri, Mar 31, 2023 at 10:28:35AM +0200, mwi...@suse.com wrote:
> From: Martin Wilck
>
> When we set the check to something other than TUR, we need
> to disable checker detection, too.
>
> Signed-off-by: Martin Wilck
Reviewed-by: Benjamin Marzinski
> ---
> libmultipath/hwtable.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> index c2a024c..65bca74 100644
> --- a/libmultipath/hwtable.c
> +++ b/libmultipath/hwtable.c
> @@ -1068,6 +1068,7 @@ static struct hwentry default_hw[] = {
> .no_path_retry = 12,
> .prio_name = PRIO_ALUA,
> .checker_name = DIRECTIO,
> + .detect_checker = DETECT_CHECKER_OFF,
> },
> /*
>* DataCore
> --
> 2.39.2
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 13/19] zram: use __bio_add_page for adding single page to bio
On Wed, Mar 29, 2023 at 10:05:59AM -0700, Johannes Thumshirn wrote: > The zram writeback code uses bio_add_page() to add a page to a newly > created bio. bio_add_page() can fail, but the return value is never > checked. > > Use __bio_add_page() as adding a single page to a newly created bio is > guaranteed to succeed. > > This brings us a step closer to marking bio_add_page() as __must_check. > > Signed-off-by: Johannes Thumshirn Looks good, Reviewed-by: Pankaj Raghav -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 01/19] swap: use __bio_add_page to add page to bio
On Wed, Mar 29, 2023 at 10:05:47AM -0700, Johannes Thumshirn wrote: > The swap code only adds a single page to a newly created bio. So use > __bio_add_page() to add the page which is guaranteed to succeed in this > case. > > This brings us closer to marking bio_add_page() as __must_check. > > Signed-off-by: Johannes Thumshirn Looks good, Reviewed-by: Pankaj Raghav -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 2/7] dm: Add support for block provisioning
On Thu, Mar 30, 2023 at 05:30:22PM -0700, Sarthak Kukreti wrote:
> On Thu, Jan 5, 2023 at 6:42 AM Brian Foster wrote:
> >
> > On Thu, Dec 29, 2022 at 12:12:47AM -0800, Sarthak Kukreti wrote:
> > > Add support to dm devices for REQ_OP_PROVISION. The default mode
> > > is to pass through the request and dm-thin will utilize it to provision
> > > blocks.
> > >
> > > Signed-off-by: Sarthak Kukreti
> > > ---
> > > drivers/md/dm-crypt.c | 4 +-
> > > drivers/md/dm-linear.c| 1 +
> > > drivers/md/dm-snap.c | 7 +++
> > > drivers/md/dm-table.c | 25 ++
> > > drivers/md/dm-thin.c | 90 ++-
> > > drivers/md/dm.c | 4 ++
> > > include/linux/device-mapper.h | 11 +
> > > 7 files changed, 139 insertions(+), 3 deletions(-)
> > >
> > ...
> > > diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
> > > index 64cfcf46881d..ab3f1abfabaf 100644
> > > --- a/drivers/md/dm-thin.c
> > > +++ b/drivers/md/dm-thin.c
> > ...
> > > @@ -1980,6 +1992,70 @@ static void process_cell(struct thin_c *tc, struct
> > > dm_bio_prison_cell *cell)
> > > }
> > > }
> > >
> > > +static void process_provision_cell(struct thin_c *tc, struct
> > > dm_bio_prison_cell *cell)
> > > +{
> > > + int r;
> > > + struct pool *pool = tc->pool;
> > > + struct bio *bio = cell->holder;
> > > + dm_block_t begin, end;
> > > + struct dm_thin_lookup_result lookup_result;
> > > +
> > > + if (tc->requeue_mode) {
> > > + cell_requeue(pool, cell);
> > > + return;
> > > + }
> > > +
> > > + get_bio_block_range(tc, bio, &begin, &end);
> > > +
> > > + while (begin != end) {
> > > + r = ensure_next_mapping(pool);
> > > + if (r)
> > > + /* we did our best */
> > > + return;
> > > +
> > > + r = dm_thin_find_block(tc->td, begin, 1, &lookup_result);
> >
> > Hi Sarthak,
> >
> > I think we discussed this before.. but remind me if/how we wanted to
> > handle the case if the thin blocks are shared..? Would a provision op
> > carry enough information to distinguish an FALLOC_FL_UNSHARE_RANGE
> > request from upper layers to conditionally provision in that case?
> >
> I think that should depend on how the filesystem implements unsharing:
> assuming that we use provision on first allocation, unsharing on xfs
> should result in xfs calling REQ_OP_PROVISION on the newly allocated
> blocks first. But for ext4, we'd fail UNSHARE_RANGE unless provision
> (instead of noprovision, provision_on_alloc), in which case, we'd send
> REQ_OP_PROVISION.
>
I think my question was unclear... It doesn't necessarily have much to
do with the filesystem or associated provision policy. Since dm-thin can
share blocks internally via snapshots, do you intend to support
FL_UNSHARE_RANGE via blkdev_fallocate() and REQ_OP_PROVISION?
If so, then presumably this wants an UNSHARE request flag to pair with
REQ_OP_PROVISION. Also, the dm-thin code above needs to check whether an
existing block it finds is shared and basically do whatever COW breaking
is necessary during the PROVISION request.
If not, why? And what is expected behavior if blkdev_fallocate() is
called with FL_UNSHARE_RANGE?
Brian
> Best
> Sarthak
>
>
> Sarthak
>
> > Brian
> >
> > > + switch (r) {
> > > + case 0:
> > > + begin++;
> > > + break;
> > > + case -ENODATA:
> > > + bio_inc_remaining(bio);
> > > + provision_block(tc, bio, begin, cell);
> > > + begin++;
> > > + break;
> > > + default:
> > > + DMERR_LIMIT(
> > > + "%s: dm_thin_find_block() failed: error =
> > > %d",
> > > + __func__, r);
> > > + cell_defer_no_holder(tc, cell);
> > > + bio_io_error(bio);
> > > + begin++;
> > > + break;
> > > + }
> > > + }
> > > + bio_endio(bio);
> > > + cell_defer_no_holder(tc, cell);
> > > +}
> > > +
> > > +static void process_provision_bio(struct thin_c *tc, struct bio *bio)
> > > +{
> > > + dm_block_t begin, end;
> > > + struct dm_cell_key virt_key;
> > > + struct dm_bio_prison_cell *virt_cell;
> > > +
> > > + get_bio_block_range(tc, bio, &begin, &end);
> > > + if (begin == end) {
> > > + bio_endio(bio);
> > > + return;
> > > + }
> > > +
> > > + build_key(tc->td, VIRTUAL, begin, end, &virt_key);
> > > + if (bio_detain(tc->pool, &virt_key, bio, &virt_cell))
> > > + return;
> > > +
> > > + process_provision_cell(tc, virt_cell);
> > > +}
> > > +
> > > static void process_bio(struct thin_c *tc, struct bio *bio)
> > > {
> > > struct pool *pool = tc->pool;
> > > @@ -2200,6 +2276,8 @@ stat
Re: [dm-devel] [PATCH 04/19] fs: buffer: use __bio_add_page to add single page to bio
On Wed, Mar 29, 2023 at 10:05:50AM -0700, Johannes Thumshirn wrote: > The buffer_head submission code uses bio_add_page() to add a page to a > newly created bio. bio_add_page() can fail, but the return value is never > checked. > > Use __bio_add_page() as adding a single page to a newly created bio is > guaranteed to succeed. > > This brings us a step closer to marking bio_add_page() as __must_check. > > Signed-off-by: Johannes Thumshirn > --- Looks good, Reviewed-by: Pankaj Raghav -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] fixup! libmultipath: use directio checker for LIO targets
From: Martin Wilck
When we set the check to something other than TUR, we need
to disable checker detection, too.
Signed-off-by: Martin Wilck
---
libmultipath/hwtable.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index c2a024c..65bca74 100644
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -1068,6 +1068,7 @@ static struct hwentry default_hw[] = {
.no_path_retry = 12,
.prio_name = PRIO_ALUA,
.checker_name = DIRECTIO,
+ .detect_checker = DETECT_CHECKER_OFF,
},
/*
* DataCore
--
2.39.2
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
