| Message ID | 20200619123803.1441373-2-yangerkun@huawei.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | introduce interface to list badblocks | expand |
On Fri, Jun 19, 2020 at 8:37 AM yangerkun <yangerkun@huawei.com> wrote: > > Some type of message(queryblock/countbadblocks/removebadblock) may better > report results to user directly. Do it with DMEMIT. > > Signed-off-by: yangerkun <yangerkun@huawei.com> > --- > drivers/md/dm-dust.c | 31 ++++++++++++++++++------------- > 1 file changed, 18 insertions(+), 13 deletions(-) > > diff --git a/drivers/md/dm-dust.c b/drivers/md/dm-dust.c > index ff03b90072c5..a0c75c104de0 100644 > --- a/drivers/md/dm-dust.c > +++ b/drivers/md/dm-dust.c > @@ -138,20 +138,22 @@ static int dust_add_block(struct dust_device *dd, unsigned long long block, > return 0; > } > > -static int dust_query_block(struct dust_device *dd, unsigned long long block) > +static int dust_query_block(struct dust_device *dd, unsigned long long block, char *result, > + unsigned int maxlen, unsigned int *sz_ptr) > { > struct badblock *bblock; > unsigned long flags; > + unsigned int sz = *sz_ptr; > > spin_lock_irqsave(&dd->dust_lock, flags); > bblock = dust_rb_search(&dd->badblocklist, block); > if (bblock != NULL) > - DMINFO("%s: block %llu found in badblocklist", __func__, block); > + DMEMIT("block %llu found in badblocklist", block); > else > - DMINFO("%s: block %llu not found in badblocklist", __func__, block); > + DMEMIT("block %llu not found in badblocklist", block); > spin_unlock_irqrestore(&dd->dust_lock, flags); > > - return 0; > + return 1; First, thank you very much for this patch. After the concerns to convert some functions to use DMEMIT were brought up, I was trying to start the conversion, when this patch arrived, so I installed it, and tested it. I do have a question, though: First, I see that in dust_query_block() (above) and dust_clear_badblocks(), the "return 0" statement is changed to "return 1". (Additionally, there is a change from "r = 0" to "r = 1", in the "countbadblocks" message handler) On testing the functions, they still work, but why was this change made? Is it related to the use of DMEMIT? Thanks, Bryan > } > > static int __dust_map_read(struct dust_device *dd, sector_t thisblock) > @@ -259,11 +261,13 @@ static bool __dust_clear_badblocks(struct rb_root *tree, > return true; > } > > -static int dust_clear_badblocks(struct dust_device *dd) > +static int dust_clear_badblocks(struct dust_device *dd, char *result, unsigned int maxlen, > + unsigned int *sz_ptr) > { > unsigned long flags; > struct rb_root badblocklist; > unsigned long long badblock_count; > + unsigned int sz = *sz_ptr; > > spin_lock_irqsave(&dd->dust_lock, flags); > badblocklist = dd->badblocklist; > @@ -273,11 +277,11 @@ static int dust_clear_badblocks(struct dust_device *dd) > spin_unlock_irqrestore(&dd->dust_lock, flags); > > if (!__dust_clear_badblocks(&badblocklist, badblock_count)) > - DMINFO("%s: no badblocks found", __func__); > + DMEMIT("no badblocks found"); > else > - DMINFO("%s: badblocks cleared", __func__); > + DMEMIT("badblocks cleared"); > > - return 0; > + return 1; > } > > /* > @@ -383,7 +387,7 @@ static void dust_dtr(struct dm_target *ti) > } > > static int dust_message(struct dm_target *ti, unsigned int argc, char **argv, > - char *result_buf, unsigned int maxlen) > + char *result, unsigned int maxlen) > { > struct dust_device *dd = ti->private; > sector_t size = i_size_read(dd->dev->bdev->bd_inode) >> SECTOR_SHIFT; > @@ -393,6 +397,7 @@ static int dust_message(struct dm_target *ti, unsigned int argc, char **argv, > unsigned char wr_fail_cnt; > unsigned int tmp_ui; > unsigned long flags; > + unsigned int sz = 0; > char dummy; > > if (argc == 1) { > @@ -410,12 +415,12 @@ static int dust_message(struct dm_target *ti, unsigned int argc, char **argv, > r = 0; > } else if (!strcasecmp(argv[0], "countbadblocks")) { > spin_lock_irqsave(&dd->dust_lock, flags); > - DMINFO("countbadblocks: %llu badblock(s) found", > + DMEMIT("countbadblocks: %llu badblock(s) found", > dd->badblock_count); > spin_unlock_irqrestore(&dd->dust_lock, flags); > - r = 0; > + r = 1; > } else if (!strcasecmp(argv[0], "clearbadblocks")) { > - r = dust_clear_badblocks(dd); > + r = dust_clear_badblocks(dd, result, maxlen, &sz); > } else if (!strcasecmp(argv[0], "quiet")) { > if (!dd->quiet_mode) > dd->quiet_mode = true; > @@ -441,7 +446,7 @@ static int dust_message(struct dm_target *ti, unsigned int argc, char **argv, > else if (!strcasecmp(argv[0], "removebadblock")) > r = dust_remove_block(dd, block); > else if (!strcasecmp(argv[0], "queryblock")) > - r = dust_query_block(dd, block); > + r = dust_query_block(dd, block, result, maxlen, &sz); > else > invalid_msg = true; > > -- > 2.25.4 > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Fri, Jun 19 2020 at 11:40am -0400, Bryan Gurney <bgurney@redhat.com> wrote: > On Fri, Jun 19, 2020 at 8:37 AM yangerkun <yangerkun@huawei.com> wrote: > > > > Some type of message(queryblock/countbadblocks/removebadblock) may better > > report results to user directly. Do it with DMEMIT. > > > > Signed-off-by: yangerkun <yangerkun@huawei.com> > > --- > > drivers/md/dm-dust.c | 31 ++++++++++++++++++------------- > > 1 file changed, 18 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/md/dm-dust.c b/drivers/md/dm-dust.c > > index ff03b90072c5..a0c75c104de0 100644 > > --- a/drivers/md/dm-dust.c > > +++ b/drivers/md/dm-dust.c > > @@ -138,20 +138,22 @@ static int dust_add_block(struct dust_device *dd, unsigned long long block, > > return 0; > > } > > > > -static int dust_query_block(struct dust_device *dd, unsigned long long block) > > +static int dust_query_block(struct dust_device *dd, unsigned long long block, char *result, > > + unsigned int maxlen, unsigned int *sz_ptr) > > { > > struct badblock *bblock; > > unsigned long flags; > > + unsigned int sz = *sz_ptr; > > > > spin_lock_irqsave(&dd->dust_lock, flags); > > bblock = dust_rb_search(&dd->badblocklist, block); > > if (bblock != NULL) > > - DMINFO("%s: block %llu found in badblocklist", __func__, block); > > + DMEMIT("block %llu found in badblocklist", block); > > else > > - DMINFO("%s: block %llu not found in badblocklist", __func__, block); > > + DMEMIT("block %llu not found in badblocklist", block); > > spin_unlock_irqrestore(&dd->dust_lock, flags); > > > > - return 0; > > + return 1; > > First, thank you very much for this patch. After the concerns to > convert some functions to use DMEMIT were brought up, I was trying to > start the conversion, when this patch arrived, so I installed it, and > tested it. > > I do have a question, though: > > First, I see that in dust_query_block() (above) and > dust_clear_badblocks(), the "return 0" statement is changed to "return > 1". > > (Additionally, there is a change from "r = 0" to "r = 1", in the > "countbadblocks" message handler) > > On testing the functions, they still work, but why was this change > made? Is it related to the use of DMEMIT? It is, but we need to review the returns closer. Looked to me that 1 was being returned even if nothing was DMEMIT()'d.. but I could be wrong (only looked quickly). I also noticed that some output was changed to not include __func__. Please review that the output reflects what you'd like displayed. Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Fri, Jun 19, 2020 at 11:45 AM Mike Snitzer <snitzer@redhat.com> wrote: > > On Fri, Jun 19 2020 at 11:40am -0400, > Bryan Gurney <bgurney@redhat.com> wrote: > > > On Fri, Jun 19, 2020 at 8:37 AM yangerkun <yangerkun@huawei.com> wrote: > > > > > > Some type of message(queryblock/countbadblocks/removebadblock) may better > > > report results to user directly. Do it with DMEMIT. > > > > > > Signed-off-by: yangerkun <yangerkun@huawei.com> > > > --- > > > drivers/md/dm-dust.c | 31 ++++++++++++++++++------------- > > > 1 file changed, 18 insertions(+), 13 deletions(-) > > > > > > diff --git a/drivers/md/dm-dust.c b/drivers/md/dm-dust.c > > > index ff03b90072c5..a0c75c104de0 100644 > > > --- a/drivers/md/dm-dust.c > > > +++ b/drivers/md/dm-dust.c > > > @@ -138,20 +138,22 @@ static int dust_add_block(struct dust_device *dd, unsigned long long block, > > > return 0; > > > } > > > > > > -static int dust_query_block(struct dust_device *dd, unsigned long long block) > > > +static int dust_query_block(struct dust_device *dd, unsigned long long block, char *result, > > > + unsigned int maxlen, unsigned int *sz_ptr) > > > { > > > struct badblock *bblock; > > > unsigned long flags; > > > + unsigned int sz = *sz_ptr; > > > > > > spin_lock_irqsave(&dd->dust_lock, flags); > > > bblock = dust_rb_search(&dd->badblocklist, block); > > > if (bblock != NULL) > > > - DMINFO("%s: block %llu found in badblocklist", __func__, block); > > > + DMEMIT("block %llu found in badblocklist", block); > > > else > > > - DMINFO("%s: block %llu not found in badblocklist", __func__, block); > > > + DMEMIT("block %llu not found in badblocklist", block); > > > spin_unlock_irqrestore(&dd->dust_lock, flags); > > > > > > - return 0; > > > + return 1; > > > > First, thank you very much for this patch. After the concerns to > > convert some functions to use DMEMIT were brought up, I was trying to > > start the conversion, when this patch arrived, so I installed it, and > > tested it. > > > > I do have a question, though: > > > > First, I see that in dust_query_block() (above) and > > dust_clear_badblocks(), the "return 0" statement is changed to "return > > 1". > > > > (Additionally, there is a change from "r = 0" to "r = 1", in the > > "countbadblocks" message handler) > > > > On testing the functions, they still work, but why was this change > > made? Is it related to the use of DMEMIT? > > It is, but we need to review the returns closer. Looked to me that 1 > was being returned even if nothing was DMEMIT()'d.. but I could be wrong > (only looked quickly). > > I also noticed that some output was changed to not include __func__. > Please review that the output reflects what you'd like displayed. > > Mike > After adding __func__ back into the string, here's what it looks like: $ sudo dmsetup message dust1 0 addbadblock 60 $ sudo dmsetup message dust1 0 queryblock 60 dust_query_block: block 60 found in badblocklist $ sudo dmsetup message dust1 0 queryblock 61 dust_query_block: block 61 not found in badblocklist I feel that the output's origin is more clear, so I'd like to leave __func__ in the output. I took a look at the DMEMIT calls, and in all three cases, it looks like there's something DMEMIT()'d: dust_query_block: either "block found", or "block not found". dust_clear_badblocks: either "no badblocks found", or "badblocks cleared". result of "message ... countbadblocks": always prints "%llu badblocks found". Thanks, Bryan -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Fri, Jun 19 2020 at 1:23pm -0400, Bryan Gurney <bgurney@redhat.com> wrote: > On Fri, Jun 19, 2020 at 11:45 AM Mike Snitzer <snitzer@redhat.com> wrote: > > > > On Fri, Jun 19 2020 at 11:40am -0400, > > Bryan Gurney <bgurney@redhat.com> wrote: > > > > > On Fri, Jun 19, 2020 at 8:37 AM yangerkun <yangerkun@huawei.com> wrote: > > > > > > > > Some type of message(queryblock/countbadblocks/removebadblock) may better > > > > report results to user directly. Do it with DMEMIT. > > > > > > > > Signed-off-by: yangerkun <yangerkun@huawei.com> > > > > --- > > > > drivers/md/dm-dust.c | 31 ++++++++++++++++++------------- > > > > 1 file changed, 18 insertions(+), 13 deletions(-) > > > > > > > > diff --git a/drivers/md/dm-dust.c b/drivers/md/dm-dust.c > > > > index ff03b90072c5..a0c75c104de0 100644 > > > > --- a/drivers/md/dm-dust.c > > > > +++ b/drivers/md/dm-dust.c > > > > @@ -138,20 +138,22 @@ static int dust_add_block(struct dust_device *dd, unsigned long long block, > > > > return 0; > > > > } > > > > > > > > -static int dust_query_block(struct dust_device *dd, unsigned long long block) > > > > +static int dust_query_block(struct dust_device *dd, unsigned long long block, char *result, > > > > + unsigned int maxlen, unsigned int *sz_ptr) > > > > { > > > > struct badblock *bblock; > > > > unsigned long flags; > > > > + unsigned int sz = *sz_ptr; > > > > > > > > spin_lock_irqsave(&dd->dust_lock, flags); > > > > bblock = dust_rb_search(&dd->badblocklist, block); > > > > if (bblock != NULL) > > > > - DMINFO("%s: block %llu found in badblocklist", __func__, block); > > > > + DMEMIT("block %llu found in badblocklist", block); > > > > else > > > > - DMINFO("%s: block %llu not found in badblocklist", __func__, block); > > > > + DMEMIT("block %llu not found in badblocklist", block); > > > > spin_unlock_irqrestore(&dd->dust_lock, flags); > > > > > > > > - return 0; > > > > + return 1; > > > > > > First, thank you very much for this patch. After the concerns to > > > convert some functions to use DMEMIT were brought up, I was trying to > > > start the conversion, when this patch arrived, so I installed it, and > > > tested it. > > > > > > I do have a question, though: > > > > > > First, I see that in dust_query_block() (above) and > > > dust_clear_badblocks(), the "return 0" statement is changed to "return > > > 1". > > > > > > (Additionally, there is a change from "r = 0" to "r = 1", in the > > > "countbadblocks" message handler) > > > > > > On testing the functions, they still work, but why was this change > > > made? Is it related to the use of DMEMIT? > > > > It is, but we need to review the returns closer. Looked to me that 1 > > was being returned even if nothing was DMEMIT()'d.. but I could be wrong > > (only looked quickly). > > > > I also noticed that some output was changed to not include __func__. > > Please review that the output reflects what you'd like displayed. > > > > Mike > > > > After adding __func__ back into the string, here's what it looks like: > > $ sudo dmsetup message dust1 0 addbadblock 60 > $ sudo dmsetup message dust1 0 queryblock 60 > dust_query_block: block 60 found in badblocklist > $ sudo dmsetup message dust1 0 queryblock 61 > dust_query_block: block 61 not found in badblocklist > > I feel that the output's origin is more clear, so I'd like to leave > __func__ in the output. > > I took a look at the DMEMIT calls, and in all three cases, it looks > like there's something DMEMIT()'d: > > dust_query_block: either "block found", or "block not found". > dust_clear_badblocks: either "no badblocks found", or "badblocks cleared". > result of "message ... countbadblocks": always prints "%llu badblocks found". OK, can you provide an incremental patch that restores the __func__ where you'd like? I can deal with it, but I'll be slower to do so. Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Fri, Jun 19, 2020 at 1:29 PM Mike Snitzer <snitzer@redhat.com> wrote: > > On Fri, Jun 19 2020 at 1:23pm -0400, > Bryan Gurney <bgurney@redhat.com> wrote: > > > On Fri, Jun 19, 2020 at 11:45 AM Mike Snitzer <snitzer@redhat.com> wrote: > > > > > > On Fri, Jun 19 2020 at 11:40am -0400, > > > Bryan Gurney <bgurney@redhat.com> wrote: > > > > > > > On Fri, Jun 19, 2020 at 8:37 AM yangerkun <yangerkun@huawei.com> wrote: > > > > > > > > > > Some type of message(queryblock/countbadblocks/removebadblock) may better > > > > > report results to user directly. Do it with DMEMIT. > > > > > > > > > > Signed-off-by: yangerkun <yangerkun@huawei.com> > > > > > --- > > > > > drivers/md/dm-dust.c | 31 ++++++++++++++++++------------- > > > > > 1 file changed, 18 insertions(+), 13 deletions(-) > > > > > > > > > > diff --git a/drivers/md/dm-dust.c b/drivers/md/dm-dust.c > > > > > index ff03b90072c5..a0c75c104de0 100644 > > > > > --- a/drivers/md/dm-dust.c > > > > > +++ b/drivers/md/dm-dust.c > > > > > @@ -138,20 +138,22 @@ static int dust_add_block(struct dust_device *dd, unsigned long long block, > > > > > return 0; > > > > > } > > > > > > > > > > -static int dust_query_block(struct dust_device *dd, unsigned long long block) > > > > > +static int dust_query_block(struct dust_device *dd, unsigned long long block, char *result, > > > > > + unsigned int maxlen, unsigned int *sz_ptr) > > > > > { > > > > > struct badblock *bblock; > > > > > unsigned long flags; > > > > > + unsigned int sz = *sz_ptr; > > > > > > > > > > spin_lock_irqsave(&dd->dust_lock, flags); > > > > > bblock = dust_rb_search(&dd->badblocklist, block); > > > > > if (bblock != NULL) > > > > > - DMINFO("%s: block %llu found in badblocklist", __func__, block); > > > > > + DMEMIT("block %llu found in badblocklist", block); > > > > > else > > > > > - DMINFO("%s: block %llu not found in badblocklist", __func__, block); > > > > > + DMEMIT("block %llu not found in badblocklist", block); > > > > > spin_unlock_irqrestore(&dd->dust_lock, flags); > > > > > > > > > > - return 0; > > > > > + return 1; > > > > > > > > First, thank you very much for this patch. After the concerns to > > > > convert some functions to use DMEMIT were brought up, I was trying to > > > > start the conversion, when this patch arrived, so I installed it, and > > > > tested it. > > > > > > > > I do have a question, though: > > > > > > > > First, I see that in dust_query_block() (above) and > > > > dust_clear_badblocks(), the "return 0" statement is changed to "return > > > > 1". > > > > > > > > (Additionally, there is a change from "r = 0" to "r = 1", in the > > > > "countbadblocks" message handler) > > > > > > > > On testing the functions, they still work, but why was this change > > > > made? Is it related to the use of DMEMIT? > > > > > > It is, but we need to review the returns closer. Looked to me that 1 > > > was being returned even if nothing was DMEMIT()'d.. but I could be wrong > > > (only looked quickly). > > > > > > I also noticed that some output was changed to not include __func__. > > > Please review that the output reflects what you'd like displayed. > > > > > > Mike > > > > > > > After adding __func__ back into the string, here's what it looks like: > > > > $ sudo dmsetup message dust1 0 addbadblock 60 > > $ sudo dmsetup message dust1 0 queryblock 60 > > dust_query_block: block 60 found in badblocklist > > $ sudo dmsetup message dust1 0 queryblock 61 > > dust_query_block: block 61 not found in badblocklist > > > > I feel that the output's origin is more clear, so I'd like to leave > > __func__ in the output. > > > > I took a look at the DMEMIT calls, and in all three cases, it looks > > like there's something DMEMIT()'d: > > > > dust_query_block: either "block found", or "block not found". > > dust_clear_badblocks: either "no badblocks found", or "badblocks cleared". > > result of "message ... countbadblocks": always prints "%llu badblocks found". > > OK, can you provide an incremental patch that restores the __func__ > where you'd like? I can deal with it, but I'll be slower to do so. > > Mike > Yes, I just sent the patch to dm-devel. Thanks, Bryan -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Fri, Jun 19, 2020 at 1:43 PM Bryan Gurney <bgurney@redhat.com> wrote: > > On Fri, Jun 19, 2020 at 1:29 PM Mike Snitzer <snitzer@redhat.com> wrote: > > > > On Fri, Jun 19 2020 at 1:23pm -0400, > > Bryan Gurney <bgurney@redhat.com> wrote: > > > > > On Fri, Jun 19, 2020 at 11:45 AM Mike Snitzer <snitzer@redhat.com> wrote: > > > > > > > > On Fri, Jun 19 2020 at 11:40am -0400, > > > > Bryan Gurney <bgurney@redhat.com> wrote: > > > > > > > > > On Fri, Jun 19, 2020 at 8:37 AM yangerkun <yangerkun@huawei.com> wrote: > > > > > > > > > > > > Some type of message(queryblock/countbadblocks/removebadblock) may better > > > > > > report results to user directly. Do it with DMEMIT. > > > > > > > > > > > > Signed-off-by: yangerkun <yangerkun@huawei.com> > > > > > > --- > > > > > > drivers/md/dm-dust.c | 31 ++++++++++++++++++------------- > > > > > > 1 file changed, 18 insertions(+), 13 deletions(-) > > > > > > > > > > > > diff --git a/drivers/md/dm-dust.c b/drivers/md/dm-dust.c > > > > > > index ff03b90072c5..a0c75c104de0 100644 > > > > > > --- a/drivers/md/dm-dust.c > > > > > > +++ b/drivers/md/dm-dust.c > > > > > > @@ -138,20 +138,22 @@ static int dust_add_block(struct dust_device *dd, unsigned long long block, > > > > > > return 0; > > > > > > } > > > > > > > > > > > > -static int dust_query_block(struct dust_device *dd, unsigned long long block) > > > > > > +static int dust_query_block(struct dust_device *dd, unsigned long long block, char *result, > > > > > > + unsigned int maxlen, unsigned int *sz_ptr) > > > > > > { > > > > > > struct badblock *bblock; > > > > > > unsigned long flags; > > > > > > + unsigned int sz = *sz_ptr; > > > > > > > > > > > > spin_lock_irqsave(&dd->dust_lock, flags); > > > > > > bblock = dust_rb_search(&dd->badblocklist, block); > > > > > > if (bblock != NULL) > > > > > > - DMINFO("%s: block %llu found in badblocklist", __func__, block); > > > > > > + DMEMIT("block %llu found in badblocklist", block); > > > > > > else > > > > > > - DMINFO("%s: block %llu not found in badblocklist", __func__, block); > > > > > > + DMEMIT("block %llu not found in badblocklist", block); > > > > > > spin_unlock_irqrestore(&dd->dust_lock, flags); > > > > > > > > > > > > - return 0; > > > > > > + return 1; > > > > > > > > > > First, thank you very much for this patch. After the concerns to > > > > > convert some functions to use DMEMIT were brought up, I was trying to > > > > > start the conversion, when this patch arrived, so I installed it, and > > > > > tested it. > > > > > > > > > > I do have a question, though: > > > > > > > > > > First, I see that in dust_query_block() (above) and > > > > > dust_clear_badblocks(), the "return 0" statement is changed to "return > > > > > 1". > > > > > > > > > > (Additionally, there is a change from "r = 0" to "r = 1", in the > > > > > "countbadblocks" message handler) > > > > > > > > > > On testing the functions, they still work, but why was this change > > > > > made? Is it related to the use of DMEMIT? > > > > > > > > It is, but we need to review the returns closer. Looked to me that 1 > > > > was being returned even if nothing was DMEMIT()'d.. but I could be wrong > > > > (only looked quickly). > > > > > > > > I also noticed that some output was changed to not include __func__. > > > > Please review that the output reflects what you'd like displayed. > > > > > > > > Mike > > > > > > > > > > After adding __func__ back into the string, here's what it looks like: > > > > > > $ sudo dmsetup message dust1 0 addbadblock 60 > > > $ sudo dmsetup message dust1 0 queryblock 60 > > > dust_query_block: block 60 found in badblocklist > > > $ sudo dmsetup message dust1 0 queryblock 61 > > > dust_query_block: block 61 not found in badblocklist > > > > > > I feel that the output's origin is more clear, so I'd like to leave > > > __func__ in the output. > > > > > > I took a look at the DMEMIT calls, and in all three cases, it looks > > > like there's something DMEMIT()'d: > > > > > > dust_query_block: either "block found", or "block not found". > > > dust_clear_badblocks: either "no badblocks found", or "badblocks cleared". > > > result of "message ... countbadblocks": always prints "%llu badblocks found". > > > > OK, can you provide an incremental patch that restores the __func__ > > where you'd like? I can deal with it, but I'll be slower to do so. > > > > Mike > > > > Yes, I just sent the patch to dm-devel. > > > Thanks, > > Bryan I'm going to work on folding the review changes above into a v4 series. Thanks, Bryan -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/md/dm-dust.c b/drivers/md/dm-dust.c index ff03b90072c5..a0c75c104de0 100644 --- a/drivers/md/dm-dust.c +++ b/drivers/md/dm-dust.c @@ -138,20 +138,22 @@ static int dust_add_block(struct dust_device *dd, unsigned long long block, return 0; } -static int dust_query_block(struct dust_device *dd, unsigned long long block) +static int dust_query_block(struct dust_device *dd, unsigned long long block, char *result, + unsigned int maxlen, unsigned int *sz_ptr) { struct badblock *bblock; unsigned long flags; + unsigned int sz = *sz_ptr; spin_lock_irqsave(&dd->dust_lock, flags); bblock = dust_rb_search(&dd->badblocklist, block); if (bblock != NULL) - DMINFO("%s: block %llu found in badblocklist", __func__, block); + DMEMIT("block %llu found in badblocklist", block); else - DMINFO("%s: block %llu not found in badblocklist", __func__, block); + DMEMIT("block %llu not found in badblocklist", block); spin_unlock_irqrestore(&dd->dust_lock, flags); - return 0; + return 1; } static int __dust_map_read(struct dust_device *dd, sector_t thisblock) @@ -259,11 +261,13 @@ static bool __dust_clear_badblocks(struct rb_root *tree, return true; } -static int dust_clear_badblocks(struct dust_device *dd) +static int dust_clear_badblocks(struct dust_device *dd, char *result, unsigned int maxlen, + unsigned int *sz_ptr) { unsigned long flags; struct rb_root badblocklist; unsigned long long badblock_count; + unsigned int sz = *sz_ptr; spin_lock_irqsave(&dd->dust_lock, flags); badblocklist = dd->badblocklist; @@ -273,11 +277,11 @@ static int dust_clear_badblocks(struct dust_device *dd) spin_unlock_irqrestore(&dd->dust_lock, flags); if (!__dust_clear_badblocks(&badblocklist, badblock_count)) - DMINFO("%s: no badblocks found", __func__); + DMEMIT("no badblocks found"); else - DMINFO("%s: badblocks cleared", __func__); + DMEMIT("badblocks cleared"); - return 0; + return 1; } /* @@ -383,7 +387,7 @@ static void dust_dtr(struct dm_target *ti) } static int dust_message(struct dm_target *ti, unsigned int argc, char **argv, - char *result_buf, unsigned int maxlen) + char *result, unsigned int maxlen) { struct dust_device *dd = ti->private; sector_t size = i_size_read(dd->dev->bdev->bd_inode) >> SECTOR_SHIFT; @@ -393,6 +397,7 @@ static int dust_message(struct dm_target *ti, unsigned int argc, char **argv, unsigned char wr_fail_cnt; unsigned int tmp_ui; unsigned long flags; + unsigned int sz = 0; char dummy; if (argc == 1) { @@ -410,12 +415,12 @@ static int dust_message(struct dm_target *ti, unsigned int argc, char **argv, r = 0; } else if (!strcasecmp(argv[0], "countbadblocks")) { spin_lock_irqsave(&dd->dust_lock, flags); - DMINFO("countbadblocks: %llu badblock(s) found", + DMEMIT("countbadblocks: %llu badblock(s) found", dd->badblock_count); spin_unlock_irqrestore(&dd->dust_lock, flags); - r = 0; + r = 1; } else if (!strcasecmp(argv[0], "clearbadblocks")) { - r = dust_clear_badblocks(dd); + r = dust_clear_badblocks(dd, result, maxlen, &sz); } else if (!strcasecmp(argv[0], "quiet")) { if (!dd->quiet_mode) dd->quiet_mode = true; @@ -441,7 +446,7 @@ static int dust_message(struct dm_target *ti, unsigned int argc, char **argv, else if (!strcasecmp(argv[0], "removebadblock")) r = dust_remove_block(dd, block); else if (!strcasecmp(argv[0], "queryblock")) - r = dust_query_block(dd, block); + r = dust_query_block(dd, block, result, maxlen, &sz); else invalid_msg = true;
Some type of message(queryblock/countbadblocks/removebadblock) may better report results to user directly. Do it with DMEMIT. Signed-off-by: yangerkun <yangerkun@huawei.com> --- drivers/md/dm-dust.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-)