nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Coly Li <colyli@suse.de>
To: Geliang Tang <geliangtang@gmail.com>
Cc: antlists@youngman.org.uk, Dan Williams <dan.j.williams@intel.com>,
	nvdimm@lists.linux.dev, linux-raid@vger.kernel.org,
	linux-kernel@vger.kernel.org, Hannes Reinecke <hare@suse.de>,
	linux-block@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
	NeilBrown <neilb@suse.de>, Richard Fan <richard.fan@suse.com>,
	Vishal L Verma <vishal.l.verma@intel.com>
Subject: Re: [PATCH v3 3/6] badblocks: improvement badblocks_set() for multiple ranges handling
Date: Mon, 27 Sep 2021 16:16:27 +0800	[thread overview]
Message-ID: <8c1f4bc4-20c9-4711-4038-c23be7ae9a80@suse.de> (raw)
In-Reply-To: <8efbe758-74e4-32ea-d41e-639b750b27c0@gmail.com>

On 9/27/21 3:30 PM, Geliang Tang wrote:
> On 9/14/21 00:36, Coly Li wrote:

[snipped]
>> + * 2.1.1) If S and E are both acked or unacked range, the setting 
>> range S can
>> + *    be merged into existing bad range E. The result is,
>> + *        +-------------+
>> + *        |      S      |
>> + *        +-------------+
>> + * 2.1.2) If S is uncked setting and E is acked, the setting will be 
>> dinied, and
>
> uncked -> unacked
> dinied?
>

You are correct, it should be unacked.

>> + *    the result is,
>> + *        +-------------+
>> + *        |      E      |
>> + *        +-------------+
>> + * 2.1.3) If S is acked setting and E is unacked, range S can 
>> overwirte on E.
>> + *    An extra slot from the bad blocks table will be allocated for 
>> S, and head
>> + *    of E will move to end of the inserted range E. The result is,
>> + *        +--------+----+
>> + *        |    S   | E  |
>> + *        +--------+----+
>> + * 2.2) The setting range size == already set range size
>> + * 2.2.1) If S and E are both acked or unacked range, the setting 
>> range S can
>> + *    be merged into existing bad range E. The result is,
>> + *        +-------------+
>> + *        |      S      |
>> + *        +-------------+
>> + * 2.2.2) If S is uncked setting and E is acked, the setting will be 
>> dinied, and
>
> uncked -> unacked

Yes, thanks for pointing out the typo. I will fix them in next version.

[snipped]
>> +/* Do exact work to set bad block range into the bad block table */
>> +static int _badblocks_set(struct badblocks *bb, sector_t s, int 
>> sectors,
>> +              int acknowledged)
>> +{
>> +    u64 *p;
>> +    struct badblocks_context bad;
>> +    int prev = -1, hint = -1;
>> +    int len = 0, added = 0;
>> +    int retried = 0, space_desired = 0;
>> +    int rv = 0;
>> +    unsigned long flags;
>
> orig_start and orig_len are used in _badblocks_set() only, we can drop 
> them from struct badblocks_context, declare two local variables instead:
>
>         sector_t orig_start;
>         int orig_len;
>

It's fair, let me change it in next version.

[snipped]

Thanks for your review!

Coly Li

  reply	other threads:[~2021-09-27  8:16 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-13 16:36 [PATCH v3 0/7] badblocks improvement for multiple bad block ranges Coly Li
2021-09-13 16:36 ` [PATCH v3 1/6] badblocks: add more helper structure and routines in badblocks.h Coly Li
2021-09-27  7:23   ` Geliang Tang
2021-09-27  8:23     ` Coly Li
2021-09-13 16:36 ` [PATCH v3 2/6] badblocks: add helper routines for badblock ranges handling Coly Li
2021-09-27  7:25   ` Geliang Tang
2021-09-27  8:17     ` Coly Li
2021-09-13 16:36 ` [PATCH v3 3/6] badblocks: improvement badblocks_set() for multiple " Coly Li
2021-09-27  7:30   ` Geliang Tang
2021-09-27  8:16     ` Coly Li [this message]
2021-09-13 16:36 ` [PATCH v3 4/6] badblocks: improve badblocks_clear() " Coly Li
2021-09-13 16:36 ` [PATCH v3 5/6] badblocks: improve badblocks_check() " Coly Li
2021-09-13 16:36 ` [PATCH v3 6/6] badblocks: switch to the improved badblock handling code Coly Li
2021-09-13 16:36 ` [PATCH] test: user space code to test badblocks APIs Coly Li
2021-09-23  5:59 ` Too large badblocks sysfs file (was: [PATCH v3 0/7] badblocks improvement for multiple bad block ranges) Coly Li
2021-09-23  6:08   ` Greg Kroah-Hartman
2021-09-23  6:14     ` Coly Li
2021-09-23  6:47       ` Greg Kroah-Hartman
2021-09-23  7:13         ` Coly Li
2021-09-23  9:40   ` Hannes Reinecke
2021-09-23  9:57     ` Greg Kroah-Hartman
2021-09-23 10:09   ` NeilBrown
2021-09-23 10:39     ` Greg Kroah-Hartman
2021-09-23 12:55     ` Coly Li
2021-12-02 12:52 [PATCH v4 0/6] badblocks improvement for multiple bad block ranges Coly Li
2021-12-02 12:52 ` [PATCH v3 3/6] badblocks: improvement badblocks_set() for multiple ranges handling Coly Li
2021-12-09  7:28   ` Geliang Tang
2021-12-09 18:37     ` Wols Lists
2021-12-10  6:55       ` Coly Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8c1f4bc4-20c9-4711-4038-c23be7ae9a80@suse.de \
    --to=colyli@suse.de \
    --cc=antlists@youngman.org.uk \
    --cc=axboe@kernel.dk \
    --cc=dan.j.williams@intel.com \
    --cc=geliangtang@gmail.com \
    --cc=hare@suse.de \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=nvdimm@lists.linux.dev \
    --cc=richard.fan@suse.com \
    --cc=vishal.l.verma@intel.com \
    --subject='Re: [PATCH v3 3/6] badblocks: improvement badblocks_set() for multiple ranges handling' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).