On Tue, 25 May 2021, Damien Le Moal wrote:
> On 2021/05/26 4:50, Mikulas Patocka wrote:
> > The functions set_bit and clear_bit are atomic. We don't need atomicity
> > when making flags for dm-kcopyd. So, change them to direct manipulation of
> > the flags.
> >
> > Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> >
> > Index: linux-2.6/drivers/md/dm-kcopyd.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/md/dm-kcopyd.c
> > +++ linux-2.6/drivers/md/dm-kcopyd.c
> > @@ -812,7 +812,7 @@ void dm_kcopyd_copy(struct dm_kcopyd_cli
> > if (!test_bit(DM_KCOPYD_WRITE_SEQ, &job->flags)) {
> > for (i = 0; i < job->num_dests; i++) {
> > if (bdev_zoned_model(dests[i].bdev) == BLK_ZONED_HM) {
> > - set_bit(DM_KCOPYD_WRITE_SEQ, &job->flags);
> > + job->flags |= 1UL << DM_KCOPYD_WRITE_SEQ;
>
> How about using the BIT() macro ?
>
> job->flags |= BIT(DM_KCOPYD_WRITE_SEQ);
>
> But I know some do not like that macro :)
Yes - it is better to use it.
I also changed flags from unsigned long to unsigned, to make the structure
smaller.
From: Mikulas Patocka <mpatocka@xxxxxxxxxx>
dm-kcopyd: avoid useless atomic operations
The functions set_bit and clear_bit are atomic. We don't need atomicity
when making flags for dm-kcopyd. So, change them to direct manipulation of
the flags.
Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
Index: linux-2.6/drivers/md/dm-kcopyd.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-kcopyd.c
+++ linux-2.6/drivers/md/dm-kcopyd.c
@@ -341,7 +341,7 @@ static void client_free_pages(struct dm_
struct kcopyd_job {
struct dm_kcopyd_client *kc;
struct list_head list;
- unsigned long flags;
+ unsigned flags;
/*
* Error state of the job.
@@ -418,7 +418,7 @@ static struct kcopyd_job *pop_io_job(str
* constraint and sequential writes that are at the right position.
*/
list_for_each_entry(job, jobs, list) {
- if (job->rw == READ || !test_bit(DM_KCOPYD_WRITE_SEQ, &job->flags)) {
+ if (job->rw == READ || !(job->flags & BIT(DM_KCOPYD_WRITE_SEQ))) {
list_del(&job->list);
return job;
}
@@ -529,7 +529,7 @@ static void complete_io(unsigned long er
else
job->read_err = 1;
- if (!test_bit(DM_KCOPYD_IGNORE_ERROR, &job->flags)) {
+ if (!(job->flags & BIT(DM_KCOPYD_IGNORE_ERROR))) {
push(&kc->complete_jobs, job);
wake(kc);
return;
@@ -572,7 +572,7 @@ static int run_io_job(struct kcopyd_job
* If we need to write sequentially and some reads or writes failed,
* no point in continuing.
*/
- if (test_bit(DM_KCOPYD_WRITE_SEQ, &job->flags) &&
+ if (job->flags & BIT(DM_KCOPYD_WRITE_SEQ) &&
job->master_job->write_err) {
job->write_err = job->master_job->write_err;
return -EIO;
@@ -716,7 +716,7 @@ static void segment_complete(int read_er
* Only dispatch more work if there hasn't been an error.
*/
if ((!job->read_err && !job->write_err) ||
- test_bit(DM_KCOPYD_IGNORE_ERROR, &job->flags)) {
+ job->flags & BIT(DM_KCOPYD_IGNORE_ERROR)) {
/* get the next chunk of work */
progress = job->progress;
count = job->source.count - progress;
@@ -809,10 +809,10 @@ void dm_kcopyd_copy(struct dm_kcopyd_cli
* we need to write sequentially. If one of the destination is a
* host-aware device, then leave it to the caller to choose what to do.
*/
- if (!test_bit(DM_KCOPYD_WRITE_SEQ, &job->flags)) {
+ if (!(job->flags & BIT(DM_KCOPYD_WRITE_SEQ))) {
for (i = 0; i < job->num_dests; i++) {
if (bdev_zoned_model(dests[i].bdev) == BLK_ZONED_HM) {
- set_bit(DM_KCOPYD_WRITE_SEQ, &job->flags);
+ job->flags |= BIT(DM_KCOPYD_WRITE_SEQ);
break;
}
}
@@ -821,9 +821,9 @@ void dm_kcopyd_copy(struct dm_kcopyd_cli
/*
* If we need to write sequentially, errors cannot be ignored.
*/
- if (test_bit(DM_KCOPYD_WRITE_SEQ, &job->flags) &&
- test_bit(DM_KCOPYD_IGNORE_ERROR, &job->flags))
- clear_bit(DM_KCOPYD_IGNORE_ERROR, &job->flags);
+ if (job->flags & BIT(DM_KCOPYD_WRITE_SEQ) &&
+ job->flags & BIT(DM_KCOPYD_IGNORE_ERROR))
+ job->flags &= ~BIT(DM_KCOPYD_IGNORE_ERROR);
if (from) {
job->source = *from;
Index: linux-2.6/drivers/md/dm-raid1.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-raid1.c
+++ linux-2.6/drivers/md/dm-raid1.c
@@ -364,7 +364,7 @@ static void recover(struct mirror_set *m
/* hand to kcopyd */
if (!errors_handled(ms))
- set_bit(DM_KCOPYD_IGNORE_ERROR, &flags);
+ flags |= BIT(DM_KCOPYD_IGNORE_ERROR);
dm_kcopyd_copy(ms->kcopyd_client, &from, ms->nr_mirrors - 1, to,
flags, recovery_complete, reg);
Index: linux-2.6/drivers/md/dm-zoned-reclaim.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-zoned-reclaim.c
+++ linux-2.6/drivers/md/dm-zoned-reclaim.c
@@ -134,7 +134,7 @@ static int dmz_reclaim_copy(struct dmz_r
dst_zone_block = dmz_start_block(zmd, dst_zone);
if (dmz_is_seq(dst_zone))
- set_bit(DM_KCOPYD_WRITE_SEQ, &flags);
+ flags |= BIT(DM_KCOPYD_WRITE_SEQ);
while (block < end_block) {
if (src_zone->dev->flags & DMZ_BDEV_DYING)
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
