Re: [PATCH v3 1/4] dm dust: report some message results back to user directly

[Mike Snitzer <snitzer@redhat.com>]

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