| Message ID | 20210913163643.10233-2-colyli@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | badblocks improvement for multiple bad block ranges | expand |
Hi Coly, On 9/14/21 00:36, Coly Li wrote: > This patch adds the following helper structure and routines into > badblocks.h, > - struct badblocks_context > This structure is used in improved badblocks code for bad table > iteration. > - BB_END() > The macro to culculate end LBA of a bad range record from bad > table. > - badblocks_full() and badblocks_empty() > The inline routines to check whether bad table is full or empty. > - set_changed() and clear_changed() > The inline routines to set and clear 'changed' tag from struct > badblocks. > > These new helper structure and routines can help to make the code more > clear, they will be used in the improved badblocks code in following > patches. > > Signed-off-by: Coly Li <colyli@suse.de> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Hannes Reinecke <hare@suse.de> > Cc: Jens Axboe <axboe@kernel.dk> > Cc: NeilBrown <neilb@suse.de> > Cc: Richard Fan <richard.fan@suse.com> > Cc: Vishal L Verma <vishal.l.verma@intel.com> > --- > include/linux/badblocks.h | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/include/linux/badblocks.h b/include/linux/badblocks.h > index 2426276b9bd3..166161842d1f 100644 > --- a/include/linux/badblocks.h > +++ b/include/linux/badblocks.h > @@ -15,6 +15,7 @@ > #define BB_OFFSET(x) (((x) & BB_OFFSET_MASK) >> 9) > #define BB_LEN(x) (((x) & BB_LEN_MASK) + 1) > #define BB_ACK(x) (!!((x) & BB_ACK_MASK)) > +#define BB_END(x) (BB_OFFSET(x) + BB_LEN(x)) > #define BB_MAKE(a, l, ack) (((a)<<9) | ((l)-1) | ((u64)(!!(ack)) << 63)) > > /* Bad block numbers are stored sorted in a single page. > @@ -41,6 +42,14 @@ struct badblocks { > sector_t size; /* in sectors */ > }; > > +struct badblocks_context { > + sector_t start; > + sector_t len; I think the type of 'len' should be 'int' instead of 'sector_t', since we used 'int sectors' as one of the arguments of _badblocks_set(). > + int ack; > + sector_t orig_start; > + sector_t orig_len; I think 'orig_start' and 'orig_len' can be dropped, see comments in patch 3. > +}; > + > int badblocks_check(struct badblocks *bb, sector_t s, int sectors, > sector_t *first_bad, int *bad_sectors); > int badblocks_set(struct badblocks *bb, sector_t s, int sectors, > @@ -63,4 +72,27 @@ static inline void devm_exit_badblocks(struct device *dev, struct badblocks *bb) > } > badblocks_exit(bb); > } > + > +static inline int badblocks_full(struct badblocks *bb) > +{ > + return (bb->count >= MAX_BADBLOCKS); > +} > + > +static inline int badblocks_empty(struct badblocks *bb) > +{ > + return (bb->count == 0); > +} > + > +static inline void set_changed(struct badblocks *bb) > +{ > + if (bb->changed != 1) > + bb->changed = 1; > +} > + > +static inline void clear_changed(struct badblocks *bb) > +{ > + if (bb->changed != 0) > + bb->changed = 0; > +} > + > #endif >
On 9/27/21 3:23 PM, Geliang Tang wrote: > Hi Coly, > > On 9/14/21 00:36, Coly Li wrote: >> This patch adds the following helper structure and routines into >> badblocks.h, >> - struct badblocks_context >> This structure is used in improved badblocks code for bad table >> iteration. >> - BB_END() >> The macro to culculate end LBA of a bad range record from bad >> table. >> - badblocks_full() and badblocks_empty() >> The inline routines to check whether bad table is full or empty. >> - set_changed() and clear_changed() >> The inline routines to set and clear 'changed' tag from struct >> badblocks. >> >> These new helper structure and routines can help to make the code more >> clear, they will be used in the improved badblocks code in following >> patches. >> >> Signed-off-by: Coly Li <colyli@suse.de> >> Cc: Dan Williams <dan.j.williams@intel.com> >> Cc: Hannes Reinecke <hare@suse.de> >> Cc: Jens Axboe <axboe@kernel.dk> >> Cc: NeilBrown <neilb@suse.de> >> Cc: Richard Fan <richard.fan@suse.com> >> Cc: Vishal L Verma <vishal.l.verma@intel.com> >> --- >> include/linux/badblocks.h | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/include/linux/badblocks.h b/include/linux/badblocks.h >> index 2426276b9bd3..166161842d1f 100644 >> --- a/include/linux/badblocks.h >> +++ b/include/linux/badblocks.h >> @@ -15,6 +15,7 @@ >> #define BB_OFFSET(x) (((x) & BB_OFFSET_MASK) >> 9) >> #define BB_LEN(x) (((x) & BB_LEN_MASK) + 1) >> #define BB_ACK(x) (!!((x) & BB_ACK_MASK)) >> +#define BB_END(x) (BB_OFFSET(x) + BB_LEN(x)) >> #define BB_MAKE(a, l, ack) (((a)<<9) | ((l)-1) | ((u64)(!!(ack)) << >> 63)) >> /* Bad block numbers are stored sorted in a single page. >> @@ -41,6 +42,14 @@ struct badblocks { >> sector_t size; /* in sectors */ >> }; >> +struct badblocks_context { >> + sector_t start; >> + sector_t len; > > I think the type of 'len' should be 'int' instead of 'sector_t', since > we used 'int sectors' as one of the arguments of _badblocks_set(). OK, I will change it. > >> + int ack; >> + sector_t orig_start; >> + sector_t orig_len; > > I think 'orig_start' and 'orig_len' can be dropped, see comments in > patch 3. Yes, I will change it in next version. Please review the new version latter. Thanks for your review. Coly Li
diff --git a/include/linux/badblocks.h b/include/linux/badblocks.h index 2426276b9bd3..166161842d1f 100644 --- a/include/linux/badblocks.h +++ b/include/linux/badblocks.h @@ -15,6 +15,7 @@ #define BB_OFFSET(x) (((x) & BB_OFFSET_MASK) >> 9) #define BB_LEN(x) (((x) & BB_LEN_MASK) + 1) #define BB_ACK(x) (!!((x) & BB_ACK_MASK)) +#define BB_END(x) (BB_OFFSET(x) + BB_LEN(x)) #define BB_MAKE(a, l, ack) (((a)<<9) | ((l)-1) | ((u64)(!!(ack)) << 63)) /* Bad block numbers are stored sorted in a single page. @@ -41,6 +42,14 @@ struct badblocks { sector_t size; /* in sectors */ }; +struct badblocks_context { + sector_t start; + sector_t len; + int ack; + sector_t orig_start; + sector_t orig_len; +}; + int badblocks_check(struct badblocks *bb, sector_t s, int sectors, sector_t *first_bad, int *bad_sectors); int badblocks_set(struct badblocks *bb, sector_t s, int sectors, @@ -63,4 +72,27 @@ static inline void devm_exit_badblocks(struct device *dev, struct badblocks *bb) } badblocks_exit(bb); } + +static inline int badblocks_full(struct badblocks *bb) +{ + return (bb->count >= MAX_BADBLOCKS); +} + +static inline int badblocks_empty(struct badblocks *bb) +{ + return (bb->count == 0); +} + +static inline void set_changed(struct badblocks *bb) +{ + if (bb->changed != 1) + bb->changed = 1; +} + +static inline void clear_changed(struct badblocks *bb) +{ + if (bb->changed != 0) + bb->changed = 0; +} + #endif
This patch adds the following helper structure and routines into badblocks.h, - struct badblocks_context This structure is used in improved badblocks code for bad table iteration. - BB_END() The macro to culculate end LBA of a bad range record from bad table. - badblocks_full() and badblocks_empty() The inline routines to check whether bad table is full or empty. - set_changed() and clear_changed() The inline routines to set and clear 'changed' tag from struct badblocks. These new helper structure and routines can help to make the code more clear, they will be used in the improved badblocks code in following patches. Signed-off-by: Coly Li <colyli@suse.de> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Hannes Reinecke <hare@suse.de> Cc: Jens Axboe <axboe@kernel.dk> Cc: NeilBrown <neilb@suse.de> Cc: Richard Fan <richard.fan@suse.com> Cc: Vishal L Verma <vishal.l.verma@intel.com> --- include/linux/badblocks.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)